diff --git a/vrtManager/instance.py b/vrtManager/instance.py index c3d4545e..862ff88e 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -16,6 +16,10 @@ try: VIR_DOMAIN_RUNNING, 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_DOMAIN_BLOCK_RESIZE_BYTES, VIR_STORAGE_VOL_FILE, VIR_MIGRATE_AUTO_CONVERGE, @@ -147,8 +151,81 @@ class wvmInstances(wvmConnect): "Destination host emulator is different. Cannot be migrated" ) + if conn.get_status() == 5: + # Workaround for a libvirt bug (RHEL-156800), see move_definition. + # Once supported hosts all run a fixed libvirt, this branch and + # move_definition can go: VIR_MIGRATE_OFFLINE is set above. + self.move_definition(dom) + return dom.migrate(self.wvm, flags, None, uri, 0) + 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. + + Workaround, to be removed in a later version: libvirt's own offline + migration (VIR_MIGRATE_OFFLINE) fails with "operation failed: domain + is no longer running" on libvirt builds that have upstream commit + a4f610ff ("Always offer block dirty bitmaps during migration") but not + its fix 59fde80f, which is in libvirt 12.2.0. RHEL backported the + first (RHEL-145770) and tracks the fix as RHEL-156800; RHEL/Rocky 9.8 + with libvirt 11.10.0-12.el9_8 is affected. When supported hosts all + have the fix, offline migration can go back to dom.migrate.""" + if dom.snapshotNum(0): + raise util.OperationError("A VM with snapshots cannot be migrated") + if dom.hasManagedSaveImage(0): + raise util.OperationError("A VM with a saved state cannot be migrated") + for lookup, key in ((self.wvm.lookupByUUIDString, dom.UUIDString()), (self.wvm.lookupByName, dom.name())): + try: + lookup(key) + except libvirtError as error: + if error.get_error_code() != VIR_ERR_NO_DOMAIN: + raise + else: + 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) + try: + started = dom.isActive() + except libvirtError as error: + if error.get_error_code() != VIR_ERR_NO_DOMAIN: + raise + started = False # gone from the source: the move is done + if started: + # started meanwhile by another client: it stays on the source + 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) + raise + + @staticmethod + def undo_definition(dom, new, keep, 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: + on_source = dom.isPersistent() + except libvirtError: + on_source = False + if not on_source: + raise util.OperationError( + f"{error}; the VM may now be defined only on the destination host: check both hosts" + ) from error + try: + new.undefineFlags(keep) + 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}" + ) from error + def graphics_type(self, name): inst = self.get_instance(name) console_type = util.get_xml_path( diff --git a/vrtManager/test_migration.py b/vrtManager/test_migration.py index 6e265e23..fd916369 100644 --- a/vrtManager/test_migration.py +++ b/vrtManager/test_migration.py @@ -1,4 +1,5 @@ -"""Migration flags: the source definition never stays behind.""" +"""Migration: the source definition never stays behind; a shut-off VM's +definition is moved by the app, not by libvirt's offline migration.""" import unittest @@ -8,21 +9,102 @@ from django.conf import settings if not settings.configured: settings.configure(MAC_OUI="52:54:10") +from vrtManager import util from vrtManager.instance import wvmInstances +def no_domain(): + error = libvirt.libvirtError("Domain not found") + error.err = (libvirt.VIR_ERR_NO_DOMAIN,) + return error + + class FakeDomain: - def __init__(self): + def __init__(self, xml="", snapshots=0, saved=False, undefine_fails=False, undefine_lost=False, + started=False): self.calls = [] + self.xml = xml + self.snapshots = snapshots + self.saved = saved + self.undefine_fails = undefine_fails + self.undefine_lost = undefine_lost # undefined, but the reply is lost + self.started = started # started by another client during the move + self.undefined = None + self.redefined = [] + self.restored_autostart = None def migrate(self, dconn, flags, dname, uri, bandwidth): self.calls.append((flags, uri)) + def name(self): + return "vm" + + def UUIDString(self): + return "u-1" + + def XMLDesc(self, flags): + return self.xml + + def snapshotNum(self, flags=0): + return self.snapshots + + def hasManagedSaveImage(self, flags=0): + return self.saved + + def undefineFlags(self, flags): + if self.undefine_fails: + raise libvirt.libvirtError("undefine failed") + self.undefined = flags + if self.undefine_lost: + raise libvirt.libvirtError("Cannot recv data") + + def isPersistent(self): + return self.undefined is None or bool(self.redefined) + + def isActive(self): + if not self.started and self.undefined is not None: + raise no_domain() # libvirt: an undefined shut-off VM is gone + return self.started + + def autostart(self): + return 1 + + def setAutostart(self, value): + self.restored_autostart = value + + def connect(self): + return self + + def defineXML(self, xml): + self.redefined.append(xml) + return self + + +class FakeHost: + """The destination's libvirt connection.""" + + def __init__(self, existing=()): + self.existing = set(existing) + self.defined = [] + self.new = [] + + def lookup(self, key): + if key in self.existing: + return FakeDomain() + raise no_domain() + + lookupByName = lookupByUUIDString = lookup + + def defineXML(self, xml): + self.defined.append(xml) + self.new.append(FakeDomain()) + return self.new[-1] + class FakeSource: - def __init__(self, state): + def __init__(self, state, **domain): self.state = state - self.instance = FakeDomain() + self.instance = FakeDomain(**domain) def get_status(self): return self.state @@ -34,16 +116,16 @@ class FakeSource: return "/usr/bin/qemu-system-x86_64" -def destination(): +def destination(host=None): dest = wvmInstances.__new__(wvmInstances) - dest.wvm = object() + dest.wvm = host or FakeHost() dest.get_emulator = lambda arch: "/usr/bin/qemu-system-x86_64" return dest class MigrationFlagsTestCase(unittest.TestCase): def test_the_source_definition_is_always_removed(self): - for state, live, offline in ((1, True, False), (5, False, True)): + for state, live, offline in ((1, True, False), (3, False, False)): with self.subTest(state=state): source = FakeSource(state) destination().moveto(source, "vm", live=live, unsafe=False, offline=offline) @@ -58,3 +140,69 @@ class MigrationFlagsTestCase(unittest.TestCase): source = FakeSource(1) destination().moveto(source, "vm", live=True, unsafe=False, offline=False) self.assertIsNone(source.instance.calls[0][1]) + + +class ShutOffMigrationTestCase(unittest.TestCase): + """libvirt's offline migration fails on some builds (RHEL-156800); a + shut-off VM is only its definition, so the app moves that itself.""" + + def test_the_definition_moves_to_the_destination(self): + host = FakeHost() + source = FakeSource(5, xml="vm") + 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) + + 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_refused_before_anything_changes(self): + cases = { + "snapshots": (FakeHost(), {"snapshots": 2}), + "saved state": (FakeHost(), {"saved": True}), + "same uuid on the destination": (FakeHost(existing={"u-1"}), {}), + "same name on the destination": (FakeHost(existing={"vm"}), {}), + } + for case, (host, domain) in cases.items(): + with self.subTest(case=case): + source = FakeSource(5, **domain) + with self.assertRaises(util.OperationError): + destination(host).moveto(source, "vm", live=False, unsafe=False, offline=True) + self.assertEqual(host.defined, []) + self.assertIsNone(source.instance.undefined) + + def test_a_failed_source_undefine_removes_the_new_definition(self): + host = FakeHost() + source = FakeSource(5, undefine_fails=True) + with self.assertRaises(libvirt.libvirtError): + destination(host).moveto(source, "vm", live=False, unsafe=False, offline=True) + self.assertIsNotNone(host.new[0].undefined) + + def test_the_destination_definition_stays_when_the_source_has_none(self): + host = FakeHost() + source = FakeSource(5, undefine_lost=True) + with self.assertRaisesRegex(util.OperationError, "only on the destination"): + destination(host).moveto(source, "vm", live=False, unsafe=False, offline=True) + self.assertIsNone(host.new[0].undefined) + + def test_a_failed_rollback_is_reported(self): + host = FakeHost() + source = FakeSource(5, undefine_fails=True) + host.defineXML = lambda xml: host.new.append(FakeDomain(undefine_fails=True)) or host.new[-1] + with self.assertRaisesRegex(util.OperationError, "undefine failed; the VM is now defined on both hosts"): + destination(host).moveto(source, "vm", live=False, unsafe=False, offline=True) + + def test_a_vm_started_meanwhile_stays_on_the_source(self): + host = FakeHost() + source = FakeSource(5, xml="vm", started=True) + with self.assertRaisesRegex(util.OperationError, "started on the source"): + destination(host).moveto(source, "vm", live=False, unsafe=False, offline=True) + self.assertEqual(source.instance.redefined, ["vm"]) + self.assertEqual(source.instance.restored_autostart, 1) + self.assertIsNotNone(host.new[0].undefined)