diff --git a/vrtManager/instance.py b/vrtManager/instance.py index f783dd78..79729784 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -248,10 +248,11 @@ class wvmInstances(wvmConnect): @staticmethod def moved_disks(xml, copies): """xml with each copied disk's source at its path here; a disk is - found by its target and its source path before the copy.""" + found by its source path before the copy, as the next start's + definition may name it with another target (a changed bus).""" tree = etree.fromstring(xml) sources = [(source, copy["dest"]) for copy in copies for source in tree.xpath( - "devices/disk[target/@dev=$target]/source[@file=$path]", target=copy["target"], path=copy["path"])] + "devices/disk/source[@file=$path]", path=copy["path"])] for source, dest in sources: source.set("file", dest) return etree.tostring(tree, encoding="unicode") @@ -367,9 +368,12 @@ class wvmInstances(wvmConnect): "%(target)s (%(path)s): pool %(pool)s of the destination host already has a file %(file)s"), **disk, pool=name, file=file)) continue - if any(copy["dest"] == f"{directory}/{file}" for copy in copies): + # one file, one copy: the definitions name a disk's copy by its path + other = next((copy for copy in copies if copy["dest"] == f"{directory}/{file}" + or copy["path"] == row["path"]), None) + if other is not None: problems.append(util.message(gettext_noop("%(target)s (%(path)s): another disk is copied to %(dest)s"), - **disk, dest=f"{directory}/{file}")) + **disk, dest=other["dest"])) continue copies.append({"target": row["target"], "path": row["path"], "dest": f"{directory}/{file}", "pool": pool}) diff --git a/vrtManager/tests/test_disk_copy_migration.py b/vrtManager/tests/test_disk_copy_migration.py index dff4be43..12ded116 100644 --- a/vrtManager/tests/test_disk_copy_migration.py +++ b/vrtManager/tests/test_disk_copy_migration.py @@ -231,6 +231,13 @@ class DiskCopyTestCase(unittest.TestCase): self.assertIn(f'', dest_xml) self.assertIn('', dest_xml) + def test_the_next_start_names_the_copy_under_another_target_too(self): + """A bus change waiting for the next start renames the disk there.""" + source = Source(1, domain(disk(f"{IMAGES}/vm.qcow2")), inactive=domain(disk(f"{IMAGES}/vm.qcow2", "sda"))) + migrate(destination(Pool("default", IMAGES), Pool("fast", FAST)), source, {"vda": "fast"}) + params = source.instance.migrate3.call_args.args[1] + self.assertIn(f'', params[libvirt.VIR_MIGRATE_PARAM_PERSIST_XML]) + def test_a_disk_only_the_next_start_has_must_be_shared_too(self): """It is neither copied nor moved: a file of its name on the destination would be another VM's, used at the next start.""" @@ -241,6 +248,14 @@ class DiskCopyTestCase(unittest.TestCase): migrate(destination(Pool("default", IMAGES)), source, {"vda": "default"}) source.instance.migrate3.assert_not_called() + def test_one_file_two_disks_use_is_copied_once(self): + xml = domain(disk(f"{IMAGES}/data.raw", "vda", fmt="raw"), disk(f"{IMAGES}/data.raw", "vdb", fmt="raw")) + source = Source(1, xml, pools=(IMAGES,)) + dest = destination(Pool("fast", FAST), Pool("slow", "/srv/slow")) + with self.assertRaisesRegex(util.OperationError, f"vdb \\({IMAGES}/data.raw\\): another disk is copied to {FAST}/data.raw"): + migrate(dest, source, {"vda": "fast", "vdb": "slow"}) + source.instance.migrate3.assert_not_called() + def test_two_copies_never_go_to_one_file(self): source = Source(1, domain(disk(f"{IMAGES}/vm.qcow2"), disk(f"{FAST}/vm.qcow2", "vdb")), pools=(IMAGES, FAST)) with self.assertRaisesRegex(util.OperationError, "vdb .*another disk is copied to /srv/slow/vm.qcow2"):