diff --git a/vrtManager/instance.py b/vrtManager/instance.py index cd1b5c45..356820f4 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -18,8 +18,6 @@ try: VIR_DOMAIN_XML_SECURE, VIR_DOMAIN_XML_INACTIVE, VIR_DOMAIN_XML_MIGRATABLE, - VIR_DOMAIN_UNDEFINE_KEEP_NVRAM, - VIR_DOMAIN_UNDEFINE_KEEP_TPM, VIR_ERR_NO_DOMAIN, VIR_ERR_NO_STORAGE_POOL, VIR_ERR_NO_STORAGE_VOL, @@ -156,6 +154,8 @@ class wvmInstances(wvmConnect): mode.append(label) dom = conn.instance + if state == 5: + self.refuse_local_state(dom.XMLDesc(0)) definitions = {dom.XMLDesc(0)} if state != 5: definitions.add(dom.XMLDesc(VIR_DOMAIN_XML_INACTIVE)) @@ -184,6 +184,25 @@ class wvmInstances(wvmConnect): self.migrate_within(dom, flags, uri, timeout) return ", ".join(mode) + @staticmethod + def refuse_local_state(xml): + """A shut-off VM moves without its UEFI variables (NVRAM) and TPM + state, which libvirt keeps in files of the source host: on the + destination it would start with new ones (lost boot entries and + enrolled keys, a new TPM that no longer unlocks disks). A live + migration carries both.""" + tree = ElementTree.fromstring(xml) + state = [] + if tree.find("os/nvram") is not None: + state.append("NVRAM") + if tree.find("devices/tpm/backend[@type='emulator']") is not None: + state.append("TPM") + if state: + raise util.OperationError( + f"A shut-off VM with {' and '.join(state)} state cannot be migrated: the state stays on the source " + "host. Start the VM and migrate it live." + ) + def migrate_within(self, dom, flags, uri, timeout): """dom.migrate, cancelled when it has not finished after timeout seconds: a VM that changes its memory faster than the network carries @@ -290,8 +309,8 @@ class wvmInstances(wvmConnect): def move_definition(self, dom): """Migrates a shut-off VM: that is only its definition, as the disks - are on storage both hosts share. NVRAM and TPM state stay on the - source. The destination's qemu migration hook does not run for it. + are on storage both hosts share (moveto refuses a VM with NVRAM or + TPM state). The destination's qemu migration hook does not run for it. Workaround, to be removed in a later version: libvirt's own offline migration (VIR_MIGRATE_OFFLINE) fails with "operation failed: domain @@ -315,13 +334,10 @@ class wvmInstances(wvmConnect): raise util.OperationError(f"The destination host already has a VM {key}") xml = dom.XMLDesc(VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_MIGRATABLE) - keep = VIR_DOMAIN_UNDEFINE_KEEP_NVRAM - if ElementTree.fromstring(xml).find("devices/tpm") is not None: - keep |= VIR_DOMAIN_UNDEFINE_KEEP_TPM # libvirt >= 8.9 knows the flag autostart = dom.autostart() new = self.wvm.defineXML(xml) try: - dom.undefineFlags(keep) + dom.undefineFlags(0) try: started = dom.isActive() except libvirtError as error: @@ -333,11 +349,11 @@ class wvmInstances(wvmConnect): dom.connect().defineXML(xml).setAutostart(autostart) raise util.OperationError("The VM was started on the source host during the migration") except Exception as error: - self.undo_definition(dom, new, keep, error) + self.undo_definition(dom, new, error) raise @staticmethod - def undo_definition(dom, new, keep, error): + def undo_definition(dom, new, error): """After a failed move, the VM is defined on one host only: the destination definition goes only while the source still has its own.""" try: @@ -349,7 +365,7 @@ class wvmInstances(wvmConnect): f"{error}; the VM may now be defined only on the destination host: check both hosts" ) from error try: - new.undefineFlags(keep) + new.undefineFlags(0) except libvirtError as undo_error: raise util.OperationError( f"{error}; the VM is now defined on both hosts, remove it from the destination: {undo_error}" diff --git a/vrtManager/test_migration.py b/vrtManager/test_migration.py index 726baf6a..729b11ee 100644 --- a/vrtManager/test_migration.py +++ b/vrtManager/test_migration.py @@ -211,15 +211,29 @@ class ShutOffMigrationTestCase(unittest.TestCase): destination(host).moveto(source, "vm", live=False, unsafe=False, offline=True) self.assertEqual(host.defined, ["vm"]) self.assertEqual(source.instance.calls, []) - self.assertEqual(source.instance.undefined, libvirt.VIR_DOMAIN_UNDEFINE_KEEP_NVRAM) + self.assertEqual(source.instance.undefined, 0) - def test_tpm_state_stays_on_the_source(self): - source = FakeSource(5, xml="") - destination().moveto(source, "vm", live=False, unsafe=False, offline=True) - self.assertEqual( - source.instance.undefined, - libvirt.VIR_DOMAIN_UNDEFINE_KEEP_NVRAM | libvirt.VIR_DOMAIN_UNDEFINE_KEEP_TPM, - ) + def test_a_vm_with_nvram_or_tpm_state_is_refused(self): + """That state stays on the source host: the VM would start on the + destination with new UEFI variables and a new TPM.""" + cases = { + "nvram": "/var/lib/libvirt/qemu/nvram/vm_VARS.fd", + "tpm": "", + } + for case, xml in cases.items(): + with self.subTest(case=case): + host = FakeHost() + source = FakeSource(5, xml=f"{xml}") + with self.assertRaisesRegex(util.OperationError, "migrate it live"): + destination(host).moveto(source, "vm", live=False, unsafe=False, offline=True) + self.assertEqual(host.defined, []) + self.assertIsNone(source.instance.undefined) + + def test_a_tpm_without_local_state_moves(self): + xml = "" + host = FakeHost() + destination(host).moveto(FakeSource(5, xml=xml), "vm", live=False, unsafe=False, offline=True) + self.assertEqual(host.defined, [xml]) def test_refused_before_anything_changes(self): cases = {