Skip to content

Conversation

cyril-s
Copy link
Contributor

@cyril-s cyril-s commented Dec 5, 2019

Description

Add key base_volume_copy for libvirt_volume resource which controls
whether to invoke StorageVolCreateXML (defualt) or StorageVolCreateXMLFrom.

Motivation

Currently workflow of creating libvirt volumes from base volumes works good for qcow2 images. New volumes are created with backingStore xml element in definition which associates them with base qcow2 copy-on-write image and allows them to grow in size without any limitations.
However for logical storage pools, backingStore element instructs libvirt to create lvm snapshot which has the following implications:

  1. The size of a snapshot volume cannot be larger than it's base volume
  2. The size of a snapshot cannot be smaller than it's base volume (which is limited by this check).

So workflow of creating libvirt volumes from base volumes for logical storage pool is not very useful:
it's impossible to create volume of arbitrary size from base volume.

To address this problem, this PR adds possibility to invoke StorageVolCreateXMLFrom which creates new ordinary logical volume of any size and just copies the base volume to it.

For qcow2 images, this option can be useful as well. The resulting volume will be created without any association with its backing volume—neither in its XML definition nor in the underlying storage backend.
For example, if a VM is expected to have a long lifetime with major OS upgrades, there is no sense in keeping a dangling CoW base volume that contains only dirty blocks.

Notes

Workaround for qcow2

In some cases, StorageVolCreateXMLFrom() ignores the requested capacity and creates a volume with the same capacity as the original.
To test this behaviour, create 6 XML definitions: 2 for base volumes with a capacity of 10 MiB, and 4 for copy volumes with a capacity of 20 MiB:

<!-- base-qcow2.xml -->
<volume>
  <name>test-base-qcow2</name>
  <capacity unit='MiB'>10</capacity>
  <target>
    <format type='qcow2'/>
  </target>
</volume>

<!-- base-raw.xml -->
<volume>
  <name>test-base-raw</name>
  <capacity unit='MiB'>10</capacity>
  <target>
    <format type='raw'/>
  </target>
</volume>

<!-- copy-qcow2-from-qcow2.xml -->
<volume>
  <name>test-copy-qcow2-from-qcow2</name>
  <capacity unit='MiB'>20</capacity>
  <target>
    <format type='qcow2'/>
  </target>
</volume>

<!-- copy-qcow2-from-raw.xml -->
<volume>
  <name>test-copy-qcow2-from-raw</name>
  <capacity unit='MiB'>20</capacity>
  <target>
    <format type='qcow2'/>
  </target>
</volume>

<!-- copy-raw-from-qcow2.xml -->
<volume>
  <name>test-copy-raw-from-qcow2</name>
  <capacity unit='MiB'>20</capacity>
  <target>
    <format type='raw'/>
  </target>
</volume>

<!-- copy-raw-from-raw.xml -->
<volume>
  <name>test-copy-raw-from-raw</name>
  <capacity unit='MiB'>20</capacity>
  <target>
    <format type='raw'/>
  </target>
</volume>
$ virsh vol-create --pool default --file base-raw.xml
$ virsh vol-create --pool default --file base-qcow2.xml
$ virsh vol-create-from --pool default --inputpool default --file copy-raw-from-raw.xml --vol test-base-raw
$ virsh vol-create-from --pool default --inputpool default --file copy-raw-from-qcow2.xml --vol test-base-qcow2
$ virsh vol-create-from --pool default --inputpool default --file copy-qcow2-from-raw.xml --vol test-base-raw
$ virsh vol-create-from --pool default --inputpool default --file copy-qcow2-from-qcow2.xml --vol test-base-qcow2
$ for f in (ls test-copy-*); qemu-img info $f | grep -e '^image:' -e '^file format:' -e '^virtual size:'; echo; end
image: test-copy-qcow2-from-qcow2
file format: qcow2
virtual size: 10 MiB (10485760 bytes)

image: test-copy-qcow2-from-raw
file format: qcow2
virtual size: 10 MiB (10485760 bytes)

image: test-copy-raw-from-qcow2
file format: raw
virtual size: 10 MiB (10485760 bytes)

image: test-copy-raw-from-raw
file format: raw
virtual size: 20 MiB (20971520 bytes)

Only the test-copy-raw-from-raw volume has the requested capacity of 20 MiB. This is because libvirt handles this case differently and copies images directly.
In other cases, libvirt calls qemu-img convert, which retains the original volume capacity.

To improve this behavior, I've added a resize step after volume creation. The resize is performed only if:

  • The base_volume_copy option is true
  • The size option is set and it's greater than the volume's allocation and the base volume's capacity
  • The volume's pool has the type dir
  • Either the volume's or the base volume's format is qcow2

There are many other format options. I think supporting the two most used formats, raw and qcow2, is a great start.

Other storage backends

Behavior for storage backends other than dir and logical was not tested.

@cyril-s cyril-s force-pushed the add_StorageVolCreateXMLFrom_feature branch from ccfee61 to 52df264 Compare December 5, 2019 13:14
@MalloZup
Copy link
Collaborator

thx for PR. I will check this soon. stay tuned. 🌞

@lblasc
Copy link

lblasc commented Sep 25, 2020

Just tested this pull request, works great!
Without base_volume_copy I'm unable to get VM root partition in specified size, as LVM COW snapshot cannot be resized.

Here is my working main.tf:

provider "libvirt" {
#  uri   = "qemu+ssh://root@builder1/system"
  uri = "qemu:///system"
}

provider "cloudinit" {}

resource "libvirt_volume" "ubuntu_focal_image" {
  name = "ubuntu_focal_image"
  pool = "default"
  source = "/var/lib/libvirt/focal-server-cloudimg-amd64-disk-kvm.raw"
}

resource "libvirt_volume" "disk" {
  name = "u1-root"
  base_volume_id = libvirt_volume.ubuntu_focal_image.id
  base_volume_copy = true
  size = 100 * 1024 * 1024 * 1024
}

resource "tls_private_key" "bootstrap_key" {
  algorithm = "RSA"
}

data "cloudinit_config" "user_data" {
  gzip = false
  base64_encode = false
  part {
    content = <<EOF
#cloud-config
hostname: u1
fqdn: u1.test
manage_etc_hosts: true
users:
  - name: root
    ssh-authorized-keys:
      - ${tls_private_key.bootstrap_key.public_key_openssh}
# only cert auth via ssh (console access can still login)
ssh_pwauth: false
disable_root: false
packages:
  - qemu-guest-agent
growpart:
  mode: auto
  devices: ['/']
# written to /var/log/cloud-init-output.log
final_message: "The system is finally up, after $UPTIME seconds"
EOF
  }

}

# Use CloudInit to add the instance
resource "libvirt_cloudinit_disk" "commoninit" {
  name      = "commoninit.iso"
  user_data = data.cloudinit_config.user_data.rendered
}

resource "libvirt_domain" "u1" {
  name   = "u1"
  memory = 32 * 1024
  vcpu   = 32
  autostart = true

  network_interface {
    wait_for_lease = true
    network_name = "default"
  }

  disk {
    volume_id = libvirt_volume.disk.id
    scsi = "true"
  }

  cloudinit = libvirt_cloudinit_disk.commoninit.id

  console {
    type = "pty"
    target_type = "serial"
    target_port = "0"
  }

  graphics {
    type = "spice"
    listen_type = "address"
    autoport = true
  }

  connection {
    host = self.network_interface.0.addresses.0
    type = "ssh"
    user = "root"
    private_key = tls_private_key.bootstrap_key.private_key_pem
    agent = false
  }

  provisioner "remote-exec" {
    inline = ["cloud-init status --wait"]
  }

  provisioner "salt-masterless" {
    local_pillar_roots = "${path.root}/../../../../salt-pillar"
    local_state_tree   = "${path.root}/../../../../salt-states"
    minion_config_file = "minion.conf"
  }

  provisioner "remote-exec" {
    inline = [
      "rm -rf /srv/pillar",
      "rm -rf /srv/salt",
      "systemctl stop salt-minion",
      "systemctl disable salt-minion"
    ]
  }
}

Copy link
Owner

@dmacvicar dmacvicar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

Something I don't get to understand from this feature is why virStorageVolCreateXMLFrom takes the backing store to copy as an extra parameter, when there is a <backingStore> element already present in the definition. Can you enlighten me?
Why doesn't virStorageVolCreateXMLFrom simply copy the backing store mentioned in the definition? why does it need a second argument?

@pznamensky
Copy link

Super useful feature for us! 🎉

@beddari
Copy link

beddari commented Dec 2, 2022

@dmacvicar seems like this is just how virStorageVolCreateXMLFrom is defined? https://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolCreateXMLFrom

This feature is really useful and should be considered. A common use case is where you don't want or need a backing store for the resulting instance, just a standalone volume.

beddari added a commit to safespring/terraform-provider-libvirt that referenced this pull request Dec 5, 2022
This adds a new key base_volume_copy to the volume resource to choose
whether to invoke StorageVolCreateXML (default) or StorageVolCreateXMLFrom

Adapted from original PR dmacvicar#675
@cyril-s cyril-s force-pushed the add_StorageVolCreateXMLFrom_feature branch 2 times, most recently from e0a579c to 5240165 Compare May 1, 2025 16:15
@cyril-s
Copy link
Contributor Author

cyril-s commented May 1, 2025

@dmacvicar I've rebased this PR onto main and refactored it a bit. I also decided to add a resize step after volume creation if base_volume_copy is true, to improve libvirt's behavior with qcow2 images. See the updated PR description for a detailed explanation.

Thanks for the PR.

Something I don't get to understand from this feature is why virStorageVolCreateXMLFrom takes the backing store to copy as an extra parameter, when there is a <backingStore> element already present in the definition. Can you enlighten me? Why doesn't virStorageVolCreateXMLFrom simply copy the backing store mentioned in the definition? why does it need a second argument?

<backingStore> isn't added to the volume definition when base_volume_copy is true, so virStorageVolCreateXMLFrom needs to know which image to copy from. base_volume_copy is essentially equivalent to virsh vol-create-from.

@cyril-s cyril-s force-pushed the add_StorageVolCreateXMLFrom_feature branch from 5240165 to 94e0dbb Compare May 1, 2025 16:19
@dmitry-sinina
Copy link

@dmacvicar we are using this patch for a long time for VMs on LVM storage. Could you merge it to mainstream?

@cyril-s cyril-s force-pushed the add_StorageVolCreateXMLFrom_feature branch from 94e0dbb to dfdadba Compare May 27, 2025 15:08
@dmitry-sinina
Copy link

@MalloZup

If base_volume_copy option is set to false (default), a volume is created
using StorageVolCreateXML(). Otherwise, StorageVolCreateXMLFrom() is used
and the resulting volume doesn't have any association with the original.
@cyril-s cyril-s force-pushed the add_StorageVolCreateXMLFrom_feature branch from dfdadba to 851fc64 Compare August 4, 2025 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants