diff --git a/vrtManager/instance.py b/vrtManager/instance.py index 94232683..98728e0c 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -20,6 +20,9 @@ try: VIR_DOMAIN_UNDEFINE_KEEP_NVRAM, VIR_DOMAIN_UNDEFINE_KEEP_TPM, VIR_ERR_NO_DOMAIN, + VIR_ERR_NO_STORAGE_POOL, + VIR_ERR_NO_STORAGE_VOL, + VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE, VIR_DOMAIN_BLOCK_RESIZE_BYTES, VIR_STORAGE_VOL_FILE, VIR_MIGRATE_AUTO_CONVERGE, @@ -151,6 +154,15 @@ class wvmInstances(wvmConnect): mode.append(label) dom = conn.instance + definitions = {dom.XMLDesc(0)} + if state != 5: + definitions.add(dom.XMLDesc(VIR_DOMAIN_XML_INACTIVE)) + missing = self.missing_disks(definitions) + if missing: + raise util.OperationError( + "The destination host does not have these disks of the VM: " + + ", ".join(f"{target} ({source})" for target, source in missing) + ) dom_arch = conn.get_arch() dom_emulator = conn.get_dom_emulator() @@ -170,6 +182,76 @@ class wvmInstances(wvmConnect): dom.migrate(self.wvm, flags, None, uri, 0) return ", ".join(mode) + def missing_disks(self, definitions): + """(target, source) of the disks in the VM definitions (XML) that are + not volumes of this host's active storage pools: the VM's own files, + their backing files (as the definition or this host's volume lists + them), CD-ROM images, block devices and pool volumes. Network disks + (RBD, iSCSI, ...) are reached the same way from every host and are not + checked; other types (dir, nvme, ...) cannot be and count as missing. + Pools are refreshed before a disk counts as missing, as a file another + host made on shared storage is listed after that.""" + disks = {} + for xml in definitions: + for disk in ElementTree.fromstring(xml).iter("disk"): + target = disk.find("target") + target = target.get("dev") if target is not None else "?" + for element in (disk, *disk.iter("backingStore")): + source = element.find("source") + kind = element.get("type") + if source is None or kind == "network": + continue + if kind == "volume": + disks[(source.get("pool"), source.get("volume"))] = target + elif kind in ("file", "block") and (source.get("file") or source.get("dev")): + disks[(None, source.get("file") or source.get("dev"))] = target + elif kind not in ("file", "block"): + disks[(kind, None)] = target + + def missing(): + checked, unseen, queue = set(), [], list(disks.items()) + while queue: + key, target = queue.pop() + if key in checked: + continue + checked.add(key) + volume = self.find_volume(*key) + if volume is None: + unseen.append((target, key)) + continue + backing = ElementTree.fromstring(volume.XMLDesc(0)).findtext("backingStore/path") + if backing and "://" not in backing: # a URI is a network backing image + queue.append(((None, backing), target)) + return unseen + + if missing(): + for pool in self.wvm.listAllStoragePools(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE): + with contextlib.suppress(libvirtError): + pool.refresh(0) + return [(target, self.disk_label(*key)) for target, key in missing()] + + @staticmethod + def disk_label(pool, name): + if name is None: + return f"{pool} disk, cannot be checked" + return name if pool is None else f"{pool}/{name}" + + def find_volume(self, pool, name): + """The volume name of pool, or with pool None the volume at path name; + None when this host has none (or name is None: a disk type that + cannot be checked).""" + if name is None: + return None + try: + if pool is None: + return self.wvm.storageVolLookupByPath(name) + pool = self.wvm.storagePoolLookupByName(pool) + return pool.storageVolLookupByName(name) if pool.isActive() else None + except libvirtError as error: + if error.get_error_code() in (VIR_ERR_NO_STORAGE_VOL, VIR_ERR_NO_STORAGE_POOL): + return None + raise + 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 diff --git a/vrtManager/test_migration.py b/vrtManager/test_migration.py index 676e02ae..ecd44cfd 100644 --- a/vrtManager/test_migration.py +++ b/vrtManager/test_migration.py @@ -42,7 +42,11 @@ class FakeDomain: def UUIDString(self): return "u-1" + inactive_xml = None # the persistent definition, when it differs + def XMLDesc(self, flags): + if flags & libvirt.VIR_DOMAIN_XML_INACTIVE and self.inactive_xml: + return self.inactive_xml return self.xml def snapshotNum(self, flags=0): @@ -80,13 +84,63 @@ class FakeDomain: return self -class FakeHost: - """The destination's libvirt connection.""" +def no_volume(): + error = libvirt.libvirtError("Storage volume not found") + error.err = (libvirt.VIR_ERR_NO_STORAGE_VOL,) + return error - def __init__(self, existing=()): + +class FakeVolume: + def __init__(self, backing): + self.backing = backing + + def XMLDesc(self, flags=0): + if self.backing is None: + return "x" + return f"{self.backing}" + + +class FakePool: + def __init__(self, host, active=True): + self.host = host + self.active = active + + def isActive(self): + return self.active + + def refresh(self, flags=0): + self.host.refreshes += 1 + self.host.volumes |= self.host.unlisted + self.host.unlisted = set() + + def storageVolLookupByName(self, name): + return self.host.storageVolLookupByPath(name) + + +class FakeHost: + """The destination's libvirt connection. volumes: the paths its pools + list; unlisted: files its pools show after a refresh.""" + + def __init__(self, existing=(), volumes=(), unlisted=(), backing=None, inactive_pools=()): self.existing = set(existing) self.defined = [] self.new = [] + self.volumes = set(volumes) + self.unlisted = set(unlisted) + self.backing = backing or {} # path: the path of its backing file + self.inactive_pools = set(inactive_pools) + self.refreshes = 0 + + def storageVolLookupByPath(self, path): + if path not in self.volumes: + raise no_volume() + return FakeVolume(self.backing.get(path)) + + def storagePoolLookupByName(self, name): + return FakePool(self, active=name not in self.inactive_pools) + + def listAllStoragePools(self, flags=0): + return [FakePool(self)] def lookup(self, key): if key in self.existing: @@ -248,3 +302,101 @@ class MigrationModeTestCase(unittest.TestCase): self.assertEqual(flags & (live | unsafe | converge | compressed), live) self.assertEqual(self.migrate(5), ("offline", None)) self.assertEqual(self.migrate(5, offline=True), ("offline", None)) + + +def with_disks(*disks): + return "%s" % "".join(disks) + + +FILE_DISK = "" + + +class DestinationDisksTestCase(unittest.TestCase): + """Every disk of the VM must be a volume the destination knows; the + VM is refused before anything changes otherwise, naming the disks.""" + + def migrate(self, host, xml, state=1, inactive_xml=None): + source = FakeSource(state, xml=xml) + source.instance.inactive_xml = inactive_xml + live = state != 5 + destination(host).moveto(source, "vm", live=live, unsafe=False, offline=not live) + return source + + def test_known_volumes_migrate(self): + host = FakeHost(volumes={"/pool/vm.qcow2", "/pool/base.qcow2", "/iso/os.iso"}) + xml = with_disks( + "" + "", + "", + "", # empty + "", + "", + ) + self.assertEqual(len(self.migrate(host, xml).instance.calls), 1) + self.assertEqual(host.defined, []) + self.migrate(host, xml, state=5) + self.assertEqual(len(host.defined), 1) + + def test_missing_disks_are_refused_and_named(self): + cases = { + "vda \\(/pool/vm.qcow2\\)": with_disks(FILE_DISK), + "sda \\(/iso/os.iso\\)": with_disks( + ""), + "vda \\(/pool/base.qcow2\\)": with_disks( + "" + ""), + "vdb \\(/dev/sdz\\)": with_disks( + ""), + "vdc \\(p/v\\)": with_disks( + ""), + } + for missing, xml in cases.items(): + for state in (1, 5): + with self.subTest(missing=missing, state=state): + host = FakeHost(volumes=set() if "vm.qcow2" in missing else {"/pool/vm.qcow2"}) + source = FakeSource(state, xml=xml) + with self.assertRaisesRegex(util.OperationError, missing): + destination(host).moveto(source, "vm", live=state == 1, unsafe=False, offline=state == 5) + self.assertEqual((source.instance.calls, host.defined), ([], [])) + + def test_a_disk_of_the_persistent_definition_is_checked_too(self): + host = FakeHost(volumes={"/pool/vm.qcow2"}) + inactive = with_disks(FILE_DISK, "" + "") + with self.assertRaisesRegex(util.OperationError, "vdb \\(/pool/data.img\\)"): + self.migrate(host, with_disks(FILE_DISK), inactive_xml=inactive) + + def test_pools_are_refreshed_before_a_disk_counts_as_missing(self): + # a file another host made on shared storage is listed after a refresh + host = FakeHost(unlisted={"/pool/vm.qcow2"}) + self.assertEqual(len(self.migrate(host, with_disks(FILE_DISK)).instance.calls), 1) + self.assertEqual(host.refreshes, 1) + host = FakeHost(volumes={"/pool/vm.qcow2"}) + self.migrate(host, with_disks(FILE_DISK)) + self.assertEqual(host.refreshes, 0) + + def test_backing_files_the_definition_does_not_list_are_checked(self): + # an inactive definition has no : the volume tells + for state in (1, 5): + with self.subTest(state=state): + host = FakeHost(volumes={"/pool/vm.qcow2", "/pool/mid.qcow2"}, + backing={"/pool/vm.qcow2": "/pool/mid.qcow2", "/pool/mid.qcow2": "/local/base.qcow2"}) + with self.assertRaisesRegex(util.OperationError, "vda \\(/local/base.qcow2\\)"): + self.migrate(host, with_disks(FILE_DISK), state=state) + host.volumes.add("/local/base.qcow2") + self.migrate(host, with_disks(FILE_DISK), state=state) + + def test_a_network_backing_image_is_not_checked(self): + host = FakeHost(volumes={"/pool/vm.qcow2"}, backing={"/pool/vm.qcow2": "nbd://storage.example.org:10809/base"}) + self.assertEqual(len(self.migrate(host, with_disks(FILE_DISK)).instance.calls), 1) + + def test_disks_that_cannot_be_checked_are_refused(self): + xml = with_disks("") + with self.assertRaisesRegex(util.OperationError, "vdb \\(dir disk, cannot be checked\\)"): + self.migrate(FakeHost(), xml, state=5) + + def test_a_volume_of_an_inactive_pool_is_missing(self): + host = FakeHost(volumes={"v"}, inactive_pools={"p"}) + xml = with_disks("") + with self.assertRaisesRegex(util.OperationError, "vdc \\(p/v\\)"): + self.migrate(host, xml)