add possibility to choose whether snapshot base volume or copy

[cyril-s]

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.

[MalloZup]

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

[lblasc]

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"
    ]
  }
}

[dmacvicar]

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]

Super useful feature for us! 🎉

[beddari]

@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.

[cyril-s]

@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.

[dmitry-sinina]

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

[dmitry-sinina]

@MalloZup

[dmacvicar]

ℹ️ ℹ️ ℹ️ ℹ️ ℹ️ ℹ️

This contribution is relevant to the legacy version of the provider, which is now in the v0.8 branch of this repository.

Future development is based on a new provider, which is not compatible with this one, nor does share code.

As the new provider solves many issues with the legacy, and to make development in my free time more enjoyable, I have decided to close all PRs for the legacy provider, and to ask to check if the contribution would apply to the new one. This also to encourage trying the new version.

• https://registry.terraform.io/providers/dmacvicar/libvirt/latest

and check the documentation:

• https://github.com/dmacvicar/terraform-provider-libvirt/blob/main/README.md
• https://registry.terraform.io/providers/dmacvicar/libvirt/latest/docs

You are free to reopen the PR for v0.8, which is targetted now to the v0.8 branch. We may also start a discussion if we can assemble a team of maintainters for the legacy branch. My efforts will go into the new provider.

I ask you to check the new provider and re-evaluate this contribution. 🙏