fix(instances): a shut-off clone into a volume name its pool has is refused before it begins

libvirt refused the copy only after the job had begun; recovery then
found a volume of the planned name and ended the job uncertain, holding
the VM. The names are checked when the job is queued and before it runs,
as a live clone did; that check now refreshes the pool first, so a live
clone's free space is read current too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kk4h2TcUiaWmU5MUszYSea

instances/jobs.py

@@ -536,12 +536,15 @@ THAW_FAILED = _("The guest's file systems could not be thawed (%(error)s); thaw
 
 
 def _check_clone_state(proxy, data):
-    """A shut-off VM is copied as it is; a running or paused one only when a
-    live clone can work; other states are refused."""
+    """A shut-off VM is copied as it is, into names its pools do not have; a
+    running or paused one only when a live clone can work; other states are
+    refused."""
     status = proxy.get_status()
     if status in (1, 3):
         proxy.live_clone_targets(data, **_snapshot_free_space())
-    elif status != 5:
+    elif status == 5:
+        proxy.check_clone_volumes(data)
+    else:
         raise util.OperationError(CLONE_STATE_REFUSED)
 
 

instances/tests/test_clone_job.py

@@ -301,6 +301,15 @@ class LiveCloneJobTestCase(TransactionTestCase):
         self.proxy.live_clone_targets.assert_not_called()
         self.assertFalse(Operation.objects.get(pk=op.pk).intent["live"])
 
+    def test_a_shut_off_copy_into_a_volume_name_in_use_is_refused_before_it_begins(self):
+        """libvirt would refuse it after the job began: the job would end uncertain."""
+        self.proxy.get_status.return_value = 5
+        op = self.op()
+        self.proxy.check_clone_volumes.side_effect = OperationError("Volume copy.qcow2 already exists in storage pool default")
+        with self.assertRaisesMessage(OperationError, "Volume copy.qcow2 already exists"):
+            CloneInstance().precheck(op, None)
+        self.proxy.check_clone_volumes.assert_called_once_with(op.intent["clone"])
+
     def test_a_name_taken_since_the_job_was_queued_stops_it(self):
         op = self.op()
         other = Compute.objects.create(name="live-d", hostname="192.0.2.33", login="root", password="", type=1)

vrtManager/instance.py

@@ -2582,13 +2582,7 @@ class wvmInstance(wvmConnect):
             vol = self.get_volume_by_path(source.get("file"))
             pool = vol.storagePoolLookupByVolume()
             name = clone_data["disk-" + dev]
-            try:
-                pool.storageVolLookupByName(name)
-            except libvirtError as err:
-                if err.get_error_code() != VIR_ERR_NO_STORAGE_VOL:
-                    raise
-            else:
-                raise util.OperationError(gettext_noop("Volume %(name)s already exists in storage pool %(pool)s"), name=name, pool=pool.name())
+            self._refuse_taken(pool, name)
             capacity = vol.info()[1]
             pools[pool.name()] = pool
             copied[pool.name()] = copied.get(pool.name(), 0) + capacity
@@ -2612,6 +2606,29 @@ class wvmInstance(wvmConnect):
                     pool=name, free=free / 1024**3, size=needed / 1024**3, percent=free_percent, minimum=free_min_gib)
         return targets
 
+    @staticmethod
+    def _refuse_taken(pool, name):
+        """A copy never takes a volume's name in its pool: libvirt would
+        refuse it only after the clone began. The pool is refreshed first,
+        so its free space is current too."""
+        pool.refresh(0)
+        try:
+            pool.storageVolLookupByName(name)
+        except libvirtError as err:
+            if err.get_error_code() != VIR_ERR_NO_STORAGE_VOL:
+                raise
+        else:
+            raise util.OperationError(gettext_noop("Volume %(name)s already exists in storage pool %(pool)s"),
+                                      name=name, pool=pool.name())
+
+    def check_clone_volumes(self, clone_data):
+        """Refuses a shut-off clone whose copy of a disk would take a name its
+        pool has already, before anything is allocated (see clone_instance)."""
+        _tree, disks = self._clone_definition(clone_data, "")
+        for source, dev in disks:
+            vol = self.get_volume_by_path(source.get("file") or source.get("dev") or source.get("name"))
+            self._refuse_taken(vol.storagePoolLookupByVolume(), clone_data["disk-" + dev])
+
     def create_clone_target(self, target, owner_uid, owner_gid):
         """An empty volume for a copied disk; libvirt refuses one that exists.
         Returns its path."""

vrtManager/tests/test_live_clone.py

@@ -129,6 +129,19 @@ class LiveCloneTargetsTestCase(unittest.TestCase):
     def test_an_existing_target_volume_is_refused(self):
         self.refused(proxy(existing=True), "copy-vda.qcow2 already exists in storage pool default")
 
+    def test_the_pool_is_refreshed_before_its_names_and_free_space_are_read(self):
+        p = proxy()
+        p.live_clone_targets(FORM, 10, 5)
+        calls = [call[0] for call in p.pool.method_calls]
+        self.assertLess(calls.index("refresh"), calls.index("storageVolLookupByName"))
+        self.assertLess(calls.index("refresh"), calls.index("info"))
+
+    def test_a_shut_off_clone_into_an_existing_name_is_refused_before_it_begins(self):
+        p = proxy(existing=True)
+        with self.assertRaisesRegex(util.OperationError, "copy-vda.qcow2 already exists in storage pool default"):
+            p.check_clone_volumes(FORM)
+        proxy().check_clone_volumes(FORM)  # a free name
+
     def test_a_lookup_error_other_than_missing_is_raised(self):
         p = proxy()
         p.pool.storageVolLookupByName.side_effect = error(libvirt.VIR_ERR_INTERNAL_ERROR)