From e822e27f5ef747c22626bf88fc50310c7dff15be Mon Sep 17 00:00:00 2001 From: catborise Date: Sat, 7 Nov 2026 22:41:22 +0300 Subject: [PATCH] fix(migrate): the next start's definition names a copied disk under its new path A copied disk was found by its target as well as its path: after a bus change waiting for the next start (vda to sda), the next start's definition kept the source path, which the destination does not have, and the VM would not start again there. The path alone finds it, so two disks of one file can no longer be copied to two places. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kk4h2TcUiaWmU5MUszYSea --- vrtManager/instance.py | 12 ++++++++---- vrtManager/tests/test_disk_copy_migration.py | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) 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"):