{% trans "A file another VM on this host uses, directly or under its own disk, is kept, and so is the template a disk was made from." %}
+
{% trans "A file another VM uses on any of the panel's hosts, directly or under its own disk, is kept, and so is the template a disk was made from. If a host cannot be checked, the VM is not destroyed." %}
{% if instance.nvram %}
diff --git a/instances/tests/test_destroy.py b/instances/tests/test_destroy.py
index 045f6b3d..b8ac8b5a 100644
--- a/instances/tests/test_destroy.py
+++ b/instances/tests/test_destroy.py
@@ -7,6 +7,7 @@ from django.contrib.messages import get_messages
from django.test import TestCase
from django.urls import reverse
from instances.models import Instance
+from instances.utils import paths_used_on_other_computes
from libvirt import VIR_DOMAIN_UNDEFINE_KEEP_NVRAM, VIR_DOMAIN_UNDEFINE_NVRAM, libvirtError
from vrtManager.util import OperationError
@@ -97,6 +98,32 @@ class DestroyOrderTestCase(TestCase):
self.assertEqual(response.status_code, 302)
proxy.get_volume_by_path.assert_called_once_with("/pool/a.qcow2")
+ def test_the_files_are_checked_before_the_vm_is_stopped(self):
+ with patch("instances.models.wvmInstance") as wvm:
+ proxy = wvm.return_value
+ proxy.instance.isActive.return_value = True
+ proxy.split_disk_paths_by_use.return_value = (["/pool/own.qcow2"], [])
+ self.client.post(reverse("instances:destroy", args=[self.instance.id]), {"delete_disk": "true"})
+ calls = [name for name, *_ in proxy.mock_calls if name in ("split_disk_paths_by_use", "force_shutdown", "delete")]
+ self.assertEqual(calls, ["split_disk_paths_by_use", "force_shutdown", "delete"])
+
+ def test_a_host_that_cannot_be_checked_stops_nothing(self):
+ Compute.objects.create(name="other-host", hostname="192.0.2.50", login="u", password="p", type=1)
+ def connect(host, *args):
+ if host == "192.0.2.50":
+ raise libvirtError("unreachable")
+ return MagicMock()
+
+ with patch("instances.models.wvmInstance") as wvm, patch("instances.utils.wvmConnect", side_effect=connect):
+ proxy = wvm.return_value
+ proxy.instance.isActive.return_value = True
+ proxy.split_disk_paths_by_use.side_effect = lambda elsewhere: elsewhere(["/pool/own.qcow2"])
+ response = self.client.post(reverse("instances:destroy", args=[self.instance.id]), {"delete_disk": "true"})
+ proxy.force_shutdown.assert_not_called()
+ proxy.delete.assert_not_called()
+ self.assertTrue(Instance.objects.filter(pk=self.instance.pk).exists())
+ self.assertIn("The disk files cannot be checked on other-host", str(list(get_messages(response.wsgi_request))[-1]))
+
def test_disks_are_deleted_after_the_undefine(self):
proxy = self.destroy()
proxy.get_volume_by_path.assert_called_once_with("/pool/own.qcow2")
@@ -153,3 +180,46 @@ class DestroyOrderTestCase(TestCase):
wvm.return_value.get_status.return_value = 3 # paused
response = self.client.get(reverse("instances:destroy", args=[self.instance.id]))
self.assertIn('id="delete_form"', response.content.decode())
+
+
+class OtherComputesTestCase(TestCase):
+ """The files of a VM another host's VM or volume uses: shared storage
+ holds them at the same path on every host that mounts it."""
+
+ def setUp(self):
+ self.source = Compute.objects.create(name="source", hostname="192.0.2.60", login="u", password="p", type=1)
+ Compute.objects.create(name="host-b", hostname="192.0.2.61", login="u", password="p", type=1)
+ Compute.objects.create(name="host-c", hostname="192.0.2.62", login="u", password="p", type=1)
+
+ def test_the_paths_another_host_uses(self):
+ hosts = self.hosts({"192.0.2.61": {"/nfs/a.qcow2", "/nfs/unrelated.qcow2"}, "192.0.2.62": {"/nfs/b.qcow2"}})
+ own = ["/nfs/a.qcow2", "/nfs/b.qcow2", "/local/c.qcow2"]
+ with patch("instances.utils.wvmConnect", side_effect=lambda host, *args: hosts[host]):
+ self.assertEqual(paths_used_on_other_computes(self.source, own), {"/nfs/a.qcow2", "/nfs/b.qcow2"})
+ # the VM's own host is checked by split_disk_paths_by_use itself; on the
+ # others too, the backing files of the VM's own volumes do not count
+ hosts["192.0.2.60"].paths_in_use.assert_not_called()
+ hosts["192.0.2.61"].paths_in_use.assert_called_once_with(own=own)
+ hosts["192.0.2.62"].paths_in_use.assert_called_once_with(own=own)
+
+ def test_another_record_of_the_vm_s_own_host_is_skipped(self):
+ # there the VM itself uses its files: they would all be kept
+ hosts = self.hosts({"192.0.2.61": {"/pool/a.qcow2"}, "192.0.2.62": set()}, same_host="192.0.2.61")
+ with patch("instances.utils.wvmConnect", side_effect=lambda host, *args: hosts[host]):
+ self.assertEqual(paths_used_on_other_computes(self.source, ["/pool/a.qcow2"]), set())
+ hosts["192.0.2.61"].paths_in_use.assert_not_called()
+
+ def hosts(self, used, same_host=None):
+ """Connections by host name; same_host is the source host under another record."""
+ hosts = {"192.0.2.60": MagicMock(**{"host_uuid.return_value": "aaaaaaaa-0000-0000-0000-000000000060"})}
+ for n, (host, paths) in enumerate(used.items()):
+ uuid = "aaaaaaaa-0000-0000-0000-000000000060" if host == same_host else f"bbbbbbbb-0000-0000-0000-00000000000{n}"
+ hosts[host] = MagicMock(**{"host_uuid.return_value": uuid, "paths_in_use.return_value": paths})
+ return hosts
+
+ def test_a_host_that_cannot_be_read_is_an_error(self):
+ hosts = self.hosts({"192.0.2.61": set(), "192.0.2.62": set()})
+ hosts["192.0.2.61"].paths_in_use.side_effect = libvirtError("connection refused")
+ with patch("instances.utils.wvmConnect", side_effect=lambda host, *args: hosts[host]):
+ with self.assertRaisesMessage(OperationError, "The disk files cannot be checked on host-b"):
+ paths_used_on_other_computes(self.source, ["/nfs/a.qcow2"])
diff --git a/instances/utils.py b/instances/utils.py
index 293506bb..44371406 100644
--- a/instances/utils.py
+++ b/instances/utils.py
@@ -5,13 +5,16 @@ import string
from accounts.models import UserInstance, UserAttributes
from appsettings.settings import app_settings
+from computes.models import Compute
from django.conf import settings
from django.db import transaction
from django.utils.translation import gettext_lazy as _
from libvirt import libvirtError
from logs.audit import reason, write
from operations.models import Operation
+from vrtManager.connection import wvmConnect
from vrtManager.instance import wvmInstance, wvmInstances
+from vrtManager.util import OperationError
from .models import Instance
@@ -43,6 +46,25 @@ def get_clone_free_names(size=10):
return free_names
+def paths_used_on_other_computes(compute, own):
+ """The paths in own that the VMs and volumes of the panel's other computes
+ use (paths_in_use): shared storage holds a file at the same path on every
+ host that mounts it, as migration needs. OperationError naming a compute
+ that cannot be read: it might use them."""
+ used = set()
+ host = wvmConnect(compute.hostname, compute.login, compute.password, compute.type).host_uuid()
+ for other in Compute.objects.exclude(pk=compute.pk):
+ try:
+ conn = wvmConnect(other.hostname, other.login, other.password, other.type)
+ if host and conn.host_uuid() == host:
+ continue # the VM's own host under another record: its VM is the one going
+ used |= conn.paths_in_use(own=own)
+ except libvirtError as err:
+ raise OperationError(_("The disk files cannot be checked on %(host)s: %(err)s") % {
+ "host": other.name, "err": err.get_error_message()}) from err
+ return used & set(own)
+
+
def disk_gib(disks):
"""The size of the disks in whole GiB each, as the quota counts them."""
return sum(int(disk["size"] or 0) >> 30 for disk in disks)
diff --git a/instances/views.py b/instances/views.py
index a4b40a1c..6631f91e 100755
--- a/instances/views.py
+++ b/instances/views.py
@@ -803,15 +803,18 @@ def destroy(request, pk):
with libvirt_instance_lock(instance):
with audit_vm(request, instance, "delete"):
proxy = instance.proxy
+ # the files to delete are known before the VM is stopped: a
+ # host that cannot be checked refuses the destruction
+ to_delete, shared = [], []
+ if options.validated_data["delete_disk"]:
+ to_delete, shared = proxy.split_disk_paths_by_use(
+ lambda own: utils.paths_used_on_other_computes(instance.compute, own))
+
# Paused and other active states must stop too, or the VM keeps
# running as a transient domain after the undefine.
if proxy.instance.isActive():
proxy.force_shutdown()
- to_delete, shared = [], []
- if options.validated_data["delete_disk"]:
- to_delete, shared = proxy.split_disk_paths_by_use()
-
# Undefine before deleting any disk: if it fails, nothing is lost.
# Snapshot metadata goes with the domain; internal snapshot data
# stays in the disk images that are kept.
diff --git a/vrtManager/connection.py b/vrtManager/connection.py
index 42de52ad..2766e608 100644
--- a/vrtManager/connection.py
+++ b/vrtManager/connection.py
@@ -408,6 +408,10 @@ class wvmConnect(object):
"""The host's capabilities, read once per object (a request): most getters below parse them"""
return self.wvm.getCapabilities()
+ def host_uuid(self):
+ """The host's UUID (capabilities): compute records of one host share it"""
+ return util.get_xml_path(self.capabilities_xml, "/capabilities/host/uuid")
+
@cached_property
def host_info(self):
"""The host's info (architecture, memory in MiB, CPUs, ...), read once per object"""
diff --git a/vrtManager/instance.py b/vrtManager/instance.py
index 4ecd4c34..a48907b5 100644
--- a/vrtManager/instance.py
+++ b/vrtManager/instance.py
@@ -1691,18 +1691,25 @@ class wvmInstance(wvmConnect):
are this VM's: their backing files do not count."""
return self.paths_in_use(self.instance.UUIDString(), own)
- def split_disk_paths_by_use(self):
+ def split_disk_paths_by_use(self, elsewhere=None):
"""This VM's disk paths, split into (only this VM, also used by another
- domain). With external snapshots, the files below a disk that the
- snapshots were taken on, and their memory files, are the VM's too, and
- so is its cloud-init seed, in a CD-ROM drive or ejected."""
+ domain). Its disks are those it runs with and those of its next start.
+ With external snapshots, the files below a disk that the snapshots
+ were taken on, and their memory files, are the VM's too, and so is its
+ cloud-init seed, in a CD-ROM drive or ejected. elsewhere(own): the
+ paths of these that other hosts use."""
self.refresh_instance_pools()
- own = [disk["path"] for disk in self.get_disk_devices() if disk["path"]]
+ own = []
+ for disk in self.get_disk_devices() + self.get_disk_devices(config=True):
+ if disk["path"] and disk["path"] not in own:
+ own.append(disk["path"])
own += self._snapshot_chain_files(own)
seed = self._seed_path()
if seed:
own.append(seed)
used_elsewhere = self.paths_used_by_other_domains(own)
+ if elsewhere:
+ used_elsewhere |= elsewhere(own)
# a kept file keeps the files below it
for path in [path for path in own if path in used_elsewhere]:
while (path := self.backing_path(path)) and path not in used_elsewhere:
diff --git a/vrtManager/tests/test_cloud_init_seed.py b/vrtManager/tests/test_cloud_init_seed.py
index 75b5fb17..13fe68ba 100644
--- a/vrtManager/tests/test_cloud_init_seed.py
+++ b/vrtManager/tests/test_cloud_init_seed.py
@@ -88,7 +88,7 @@ class DeleteWithDisksTestCase(unittest.TestCase):
vm.wvm = MagicMock()
vm.refresh_instance_pools = MagicMock()
vm.get_uuid = lambda: UUID
- vm.get_disk_devices = lambda: [{"path": "/pool/web-1.qcow2"}]
+ vm.get_disk_devices = lambda config=False: [{"path": "/pool/web-1.qcow2"}]
vm._snapshot_chain_files = lambda own: []
vm.paths_used_by_other_domains = lambda own: set()
vm.backing_path = lambda path: None
@@ -112,6 +112,13 @@ class DeleteWithDisksTestCase(unittest.TestCase):
vm, _ = self.vm(seed_in_pool=False)
self.assertEqual(vm.split_disk_paths_by_use(), (["/pool/web-1.qcow2"], []))
+ def test_its_next_start_disks_and_the_files_other_hosts_use(self):
+ vm, _ = self.vm(seed_in_pool=False)
+ vm.get_disk_devices = lambda config=False: [{"path": "/pool/web-1.qcow2"}] + (
+ [{"path": "/nfs/pending.qcow2"}] if config else [])
+ self.assertEqual(vm.split_disk_paths_by_use(lambda own: {"/nfs/pending.qcow2"} & set(own)),
+ (["/pool/web-1.qcow2"], ["/nfs/pending.qcow2"]))
+
class CloneTestCase(unittest.TestCase):
def test_a_clone_does_not_take_the_vms_seed(self):
diff --git a/vrtManager/tests/test_external_snapshots.py b/vrtManager/tests/test_external_snapshots.py
index c985f6b0..ad106921 100644
--- a/vrtManager/tests/test_external_snapshots.py
+++ b/vrtManager/tests/test_external_snapshots.py
@@ -766,7 +766,7 @@ class DestroyFilesTestCase(unittest.TestCase):
"", f""
)
p = proxy(external=[snap for snap, _ in snaps], xml=self.TOP)
- p.get_disk_devices = lambda: [{"path": top}]
+ p.get_disk_devices = lambda config=False: [{"path": top}]
backing = {
"/pool/a.two": "/pool/a.one", "/pool/a.one": "/pool/a.qcow2", "/pool/a.qcow2": "/templates/base.qcow2",
}