diff --git a/README.md b/README.md index 7f0436a2..700843f5 100644 --- a/README.md +++ b/README.md @@ -539,6 +539,7 @@ A rollback leaves the clone on a detached HEAD at the restored commit; the updat - An ISO uploads to an SSH compute about 3.5 times faster (SFTP writes no longer wait for each 32 KiB); a failed write still fails the upload. - The VM page shows each NIC's own QoS and each CD-ROM's own pool (they showed the previous device's), and a NIC without a source or model no longer breaks it. - A volume of an LVM or iSCSI pool can be attached to a VM, and bridge and direct NICs can be boot devices. The NIC model `default` lets libvirt choose, and `rt18139` is `rtl8139`: a VM with either did not start; a migration corrects the default NIC type setting. +- Cloning a UEFI VM no longer defines a storage pool on the host's NVRAM directory; the clone gets its own copy of the VM's UEFI variables. - A running VM without a guest agent shows the IPv4 address the host learned by ARP (the lookup always failed before). - After a live migration the VM's NVRAM file is deleted on the source, when it is in libvirt's `/var/lib/libvirt/qemu/nvram` and the destination does not share that directory (a VM created there later with the same name would start with those UEFI variables). - gstfsd finds the guest's operating system the way libguestfs inspects it, so a root on LVM works, and it always answers (an error when it finds no or several operating systems). Update `/usr/local/bin/gstfsd` on every compute (see [Adding a Compute Node](#adding-a-compute-node), step 3). diff --git a/instances/test_live_clone.py b/instances/test_live_clone.py index cd058adc..9b7d65aa 100644 --- a/instances/test_live_clone.py +++ b/instances/test_live_clone.py @@ -2,6 +2,7 @@ import os import unittest +from xml.etree import ElementTree import libvirt from django.test import SimpleTestCase @@ -84,6 +85,30 @@ class LiveCloneTestCase(SimpleTestCase): self.assertEqual(self.pool_volumes(), before) + def test_clone_of_a_uefi_vm_has_its_own_copy_of_the_nvram(self): + disk = livetest.create_volume(self.conn, P + "clone-uefi-a") + dom = livetest.define_vm(self.conn, P + "clone-uefi", [disk], uefi=True) + dom.createWithFlags(libvirt.VIR_DOMAIN_START_PAUSED) # libvirt makes the NVRAM file at the first start + dom.destroy() + vm = wvmInstance(*self.args, None, uuid=dom.UUIDString()) + source = vm.get_nvram() + + uuid = vm.clone_instance( + {"name": P + "clone-uefi-copy", "disk-vda": P + "clone-uefi-copy-a.qcow2", "disk_owner_uid": 0, "disk_owner_gid": 0} + ) + + clone = self.conn.lookupByUUIDString(uuid) + path = ElementTree.fromstring(clone.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE)).findtext("os/nvram") + self.assertEqual(path, os.path.join(os.path.dirname(source), P + "clone-uefi-copy_VARS.fd")) + with vm._nvram_pool() as pool: + copy = pool.storageVolLookupByName(os.path.basename(path)) + self.assertEqual(copy.info()[1], pool.storageVolLookupByName(os.path.basename(source)).info()[1]) + clone.createWithFlags(libvirt.VIR_DOMAIN_START_PAUSED) # qemu opens the copy + self.assertEqual( + ElementTree.fromstring(clone.XMLDesc(0)).findtext("os/nvram"), path, "libvirt replaced the copy" + ) + clone.destroy() + def test_clone_of_a_stopped_vm(self): _, vm = self.source("clone-ok") uuid = vm.clone_instance(self.clone_data("clone-ok-copy")) diff --git a/vrtManager/connection.py b/vrtManager/connection.py index 8001470b..81a35cbd 100644 --- a/vrtManager/connection.py +++ b/vrtManager/connection.py @@ -906,7 +906,9 @@ class wvmConnect(object): try: yield pool finally: - pool.destroy() + # a transient pool left by a failed destroy goes with libvirtd's next restart + with contextlib.suppress(libvirtError): + pool.destroy() @staticmethod def _find_volume(pool, name): diff --git a/vrtManager/instance.py b/vrtManager/instance.py index c1d4d937..0424dafc 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -53,7 +53,7 @@ from xml.etree import ElementTree from lxml import etree from vrtManager import util -from vrtManager.connection import wvmConnect +from vrtManager.connection import NVRAM_DIR, wvmConnect from vrtManager.storage import wvmStorage, wvmStorages # Edits that redefine the domain start from this: the persistent definition, @@ -1794,8 +1794,9 @@ class wvmInstance(wvmConnect): return ":".join(mac_tuples) def clone_instance(self, clone_data): - """Copy a shut-off VM. Inputs are checked before anything is allocated, - and on failure only the volumes this call created are removed.""" + """Copy a shut-off VM, with its disks and UEFI variables (NVRAM). + Inputs are checked before anything is allocated, and on failure only + the volumes this call created are removed. Returns the clone's UUID.""" if self.get_status() != 5: # A running guest keeps writing while its disks are copied. raise util.OperationError("Shut the VM down before cloning it") @@ -1839,25 +1840,17 @@ class wvmInstance(wvmConnect): raise ValueError(f"No MAC address for network interface {num}") net.find("mac").set("address", self.fix_mac(mac)) + # the UEFI variables: a path as text, or a since libvirt 8.5 + nvram = tree.find("os/nvram") + nvram_source = nvram.find("source") if nvram is not None else None + nvram_path = None + if nvram is not None: + if nvram.get("type", "file") != "file": + raise util.OperationError("Cannot clone a VM whose NVRAM is not a file") + nvram_path = nvram_source.get("file") if nvram_source is not None else (nvram.text or "").strip() + created = [] # paths of volumes this call created try: - src_nvram_path = self.get_nvram() - if src_nvram_path: - nvram = tree.find("os/nvram") - nvram.getparent().remove(nvram) - - # NVRAM CLONE: create pool if nvram is not in a pool. then clone it - src_nvram_name = os.path.basename(src_nvram_path) - nvram_dir = os.path.dirname(src_nvram_path) - nvram_pool_name = os.path.basename(nvram_dir) - try: - self.get_volume_by_path(src_nvram_path) - except libvirtError: - self.get_wvmStorages().create_storage("dir", nvram_pool_name, None, nvram_dir) - nvram_stg = self.get_wvmStorage(nvram_pool_name) - name = nvram_stg.clone_volume(src_nvram_name, f"{clone_data['name']}_VARS", file_suffix="fd") - created.append(os.path.join(nvram_dir, name)) - for source, dev in disks: target_file = clone_data["disk-" + dev] meta_prealloc = bool(clone_data.get("meta-" + dev)) @@ -1910,7 +1903,39 @@ class wvmInstance(wvmConnect): created.append(new_path) source.set("dev", new_path) - self._defineXML(etree.tostring(tree).decode()) + if not nvram_path: + clone = self._defineXML(etree.tostring(tree).decode()) + else: + # the clone starts with the VM's UEFI variables: boot entries, enrolled keys + if os.path.dirname(nvram_path) == NVRAM_DIR: + pools = self._nvram_pool() + else: # a volume of a pool; libvirtError when no pool holds it + pools = contextlib.nullcontext(self.get_volume_by_path(nvram_path).storagePoolLookupByVolume()) + with pools as pool: + original = pool.storageVolLookupByName(os.path.basename(nvram_path)) + # same format, owner and mode as the original: the domain names the format, + # and qemu may run without libvirt changing the file's owner + target = etree.fromstring(original.XMLDesc(0)).find("target") + volume = etree.Element("volume") + etree.SubElement(volume, "name").text = clone_data["name"] + "_VARS" + os.path.splitext(nvram_path)[1] + etree.SubElement(volume, "capacity").text = "0" + copy_target = etree.SubElement(volume, "target") + for tag in ("format", "permissions"): + if target.find(tag) is not None: + copy_target.append(target.find(tag)) + copy = pool.createXMLFrom(etree.tostring(volume).decode(), original, 0) + if nvram_source is not None: + nvram_source.set("file", copy.path()) + else: + nvram.text = copy.path() + try: + clone = self._defineXML(etree.tostring(tree).decode()) + except Exception: + try: + copy.delete(0) + except libvirtError: + created.append(copy.path()) + raise except Exception as err: leftovers = [] for path in reversed(created): @@ -1922,7 +1947,7 @@ class wvmInstance(wvmConnect): raise util.OperationError(f"Clone failed ({err}); remove these copies by hand: {', '.join(leftovers)}") from err raise - return self.get_instance(clone_data["name"]).UUIDString() + return clone.UUIDString() def get_bridge_name(self, source, source_type="net"): if source_type == "iface": diff --git a/vrtManager/test_clone_nvram.py b/vrtManager/test_clone_nvram.py new file mode 100644 index 00000000..37328b7c --- /dev/null +++ b/vrtManager/test_clone_nvram.py @@ -0,0 +1,68 @@ +"""A clone of a UEFI VM gets its own copy of the VM's NVRAM, with the +original's format, owner and mode, wherever the definition names the file.""" + +import contextlib +import unittest +from unittest.mock import MagicMock + +from django.conf import settings + +if not settings.configured: + settings.configure(MAC_OUI="52:54:10") + +from lxml import etree + +from vrtManager import util +from vrtManager.instance import wvmInstance + +ORIGINAL = """vm_VARS.qcow2k540672 +/var/lib/libvirt/qemu/nvram/vm_VARS.qcow2 +0600107107""" + + +def domain(nvram): + return f"""vmuhvm +/usr/share/edk2/ovmf/OVMF_CODE.fd{nvram}""" + + +def proxy(nvram): + p = wvmInstance.__new__(wvmInstance) # no connection: only these calls + p.get_status = lambda: 5 + p.get_instances = lambda: [] + p._XMLDesc = lambda flags: domain(nvram) + p._defineXML = MagicMock() + p.pool = MagicMock() + p.pool.storageVolLookupByName.return_value.XMLDesc.return_value = ORIGINAL + p.pool.createXMLFrom.return_value.path.return_value = "/var/lib/libvirt/qemu/nvram/copy_VARS.qcow2" + p._nvram_pool = lambda: contextlib.nullcontext(p.pool) + return p + + +class CloneNvramTestCase(unittest.TestCase): + def test_the_copy_keeps_the_format_owner_and_mode(self): + p = proxy("/var/lib/libvirt/qemu/nvram/vm_VARS.qcow2") + p.clone_instance({"name": "copy"}) + xml, original, _ = p.pool.createXMLFrom.call_args.args + volume = etree.fromstring(xml) + self.assertEqual(volume.findtext("name"), "copy_VARS.qcow2") + self.assertEqual(volume.find("target/format").get("type"), "qcow2") + self.assertEqual(volume.findtext("target/permissions/owner"), "107") + self.assertIsNone(volume.find("target/path")) + self.assertIs(original, p.pool.storageVolLookupByName.return_value) + (defined,), _ = p._defineXML.call_args + self.assertEqual(etree.fromstring(defined).findtext("os/nvram"), "/var/lib/libvirt/qemu/nvram/copy_VARS.qcow2") + + def test_an_nvram_source_element_points_at_the_copy(self): + p = proxy("\n \n") + p.clone_instance({"name": "copy"}) + p.pool.storageVolLookupByName.assert_called_once_with("vm_VARS.qcow2") + (defined,), _ = p._defineXML.call_args + self.assertEqual( + etree.fromstring(defined).find("os/nvram/source").get("file"), "/var/lib/libvirt/qemu/nvram/copy_VARS.qcow2" + ) + + def test_an_nvram_that_is_not_a_file_is_refused_before_any_copy(self): + p = proxy("") + with self.assertRaises(util.OperationError): + p.clone_instance({"name": "copy"}) + p.pool.createXMLFrom.assert_not_called()