diff --git a/instances/jobs.py b/instances/jobs.py index 16201f19..f2c908ad 100644 --- a/instances/jobs.py +++ b/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) diff --git a/instances/tests/test_clone_job.py b/instances/tests/test_clone_job.py index 0b3f941d..a3116323 100644 --- a/instances/tests/test_clone_job.py +++ b/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) diff --git a/vrtManager/instance.py b/vrtManager/instance.py index 1ce4ee5c..2e78cba5 100644 --- a/vrtManager/instance.py +++ b/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.""" diff --git a/vrtManager/tests/test_live_clone.py b/vrtManager/tests/test_live_clone.py index 7c0baaa6..c0b24d7a 100644 --- a/vrtManager/tests/test_live_clone.py +++ b/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)