diff --git a/README.md b/README.md index 86eef6c2..291e123f 100644 --- a/README.md +++ b/README.md @@ -276,7 +276,7 @@ A compute node is a KVM host the panel manages over libvirt. Firewall: the compute's VNC ports (`5900`–`65535`) must accept connections **only** from the panel's IP, never from public networks. -For VM migration between computes, define shared storage as a storage pool on every host (a VM migrates only when its disk, backing and ISO files and block devices are volumes of the destination's pools), and where the hosts cannot resolve each other's names, set each compute's **Migration address** on its edit page. +For VM migration between computes, define shared storage as a storage pool on every host (a VM migrates only when its disk, backing and ISO files and block devices are volumes of the destination's pools), and where the hosts cannot resolve each other's names, set each compute's **Migration address** on its edit page. A VM's disks can instead be copied into storage pools of the destination during the migration: after choosing the destination, the **Disks** list shows each disk file and the destination pools that can take it (directory, filesystem and NFS pools; one with a file of the same name refuses the copy), and a disk on storage both hosts share can also stay there. Plain raw or qcow2 disks without a backing file are copied; a shut-off VM is started paused for the copy, its CPUs never run, and is shut off on the destination. With **Delete the source files of copied disks** the source files go after the migration, unless another VM or volume uses them. The copy goes through the destination's migration port range (49152-49215 by default), like the memory. If libvirt warns `Host SMBIOS information is not available`, install `dmidecode` (`apt-get`, `dnf` or `zypper install -y dmidecode`) and restart `libvirtd`. diff --git a/computes/tests/test_migration_address.py b/computes/tests/test_migration_address.py index 22e5f910..3cb46450 100644 --- a/computes/tests/test_migration_address.py +++ b/computes/tests/test_migration_address.py @@ -53,5 +53,6 @@ class MigrationAddressTests(TestCase): patch("instances.utils.wvmInstances") as instances, \ patch("instances.utils.wvmInstance"), \ patch.object(Instance, "proxy", MagicMock()): + instances.return_value.moveto.return_value = ("live", []) migrate_instance(dest, vm, user, live=True) self.assertEqual(instances.return_value.moveto.call_args.kwargs["uri"], "tcp://198.51.100.9") diff --git a/instances/api/serializers.py b/instances/api/serializers.py index 00c2f0e5..fbcf6d99 100644 --- a/instances/api/serializers.py +++ b/instances/api/serializers.py @@ -117,6 +117,9 @@ class MigrateSerializer(serializers.ModelSerializer): autoconverge = serializers.BooleanField(default=True) compress = serializers.BooleanField(default=False) unsafe = serializers.BooleanField(default=False) + # {target: pool of the destination} of the disks copied there; delete_source deletes their source files + disk_pools = serializers.DictField(child=serializers.CharField(allow_blank=True), default=dict) + delete_source = serializers.BooleanField(default=False) submission_key = serializers.CharField(write_only=True) class Meta: @@ -129,5 +132,7 @@ class MigrateSerializer(serializers.ModelSerializer): "autoconverge", "compress", "unsafe", + "disk_pools", + "delete_source", "submission_key", ] diff --git a/instances/api/viewsets.py b/instances/api/viewsets.py index acdcf24a..6d1cb8ca 100644 --- a/instances/api/viewsets.py +++ b/instances/api/viewsets.py @@ -4,6 +4,7 @@ from computes.models import Compute from django.http import Http404 from django.shortcuts import get_object_or_404 from django.urls import reverse +from instances.jobs import MIGRATE_OPTIONS from instances.models import Flavor, Instance from instances.views import destroy as instance_destroy from instances.views import ( @@ -195,8 +196,8 @@ class MigrateViewSet(viewsets.ViewSet): if not serializer.is_valid(): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) data = serializer.validated_data - raw = {"instance": data["instance"].pk, "target": data["target_compute"].pk, - **{name: data.get(name, False) for name in ("live", "unsafe", "offline", "autoconverge", "compress")}} + raw = {"instance": data["instance"].pk, "target": data["target_compute"].pk, "disk_pools": data["disk_pools"], + **{name: data.get(name, False) for name in MIGRATE_OPTIONS}} op = jobs.submit("instance.migrate", request.user, raw, data["submission_key"]) return Response(OperationSerializer(op, context={"request": request}).data, status=status.HTTP_202_ACCEPTED, headers={"Location": reverse("operation-detail", args=[op.pk])}) diff --git a/instances/jobs.py b/instances/jobs.py index 21d2df8b..464f0177 100644 --- a/instances/jobs.py +++ b/instances/jobs.py @@ -1,6 +1,7 @@ """Jobs of the instances app (operations framework, spec section 5).""" import math +import os import re import time @@ -809,7 +810,7 @@ class CreateInstance(Kind): return _log(op, state, "create") -MIGRATE_OPTIONS = ("live", "unsafe", "offline", "autoconverge", "compress") +MIGRATE_OPTIONS = ("live", "unsafe", "offline", "autoconverge", "compress", "delete_source") def _target(instance, target_pk): @@ -841,7 +842,9 @@ class MigrateInstance(Kind): progress_bytes = True # libvirt job data processed/total def normalize(self, raw): + # disk_pools: {target: pool of the destination} of the disks to copy there return {"instance": int(raw["instance"]), "target": int(raw["target"]), + "disk_pools": {str(k): str(v) for k, v in (raw.get("disk_pools") or {}).items() if v}, **{k: bool(raw.get(k)) for k in MIGRATE_OPTIONS}} def validate(self, user, params): @@ -855,6 +858,13 @@ class MigrateInstance(Kind): keys.update({claims.compute_key(instance.compute_id): Claim.SHARED, claims.reconcile_key(instance.compute_id): Claim.EXCLUSIVE, claims.instance_key(instance.uuid): Claim.EXCLUSIVE}) + if params["disk_pools"]: + # a copy takes its file's name in the chosen pool; the source file may go + for disk in instance.proxy.get_disk_devices(): + if pool := params["disk_pools"].get(disk["dev"]): + keys[claims.volume_key(params["target"], pool, os.path.basename(disk["path"]))] = Claim.EXCLUSIVE + if disk["storage"]: + keys[claims.volume_key(instance.compute_id, disk["storage"], disk["image"])] = Claim.EXCLUSIVE return Plan(claims=keys, resource=claims.instance_key(instance.uuid), label=f"{instance.compute.name}/{instance.name}", intent={"compute": instance.compute_id, "uuid": instance.uuid}) @@ -875,6 +885,7 @@ class MigrateInstance(Kind): try: # migrate_instance logs the outcome utils.migrate_instance(Compute.objects.get(pk=p["target"]), instance, op.user, + disk_pools=p["disk_pools"], **{k: p[k] for k in MIGRATE_OPTIONS}) except libvirtError: op.refresh_from_db(fields=["cancel_requested"]) diff --git a/instances/templates/instance.html b/instances/templates/instance.html index e4c3088f..615bf795 100644 --- a/instances/templates/instance.html +++ b/instances/templates/instance.html @@ -444,6 +444,40 @@ }); }); } + // the Migrate tab's disks: loaded for the chosen destination when the tab opens or the choice changes + // the form is not sent until the disks of the chosen destination are shown + var migratePlan = document.getElementById("migrate-plan"); + var migrateCompute = document.getElementById("migrate_compute"); + var migrateLoading = null; + function loadMigratePlan() { + if (!migrateCompute.value) return; + if (migrateLoading) migrateLoading.abort(); + var loading = migrateLoading = new AbortController(); + var submit = migratePlan.closest("form").querySelector('button[name="migrate"]'); + submit.disabled = true; + migratePlan.textContent = "{{ _("Checking the disks…")|escapejs }}"; + fetch(migratePlan.dataset.url + "?compute_id=" + encodeURIComponent(migrateCompute.value), + {signal: loading.signal}).then(function (response) { + if (!response.ok) throw new Error(response.statusText); + return response.text(); + }).then(function (html) { + migratePlan.innerHTML = html; + submit.disabled = false; + }).catch(function () { + if (loading.signal.aborted) return; // another destination was chosen + migratePlan.textContent = "{{ _("The disks could not be checked.")|escapejs }}"; + submit.disabled = false; // the migration checks them again + }); + } + if (migratePlan && migrateCompute) { + document.querySelector('[data-bs-target="#migrate"]').addEventListener("shown.bs.tab", function () { + if (!migratePlan.dataset.loaded) { + migratePlan.dataset.loaded = "true"; + loadMigratePlan(); + } + }); + migrateCompute.addEventListener("change", loadMigratePlan); + } function get_osinfo() { show_tab('#settings'); show_tab('#osinfo'); diff --git a/instances/templates/instances/migrate_plan.html b/instances/templates/instances/migrate_plan.html new file mode 100644 index 00000000..ba2e90a8 --- /dev/null +++ b/instances/templates/instances/migrate_plan.html @@ -0,0 +1,43 @@ +{% load i18n %} +{# the disks of the Migrate tab for the chosen destination: where each goes #} +{% if error %} +
{% blocktrans %}The disks could not be checked: {{ error }}{% endblocktrans %}
+{% elif not disks %} +

{% trans "The VM has no disk files: its disks stay where they are." %}

+{% else %} +
+ + + + + + {% for disk in disks %} + + + + + + {% endfor %} + +
{% trans "Disk" %}{% trans "Size" %}{% trans "On the destination" %}
{{ disk.target }}
{{ disk.path }}
{{ disk.capacity|filesizeformat }} + {% if disk.shared or not disk.refusal and pools %} + + {% elif disk.refusal %} + {% blocktrans with reason=disk.refusal %}Cannot be copied: {{ reason }}{% endblocktrans %} + {% else %} + {% trans "No pool of the destination host can take it" %} + {% endif %} +
+
+

+ {% trans "A copy is made during the migration and takes as long as the disk's data needs. Disks on network storage stay where they are." %} + {% if shut_off %}{% trans "A shut-off VM is started paused for a copy, its CPUs never run, and is shut off on the destination." %}{% endif %} +

+{% endif %} diff --git a/instances/templates/instances/settings_tab.html b/instances/templates/instances/settings_tab.html index 18edc643..4d14e30f 100644 --- a/instances/templates/instances/settings_tab.html +++ b/instances/templates/instances/settings_tab.html @@ -697,7 +697,7 @@
- {% if computes_count != 1 %} {% for comp in computes %} {% if comp.id != compute.id %} @@ -708,6 +708,10 @@
+
+ +
+
@@ -748,6 +752,14 @@
+
+
+
+ + +
+
+
{% if computes_count != 1 %} {% else %} diff --git a/instances/tests/test_live_jobs.py b/instances/tests/test_live_jobs.py index 57fc0f3d..aee96c39 100644 --- a/instances/tests/test_live_jobs.py +++ b/instances/tests/test_live_jobs.py @@ -172,6 +172,31 @@ class LiveJobsTestCase(TransactionTestCase): self.assertFalse(self.conn.lookupByUUIDString(uuid).isActive()) # defined on the source again self.assertEqual(Instance.objects.get(uuid=uuid).compute, self.compute) + def test_a_disk_neither_host_shares_is_copied_there_and_back(self): + """The test pool is a directory of each host's own: the disk is + copied into it, running there with its source file deleted on + request, shut off back (started paused for the copy) with it kept.""" + dest, dest_conn = self.second_host() + self.addCleanup(livetest.remove_pool, dest_conn) + livetest.ensure_pool(dest_conn) + path = livetest.create_volume(self.conn, P + "copy") + dom = livetest.define_vm(self.conn, P + "copy", [path]) + dom.create() + uuid = dom.UUIDString() + instance = Instance.objects.create(compute=self.compute, name=dom.name(), uuid=uuid) + pools = {"vda": livetest.POOL} + there = {"instance": instance.pk, "target": dest.pk, "live": True, "disk_pools": pools, "delete_source": True} + self.run_job(jobs.submit("instance.migrate", self.admin, there, "key-copy0001")) + self.assertTrue(dest_conn.lookupByUUIDString(uuid).isActive()) + self.assertEqual((livetest.volume_exists(self.conn, path), livetest.volume_exists(dest_conn, path)), + (False, True)) + dest_conn.lookupByUUIDString(uuid).destroy() + back = {"instance": instance.pk, "target": self.compute.pk, "offline": True, "disk_pools": pools} + self.run_job(jobs.submit("instance.migrate", self.admin, back, "key-copy0002")) + self.assertFalse(self.conn.lookupByUUIDString(uuid).isActive()) + self.assertEqual((livetest.volume_exists(self.conn, path), livetest.volume_exists(dest_conn, path)), + (True, True)) + def test_a_shut_off_uefi_vm_migrates_with_its_nvram_there_and_back(self): dest, _dest_conn = self.second_host() hosts = {self.compute.pk: wvmConnect(*self.args), dest.pk: wvmConnect(dest.hostname, dest.login, "", 2)} diff --git a/instances/tests/test_migrate_job.py b/instances/tests/test_migrate_job.py index ea2c2aba..a86f338d 100644 --- a/instances/tests/test_migrate_job.py +++ b/instances/tests/test_migrate_job.py @@ -31,7 +31,7 @@ class MigrateJobTestCase(TransactionTestCase): patch.object(Compute, "status", True).start() patch.object(Instance, "proxy", proxy).start() self.dest_conn = patch("instances.utils.wvmInstances").start().return_value - self.dest_conn.moveto.return_value = "live" + self.dest_conn.moveto.return_value = ("live", []) self.addCleanup(patch.stopall) def queue(self, **form): @@ -76,6 +76,41 @@ class MigrateJobTestCase(TransactionTestCase): self.vm.refresh_from_db() self.assertEqual(self.vm.compute, self.dest) + def test_a_copy_claims_its_volume_on_both_computes(self): + Instance.proxy.get_disk_devices.return_value = [ + {"dev": "vda", "storage": "default", "image": "mig-vm.qcow2", "path": "/i/mig-vm.qcow2"}, + {"dev": "vdb", "storage": None, "image": "/dev/sdb", "path": "/dev/sdb"}] + op = self.queue(live_migrate="true", delete_source="true", **{"pool-vda": "fast", "pool-vdb": ""}) + self.assertTrue({(f"volume:{self.source.pk}:default/mig-vm.qcow2", "exclusive"), + (f"volume:{self.dest.pk}:fast/mig-vm.qcow2", "exclusive")} + <= set(Claim.objects.values_list("resource", "mode"))) + self.assertEqual((op.params["disk_pools"], op.params["delete_source"]), ({"vda": "fast"}, True)) + ctx = self.start(op) + get_kind(op.kind).run(op, ctx) + self.assertEqual(self.dest_conn.moveto.call_args.kwargs["pools"], {"vda": "fast"}) + + def test_the_migrate_tab_lists_each_disk_with_where_it_can_go(self): + Instance.proxy.instance.XMLDesc.return_value = "" + Instance.proxy.get_status.return_value = 5 + with patch("instances.views.wvmInstances") as instances: + dest = instances.return_value + fast, default = MagicMock(), MagicMock() + fast.info.return_value = default.info.return_value = [2, 0, 0, 8 << 30] + dest.file_pools.return_value = {"fast": (fast, "/srv/fast"), "default": (default, "/i")} + dest.disk_plan.return_value = [ + {"target": "vda", "path": "/i/mig-vm.qcow2", "capacity": 1 << 30, "shared": False, "refusal": None}, + {"target": "sda", "path": "/i/os.iso", "capacity": 1 << 20, "shared": False, + "refusal": "an image is not copied; eject it first"}] + page = self.client.get(reverse("instances:migrate_plan", args=[self.vm.pk]), + {"compute_id": self.dest.pk}).content.decode() + dest.close.assert_called_once_with() + # the pool on the disk's directory is chosen + self.assertIn('', page) + self.assertIn('', page) + self.assertIn("Cannot be copied: an image is not copied; eject it first", page) + self.assertIn("A shut-off VM is started paused for a copy", page) + self.assertEqual(self.client.get(reverse("instances:migrate_plan", args=[self.vm.pk])).status_code, 404) + def test_a_cancelled_migration_ends_cancelled_and_the_vm_stays(self): op = self.queue(live_migrate="true") ctx = self.start(op) diff --git a/instances/tests/test_migration_bookkeeping.py b/instances/tests/test_migration_bookkeeping.py index 18ca9f1a..5d24cc70 100644 --- a/instances/tests/test_migration_bookkeeping.py +++ b/instances/tests/test_migration_bookkeeping.py @@ -29,7 +29,7 @@ class MigrationBookkeepingTests(TestCase): patch("instances.utils.wvmInstances") as instances, \ patch("instances.utils.wvmInstance") as instance, \ patch.object(Instance, "proxy", proxy): - instances.return_value.moveto.return_value = "live" + instances.return_value.moveto.return_value = ("live", []) if autostart_error: instance.return_value.set_autostart.side_effect = autostart_error self.moveto = instances.return_value.moveto diff --git a/instances/tests/test_migration_log.py b/instances/tests/test_migration_log.py index 036d4ffd..9bc44a97 100644 --- a/instances/tests/test_migration_log.py +++ b/instances/tests/test_migration_log.py @@ -26,7 +26,7 @@ class MigrationLogTests(TestCase): self.dest = Compute.objects.create(name="ml-dst", hostname="198.51.100.9", login="root", password="", type=2) self.vm = Instance.objects.create(compute=self.source, name="vm", uuid="u-ml") - def migrate(self, outcome, nvram=None, remove_nvram=True, **request): + def migrate(self, outcome, nvram=None, remove_nvram=True, copied=(), delete_fails=None, in_use=(), **request): self.proxy = MagicMock() self.proxy.get_autostart.return_value = False self.proxy.get_nvram.return_value = nvram @@ -34,6 +34,9 @@ class MigrationLogTests(TestCase): self.proxy.remove_nvram.side_effect = remove_nvram else: self.proxy.remove_nvram.return_value = remove_nvram + self.proxy.paths_in_use.return_value = set(in_use) + if delete_fails: + self.proxy.get_volume_by_path.return_value.delete.side_effect = delete_fails with patch.object(Compute, "status", True), \ patch("instances.utils.wvmInstances") as instances, \ patch("instances.utils.wvmInstance"), \ @@ -42,7 +45,7 @@ class MigrationLogTests(TestCase): if isinstance(outcome, Exception): instances.return_value.moveto.side_effect = outcome else: - instances.return_value.moveto.return_value = outcome + instances.return_value.moveto.return_value = (outcome, list(copied)) return migrate_instance(self.dest, self.vm, self.admin, **request) def test_the_nvram_file_left_on_the_source_is_removed(self): @@ -100,3 +103,37 @@ class MigrationLogTests(TestCase): with self.assertRaisesRegex(util.OperationError, "A shut-off VM cannot be migrated live"): self.migrate(util.OperationError("A shut-off VM cannot be migrated live"), live=True) self.assertEqual(rows(), [("VM vm: migrate to 198.51.100.9: A shut-off VM cannot be migrated live", False)]) + + def test_the_source_copies_stay_unless_deleted(self): + disks = ["/var/lib/libvirt/images/vm.qcow2", "/var/lib/libvirt/images/vm-data.qcow2"] + self.migrate("live, disks copied: vda to default, vdb to default", copied=disks, live=True) + self.assertFalse(self.proxy.get_volume_by_path.called) + self.assertEqual(rows(), [ + ("VM vm: migrate (live, disks copied: vda to default, vdb to default) to 198.51.100.9", True), + ("VM vm: keep the source copy /var/lib/libvirt/images/vm.qcow2", True), + ("VM vm: keep the source copy /var/lib/libvirt/images/vm-data.qcow2", True), + ]) + + def test_deleted_source_copies_and_a_failed_delete_are_logged(self): + self.migrate("live, disks copied: vda", copied=["/var/lib/libvirt/images/vm.qcow2"], live=True, + delete_source=True) + self.proxy.get_volume_by_path.assert_called_once_with("/var/lib/libvirt/images/vm.qcow2") + self.assertEqual(rows()[-1], ("VM vm: delete the source copy /var/lib/libvirt/images/vm.qcow2", True)) + self.vm.compute = self.source + self.vm.save() + self.migrate("live, disks copied: vda", copied=["/var/lib/libvirt/images/vm.qcow2"], live=True, + delete_source=True, delete_fails=libvirtError("cannot delete")) + self.assertEqual(rows()[-1], ("VM vm: delete the source copy /var/lib/libvirt/images/vm.qcow2: cannot delete", + False)) + self.vm.refresh_from_db() + self.assertEqual(self.vm.compute, self.dest) + + def test_a_source_copy_another_vm_or_volume_uses_stays(self): + disks = ["/var/lib/libvirt/images/vm.qcow2", "/var/lib/libvirt/images/base.qcow2"] + self.migrate("live, disks copied: vda, vdb", copied=disks, in_use={disks[1]}, live=True, + delete_source=True) + self.proxy.get_volume_by_path.assert_called_once_with(disks[0]) + self.assertEqual(rows()[-2:], [ + ("VM vm: delete the source copy /var/lib/libvirt/images/vm.qcow2", True), + ("VM vm: keep the source copy /var/lib/libvirt/images/base.qcow2: another VM or volume uses it", True), + ]) diff --git a/instances/tests/test_xss.py b/instances/tests/test_xss.py index 66608c5a..73fb454f 100644 --- a/instances/tests/test_xss.py +++ b/instances/tests/test_xss.py @@ -138,6 +138,7 @@ ALLOWED_HTML_SINKS = { ("accounts/templates/accounts/otp_login.html", "$btn.html("): "static spinner markup", ("instances/templates/instance.html", "//sto_input.innerHTML = pool;"): "comment", ("instances/templates/instance.html", "hostDevices.innerHTML = html;"): "the panel's own fragment, host_devices_form.html, escaped by Django", + ("instances/templates/instance.html", "migratePlan.innerHTML = html;"): "the panel's own fragment, migrate_plan.html, escaped by Django", } diff --git a/instances/urls.py b/instances/urls.py index 6292c4f3..3ddb27a0 100755 --- a/instances/urls.py +++ b/instances/urls.py @@ -58,6 +58,7 @@ urlpatterns = [ path("/add_network/", views.add_network, name="add_network"), path("/delete_network/", views.delete_network, name="delete_network"), path("/host_devices/", views.host_devices, name="host_devices"), + path("/migrate_plan/", views.migrate_plan, name="migrate_plan"), path("/add_usb_hostdev/", views.add_usb_hostdev, name="add_usb_hostdev"), path("/delete_usb_hostdev/", views.delete_usb_hostdev, name="delete_usb_hostdev"), path("/add_pci_hostdev/", views.add_pci_hostdev, name="add_pci_hostdev"), diff --git a/instances/utils.py b/instances/utils.py index d0ab61dd..9a7489b6 100644 --- a/instances/utils.py +++ b/instances/utils.py @@ -158,10 +158,15 @@ def migrate_instance( offline=False, autoconverge=False, compress=False, + delete_source=False, + disk_pools=None, ): """Migrates instance to new_compute and logs the outcome (the mode used, or the error), then each later step that failed in a row of its - own. Returns the mode.""" + own. disk_pools: {target: pool} of the disks copied into pools of + new_compute (moveto's pools); their source files go with delete_source + unless another VM or volume uses them, and are named in the log + otherwise. Returns the mode.""" source, target = instance.compute, migration_target(new_compute) def log(action, error=None): @@ -179,7 +184,7 @@ def migrate_instance( autostart = instance.proxy.get_autostart() nvram = instance.proxy.get_nvram() - mode = conn_migrate.moveto( + mode, copied = conn_migrate.moveto( instance.proxy, instance.name, live, @@ -188,6 +193,7 @@ def migrate_instance( autoconverge, compress, uri=new_compute.migration_uri, + pools=disk_pools, ) except Exception as error: log(f"migrate to {target}", error) @@ -217,6 +223,24 @@ def migrate_instance( if conn_new is not None: conn_new.close() + in_use = set() + if copied and delete_source: + try: + in_use = instance.proxy.paths_in_use() + except libvirtError as error: + log("check the source copies before deleting them", error) + delete_source = False + for path in copied: + if not delete_source or path in in_use: + log(f"keep the source copy {path}" + (": another VM or volume uses it" if path in in_use else "")) + continue + try: + instance.proxy.get_volume_by_path(path).delete(0) + except libvirtError as error: + log(f"delete the source copy {path}", error) + else: + log(f"delete the source copy {path}") + # libvirt keeps the NVRAM file on the source; a VM created there # later with the same name would start with these variables. if nvram: diff --git a/instances/views.py b/instances/views.py index d11b211b..aa771601 100755 --- a/instances/views.py +++ b/instances/views.py @@ -49,7 +49,7 @@ from operations.utils import busy_message, summary from webvirtcloud.middleware import error_text, same_origin_referer from vrtManager import util from vrtManager.create import cpu_choices, wvmCreate -from vrtManager.instance import USAGE_MAX_WINDOW +from vrtManager.instance import USAGE_MAX_WINDOW, wvmInstances from vrtManager.interface import wvmInterface from vrtManager.nodedevs import device_names, hostdev_problems, node_devices, pci_problems from vrtManager.storage import wvmStorage @@ -850,7 +850,10 @@ def migrate(request, pk): target_host = get_object_or_404(Compute, pk=compute_id) raw = {"instance": pk, "target": target_host.pk, "live": request.POST.get("live_migrate"), "unsafe": request.POST.get("unsafe_migrate"), "offline": request.POST.get("offline_migrate"), - "autoconverge": request.POST.get("autoconverge"), "compress": request.POST.get("compress")} + "autoconverge": request.POST.get("autoconverge"), "compress": request.POST.get("compress"), + "delete_source": request.POST.get("delete_source"), + "disk_pools": {key.removeprefix("pool-"): value for key, value in request.POST.items() + if key.startswith("pool-")}} try: op = jobs.submit("instance.migrate", request.user, raw, request.POST.get("submission_key", "")) except claims.Busy as busy: @@ -863,6 +866,36 @@ def migrate(request, pk): return get_safe_redirect(request, default=reverse("instances:instance", args=[pk])) +@superuser_only +def migrate_plan(request, pk): + """The disks of the Migrate tab for the chosen destination, loaded when + one is chosen: each with the pools of the destination it can be copied + into, or the storage both hosts share.""" + instance = get_instance(request.user, pk) + compute_id = request.GET.get("compute_id", "") + if not compute_id.isdigit(): + raise Http404 + target = get_object_or_404(Compute, pk=compute_id) + dest = None + try: + dest = wvmInstances(target.hostname, target.login, target.password, target.type) + pools = [{"name": name, "directory": directory, "free": pool.info()[3]} + for name, (pool, directory) in sorted(dest.file_pools().items())] + disks = dest.disk_plan(instance.proxy, instance.proxy.instance.XMLDesc(0)) + for disk in disks: + # a copy goes by default to the pool on the same directory, else the first + names = [pool["name"] for pool in pools if pool["directory"] == os.path.dirname(disk["path"])] + names = names or [pool["name"] for pool in pools] + disk["default"] = names[0] if names else None + context = {"disks": disks, "pools": pools, "shut_off": instance.proxy.get_status() == 5} + except libvirtError as err: + context = {"error": error_text(request, err)} + finally: + if dest is not None: + dest.close() + return render(request, "instances/migrate_plan.html", context) + + GSTFSD_PORT = 16510 GSTFSD_TIMEOUT = 120 # gstfsd answers within its 100 s limit for a request (GSTFSD_DEADLINE) diff --git a/vrtManager/connection.py b/vrtManager/connection.py index 5d62439e..0c206e84 100644 --- a/vrtManager/connection.py +++ b/vrtManager/connection.py @@ -880,8 +880,11 @@ class wvmConnect(object): if os.path.dirname(path) != NVRAM_DIR: return False with self._nvram_pool() as pool, destination._nvram_pool() as other: - if self._nvram_shared(pool, other): - return False + try: + if self.same_directory(pool, other): + return False + except libvirtError: + return False # not known to be the source's own file vol = self._find_volume(pool, os.path.basename(path)) if vol is None: return False @@ -889,11 +892,11 @@ class wvmConnect(object): return True @staticmethod - def _nvram_shared(pool, other): - """Whether pools on NVRAM_DIR of two hosts are one directory, or other - is not known to be one of its own: creating the same file in both - fails then. The create is exclusive on the file server, unlike a - directory listing, which an NFS client may cache.""" + def same_directory(pool, other): + """Whether pools of two hosts are one directory: creating the same + file in both fails then, as it exists already. The create is + exclusive on the file server, unlike a directory listing, which an + NFS client may cache. Any other failure raises: it tells nothing.""" marker = ( f"webvirtcloud-check-{uuid.uuid4().hex}0" "" @@ -901,22 +904,32 @@ class wvmConnect(object): mine = pool.createXML(marker, 0) try: other.createXML(marker, 0).delete(0) - except libvirtError: - return True + except libvirtError as error: + # the file exists (OPERATION_INVALID), or the pool lists it already + if error.get_error_code() in (libvirt.VIR_ERR_OPERATION_INVALID, libvirt.VIR_ERR_STORAGE_VOL_EXIST): + return True + raise finally: mine.delete(0) return False + def pool_at(self, directory): + """The active pool whose directory is directory, or None. A pool + without a target path (rbd, gluster, ...) holds no files.""" + for pool in self.wvm.listAllStoragePools(libvirt.VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE): + if (util.get_xml_path(pool.XMLDesc(0), "/pool/target/path") or "").rstrip("/") == directory: + return pool + return None + @contextlib.contextmanager def _nvram_pool(self): """An active, refreshed pool on NVRAM_DIR: libvirt reads and deletes files only as volumes of a pool. A transient one when none exists.""" - for pool in self.wvm.listAllStoragePools(libvirt.VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE): - # a pool without a target path (rbd, gluster, ...) holds no files - if (util.get_xml_path(pool.XMLDesc(0), "/pool/target/path") or "").rstrip("/") == NVRAM_DIR: - pool.refresh(0) - yield pool - return + pool = self.pool_at(NVRAM_DIR) + if pool is not None: + pool.refresh(0) + yield pool + return pool = self.wvm.storagePoolCreateXML( f"webvirtcloud-nvram-{uuid.uuid4().hex[:8]}" f"{NVRAM_DIR}", diff --git a/vrtManager/instance.py b/vrtManager/instance.py index 6b54a08d..195a002b 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -17,6 +17,7 @@ try: VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP, VIR_DOMAIN_RUNNING, VIR_DOMAIN_SHUTOFF, + VIR_DOMAIN_START_PAUSED, VIR_DOMAIN_XML_SECURE, VIR_DOMAIN_XML_INACTIVE, VIR_DOMAIN_XML_MIGRATABLE, @@ -30,7 +31,12 @@ try: VIR_MIGRATE_AUTO_CONVERGE, VIR_MIGRATE_COMPRESSED, VIR_MIGRATE_LIVE, + VIR_MIGRATE_NON_SHARED_DISK, VIR_MIGRATE_OFFLINE, + VIR_MIGRATE_PARAM_DEST_XML, + VIR_MIGRATE_PARAM_MIGRATE_DISKS, + VIR_MIGRATE_PARAM_PERSIST_XML, + VIR_MIGRATE_PARAM_URI, VIR_MIGRATE_PERSIST_DEST, VIR_MIGRATE_UNDEFINE_SOURCE, VIR_MIGRATE_UNSAFE, @@ -99,7 +105,8 @@ def _rate(before, after, seconds, scale=1): class wvmInstances(wvmConnect): - def moveto(self, conn, name, live, unsafe, offline, autoconverge=False, compress=False, uri=None): + def moveto(self, conn, name, live, unsafe, offline, autoconverge=False, compress=False, uri=None, + pools=None): """Migrates the VM of conn (a wvmInstance on the source) here. The mode follows the VM's state, read once: a running VM migrates @@ -108,8 +115,10 @@ class wvmInstances(wvmConnect): fit the state is refused. unsafe, autoconverge (live only) and compress apply to a running VM. uri: the native migration URI (tcp://address) on the destination; None lets libvirt use the - destination's own hostname. Returns the mode and options used, for - the log.""" + destination's own hostname. pools: {target: pool here} of the disks + libvirt copies into those pools during the migration (disk_copies); + the other disks must be on storage both hosts share. Returns the mode + and options used, for the log, and the paths of the copied disks.""" state = conn.get_status() if state == 5: if live: @@ -146,13 +155,17 @@ class wvmInstances(wvmConnect): raise util.OperationError("A VM with snapshots cannot be migrated") if state == 5: self.refuse_local_state(dom.XMLDesc(0)) - definitions = {dom.XMLDesc(0)} + # the definition the migration carries first: it has the disks a migration copies + definitions = [dom.XMLDesc(0)] if state != 5: - definitions.add(dom.XMLDesc(VIR_DOMAIN_XML_INACTIVE)) + definitions.append(dom.XMLDesc(VIR_DOMAIN_XML_INACTIVE)) # a host device is of the source host: the destination may have none, or another one if any(etree.fromstring(xml).xpath(HOST_DEVICES) for xml in definitions): raise util.OperationError("A VM with host devices cannot be migrated; remove them first") - missing = self.missing_disks(definitions) + copies = self.disk_copies(conn, definitions, pools or {}) + if copies: + mode.append("disks copied: " + ", ".join(f"{copy['target']} to {copy['pool'].name()}" for copy in copies)) + missing = self.missing_disks(definitions, copied={copy["path"] for copy in copies}) if missing: raise util.OperationError( "The destination host does not have these disks of the VM: " @@ -167,7 +180,11 @@ class wvmInstances(wvmConnect): "Destination host emulator is different. Cannot be migrated" ) - if state == 5: + if copies: + note = self.migrate_copying(conn, state, flags, uri, copies) + if note: + mode.append(note) + elif state == 5: # Workaround for a libvirt bug (RHEL-156800), see move_definition. # Once supported hosts all run a fixed libvirt, this branch and # move_definition can go: the flags above are those for @@ -175,7 +192,185 @@ class wvmInstances(wvmConnect): self.move_definition(dom, conn) else: dom.migrate(self.wvm, flags, None, uri, 0) - return ", ".join(mode) + return ", ".join(mode), [copy["path"] for copy in copies] + + def migrate_copying(self, conn, state, flags, uri, copies): + """Migrates the VM of conn while libvirt copies the disks of copies + into their pools here (NON_SHARED_DISK), the definitions naming the + new paths. libvirt copies the disks of a running or paused VM only: a + shut-off VM is started paused for it, its CPUs never run, and is shut + off here after it; its UEFI variables are copied first, as this host + would start from the firmware's template. Returns a note for the log + when the VM could not be shut off here.""" + dom = conn.instance + nvram = None + xml_flags = VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_MIGRATABLE + params = { + VIR_MIGRATE_PARAM_MIGRATE_DISKS: [copy["target"] for copy in copies], + VIR_MIGRATE_PARAM_DEST_XML: self.moved_disks(dom.XMLDesc(xml_flags), copies), + VIR_MIGRATE_PARAM_PERSIST_XML: self.moved_disks(dom.XMLDesc(xml_flags | VIR_DOMAIN_XML_INACTIVE), copies), + } + if uri: + params[VIR_MIGRATE_PARAM_URI] = uri + try: + if state == 5: + flags &= ~VIR_MIGRATE_OFFLINE + nvram = self.copy_nvram(conn) + dom.createWithFlags(VIR_DOMAIN_START_PAUSED) + dom.migrate3(self.wvm, params, flags | VIR_MIGRATE_NON_SHARED_DISK) + except libvirtError as error: + if state == 5: + with contextlib.suppress(libvirtError): + dom.destroy() # shut off on the source again + self.remove_partial_copies(dom.UUIDString(), copies, error, nvram) + raise + if state == 5: + try: + self.wvm.lookupByUUIDString(dom.UUIDString()).destroy() + except libvirtError as error: + return f"left paused here, shut it off: {error}" + return None + + @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.""" + 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"])] + for source, dest in sources: + source.set("file", dest) + return etree.tostring(tree, encoding="unicode") + + def disk_plan(self, conn, xml): + """How the file disks of the VM of conn (on the source) in xml, the + definition the migration carries, can reach this host: a list of + {target, path, capacity, shared, refusal}. shared: both hosts see the + file at its path, as the directory both have as a storage pool is + one (same_directory); False when not, or when it cannot be told. + refusal: why the disk cannot be copied, or None.""" + rows, shared = [], {} + for disk in ElementTree.fromstring(xml).iter("disk"): + source = disk.find("source") + if disk.get("type") != "file" or source is None or not source.get("file"): + continue + path = source.get("file") + directory = os.path.dirname(path) + if directory not in shared: + here, there = self.pool_at(directory), conn.pool_at(directory) + try: + shared[directory] = here is not None and there is not None and self.same_directory(there, here) + except libvirtError: + shared[directory] = False # not known to be shared: copied, never assumed + try: + volume = conn.wvm.storageVolLookupByPath(path) + except libvirtError: + volume = None + rows.append({"target": disk.find("target").get("dev"), "path": path, "shared": shared[directory], + "capacity": volume.info()[1] if volume else None, + "refusal": self.copy_refusal(disk, volume)}) + return rows + + @staticmethod + def copy_refusal(disk, volume): + """Why the disk (its volume on the source, None if no pool has it) cannot be copied, or None.""" + if disk.get("device") != "disk": + return "an image is not copied; eject it first" + if disk.find("readonly") is not None or disk.find("encryption") is not None \ + or disk.find("source/encryption") is not None: + return "a read-only or encrypted disk is not copied" + driver = disk.find("driver") + if driver is None or driver.get("type") not in ("raw", "qcow2"): + return "only raw and qcow2 disks are copied" + if volume is None: + return "no storage pool of the source host has it" + if ElementTree.fromstring(volume.XMLDesc(0)).findtext("backingStore/path"): + return "a disk with a backing file is not copied" + return None + + def file_pools(self): + """The active pools of this host that keep disks as files, a copy's + destination: {name: (pool, directory)}.""" + pools = {} + for pool in self.wvm.listAllStoragePools(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE): + tree = ElementTree.fromstring(pool.XMLDesc(0)) + if tree.get("type") in ("dir", "fs", "netfs") and tree.findtext("target/path"): + pools[pool.name()] = (pool, tree.findtext("target/path").rstrip("/")) + return pools + + def disk_copies(self, conn, definitions, pools): + """The copies of the disks pools names a pool here for: {target, path, + dest, pool}, from the first of definitions (XML, the one the migration + carries). Each other file disk (see disk_plan), also of the next + start's definition, must be on shared storage. A copy never replaces a file: one + of the same name in the pool refuses it (libvirt would write into it). + Every problem is named in one refusal.""" + here = self.file_pools() if pools else {} + copies, problems, targets, carried = [], [], set(), set() + for row in self.disk_plan(conn, definitions[0]): + targets.add(row["target"]) + carried.add(row["path"]) + name = pools.get(row["target"]) + label = f"{row['target']} ({row['path']})" + if not name: + if not row["shared"]: + problems.append(f"{label}: the destination host does not share it; choose a pool to copy it to") + continue + if row["refusal"]: + problems.append(f"{label}: {row['refusal']}") + continue + if name not in here: + problems.append(f"{label}: the destination host has no pool {name} for disk files") + continue + pool, directory = here[name] + pool.refresh(0) + file = os.path.basename(row["path"]) + if self._find_volume(pool, file) is not None: + problems.append(f"{label}: pool {name} of the destination host already has a file {file}") + continue + if any(copy["dest"] == f"{directory}/{file}" for copy in copies): + problems.append(f"{label}: another disk is copied to {directory}/{file}") + continue + copies.append({"target": row["target"], "path": row["path"], "dest": f"{directory}/{file}", + "pool": pool}) + problems += [f"{target}: the VM has no disk file to copy" for target in pools.keys() - targets if pools[target]] + for xml in definitions[1:]: + problems += [f"{row['target']} ({row['path']}) of the next start: the destination host does not share it" + for row in self.disk_plan(conn, xml) if row["path"] not in carried and not row["shared"]] + if problems: + raise util.OperationError("The disks cannot be migrated: " + "; ".join(problems)) + return copies + + def remove_partial_copies(self, uuid, copies, error, nvram=None): + """Deletes the copies libvirt made here for a migration that failed, + and the copy of its NVRAM file, while this host has no VM of uuid (a + VM here has them as its disks).""" + try: + self.wvm.lookupByUUIDString(uuid) + return + except libvirtError as lookup_error: + if lookup_error.get_error_code() != VIR_ERR_NO_DOMAIN: + raise util.OperationError( + f"{error}; remove the partial disk copies from the destination host: " + + ", ".join(copy["dest"] for copy in copies) + ) from error + left = [] + for copy in copies: + copy["pool"].refresh(0) + volume = self._find_volume(copy["pool"], os.path.basename(copy["dest"])) + try: + if volume is not None: + volume.delete(0) + except libvirtError: + left.append(copy["dest"]) + try: + self.delete_nvram(nvram) + except libvirtError: + left.append(f"{NVRAM_DIR}/{nvram}") + if left: + raise util.OperationError( + f"{error}; remove the partial disk copies from the destination host: {', '.join(left)}" + ) from error @staticmethod def refuse_local_state(xml): @@ -201,7 +396,7 @@ class wvmInstances(wvmConnect): vol = self._find_volume(source, name) if vol is None: return None # never started: libvirt creates it at the first start - if self._nvram_shared(source, target): + if self.same_directory(source, target): if self._find_volume(target, name) is None: raise util.OperationError(f"The destination host cannot see the NVRAM file {path}") return None @@ -218,7 +413,7 @@ class wvmInstances(wvmConnect): with self._nvram_pool() as pool: pool.storageVolLookupByName(name).delete(0) - def missing_disks(self, definitions): + def missing_disks(self, definitions, copied=()): """(target, source) of the disks in the VM definitions (XML) that are not volumes of this host's active storage pools: the VM's own files, their backing files (as the definition or this host's volume lists @@ -240,6 +435,8 @@ class wvmInstances(wvmConnect): if kind == "volume": disks[(source.get("pool"), source.get("volume"))] = target elif kind in ("file", "block") and (source.get("file") or source.get("dev")): + if element is disk and source.get("file") in copied: + continue disks[(None, source.get("file") or source.get("dev"))] = target elif kind not in ("file", "block"): disks[(kind, None)] = target diff --git a/vrtManager/tests/test_disk_copy_migration.py b/vrtManager/tests/test_disk_copy_migration.py new file mode 100644 index 00000000..a368b1ec --- /dev/null +++ b/vrtManager/tests/test_disk_copy_migration.py @@ -0,0 +1,289 @@ +"""Migration with disks copied into pools of the destination: libvirt copies +the disks a pool is chosen for (NON_SHARED_DISK, only those targets), the +definitions name the new paths; every other file disk must be on storage +both hosts share. A shut-off VM is started paused for the copy and shut off +on the destination. Disks that cannot be copied are refused before anything +changes; the copies of a failed migration are removed again.""" + +import unittest +from unittest import mock + +import libvirt +from django.conf import settings + +if not settings.configured: + settings.configure(MAC_OUI="52:54:10") + +from vrtManager import util +from vrtManager.connection import wvmConnect +from vrtManager.instance import wvmInstances + +IMAGES = "/var/lib/libvirt/images" +FAST = "/srv/fast" + + +def error(code, message="error"): + err = libvirt.libvirtError(message) + err.err = (code, 0, message, 2, None, None, None, 0, 0) + return err + + +def disk(path, target="vda", device="disk", fmt="qcow2", extra=""): + return (f"" + f"{extra}") + + +def domain(*disks): + return f"{''.join(disks)}" + + +class Pool: + def __init__(self, name, path, files=(), kind="dir"): + self._name, self.path, self.kind = name, path, kind + self.files = set(files) + self.deleted = [] + + def name(self): + return self._name + + def XMLDesc(self, flags=0): + return f"{self._name}{self.path}" + + def refresh(self, flags=0): + pass + + def storageVolLookupByName(self, name): + if name not in self.files: + raise error(libvirt.VIR_ERR_NO_STORAGE_VOL) + volume = mock.Mock() + volume.delete.side_effect = lambda flags: self.deleted.append(name) + return volume + + +class Source: + """The source host's wvmInstance: a VM in state with the given running + (and next-start) definitions; its volumes have capacity and a backing + file; pools: the directories it has as pools.""" + + def __init__(self, state, xml, inactive=None, capacity=1 << 30, backing=None, pools=(IMAGES,)): + self.state = state + self.instance = mock.Mock() + self.instance.XMLDesc.side_effect = lambda flags: inactive if inactive and flags & 2 else xml + self.instance.snapshotNum.return_value = 0 + self.instance.UUIDString.return_value = "u-1" + self.wvm = mock.Mock() + volume = self.wvm.storageVolLookupByPath.return_value + volume.info.return_value = [0, capacity, capacity] + backing_xml = f"{backing}" if backing else "" + volume.XMLDesc.return_value = f"{backing_xml}" + self.pools = pools + + def get_status(self): + return self.state + + def get_arch(self): + return "x86_64" + + def get_dom_emulator(self): + return "/usr/bin/qemu-kvm" + + def pool_at(self, directory): + return Pool("src", directory) if directory in self.pools else None + + +def destination(*pools, shared=False, vm_here=False): + dest = wvmInstances.__new__(wvmInstances) + dest.wvm = mock.Mock() + dest.wvm.listAllStoragePools.return_value = list(pools) + if not vm_here: + dest.wvm.lookupByUUIDString.side_effect = error(libvirt.VIR_ERR_NO_DOMAIN) + dest.get_emulator = lambda arch: "/usr/bin/qemu-kvm" + if isinstance(shared, Exception): + dest.same_directory = mock.Mock(side_effect=shared) + else: + dest.same_directory = mock.Mock(return_value=shared) + dest.missing_disks = mock.Mock(return_value=[]) + dest.copy_nvram = mock.Mock(return_value=None) + dest.delete_nvram = mock.Mock() + return dest + + +def migrate(dest, source, pools, **options): + return dest.moveto(source, "vm", live=source.state == 1, unsafe=False, offline=source.state == 5, + uri="tcp://192.0.2.9", pools=pools, **options) + + +class DiskCopyTestCase(unittest.TestCase): + def test_disks_on_shared_storage_migrate_as_before(self): + source = Source(1, domain(disk(f"{IMAGES}/vm.qcow2"))) + dest = destination(Pool("default", IMAGES), shared=True) + self.assertEqual(migrate(dest, source, {}), ("live", [])) + source.instance.migrate.assert_called_once() + source.instance.migrate3.assert_not_called() + + def test_a_disk_the_destination_does_not_share_needs_a_pool(self): + for pools, shared in (((IMAGES,), False), ((), True)): # not shared; no pool to probe on the source + with self.subTest(pools=pools): + source = Source(1, domain(disk(f"{IMAGES}/vm.qcow2")), pools=pools) + with self.assertRaisesRegex(util.OperationError, "vda .*does not share it; choose a pool"): + migrate(destination(Pool("default", IMAGES), shared=shared), source, {}) + source.instance.migrate.assert_not_called() + + def test_disks_are_copied_into_the_chosen_pools_under_their_new_paths(self): + xml = domain(disk(f"{IMAGES}/vm.qcow2"), disk(f"{IMAGES}/data.raw", "vdb", fmt="raw"), + "") + source = Source(1, xml, inactive=xml) + dest = destination(Pool("default", IMAGES), Pool("fast", FAST)) + mode, copied = migrate(dest, source, {"vda": "fast", "vdb": "default"}) + self.assertEqual(mode, "live, disks copied: vda to fast, vdb to default") + self.assertEqual(copied, [f"{IMAGES}/vm.qcow2", f"{IMAGES}/data.raw"]) + params, flags = source.instance.migrate3.call_args.args[1:] + self.assertEqual(params[libvirt.VIR_MIGRATE_PARAM_MIGRATE_DISKS], ["vda", "vdb"]) + self.assertEqual(params[libvirt.VIR_MIGRATE_PARAM_URI], "tcp://192.0.2.9") + for key in (libvirt.VIR_MIGRATE_PARAM_DEST_XML, libvirt.VIR_MIGRATE_PARAM_PERSIST_XML): + self.assertIn(f'', params[key]) + self.assertIn(f'', params[key]) + self.assertEqual(flags & (libvirt.VIR_MIGRATE_NON_SHARED_DISK | libvirt.VIR_MIGRATE_LIVE), + libvirt.VIR_MIGRATE_NON_SHARED_DISK | libvirt.VIR_MIGRATE_LIVE) + self.assertEqual(dest.missing_disks.call_args.kwargs["copied"], {f"{IMAGES}/vm.qcow2", f"{IMAGES}/data.raw"}) + + def test_a_shut_off_vm_runs_paused_for_the_copy_and_is_shut_off_here(self): + source = Source(5, domain(disk(f"{IMAGES}/vm.qcow2"))) + dest = destination(Pool("default", IMAGES), vm_here=True) + mode, _copied = migrate(dest, source, {"vda": "default"}) + self.assertEqual(mode, "offline, disks copied: vda to default") + dest.copy_nvram.assert_called_once_with(source) # this host would start from the firmware's template + source.instance.createWithFlags.assert_called_once_with(libvirt.VIR_DOMAIN_START_PAUSED) + flags = source.instance.migrate3.call_args.args[2] + self.assertFalse(flags & (libvirt.VIR_MIGRATE_OFFLINE | libvirt.VIR_MIGRATE_LIVE)) + dest.wvm.lookupByUUIDString.return_value.destroy.assert_called_once_with() + + def test_a_shut_off_vm_whose_migration_fails_is_shut_off_on_the_source_again(self): + source = Source(5, domain(disk(f"{IMAGES}/vm.qcow2"))) + source.instance.migrate3.side_effect = libvirt.libvirtError("no route") + dest = destination(Pool("default", IMAGES)) + dest.copy_nvram.return_value = "vm_VARS.fd" + with self.assertRaisesRegex(libvirt.libvirtError, "no route"): + migrate(dest, source, {"vda": "default"}) + source.instance.destroy.assert_called_once_with() + dest.delete_nvram.assert_called_once_with("vm_VARS.fd") + + def test_a_vm_left_paused_here_is_named_but_the_migration_stands(self): + source = Source(5, domain(disk(f"{IMAGES}/vm.qcow2"))) + dest = destination(Pool("default", IMAGES), vm_here=True) + dest.wvm.lookupByUUIDString.return_value.destroy.side_effect = libvirt.libvirtError("busy") + mode, _copied = migrate(dest, source, {"vda": "default"}) + self.assertEqual(mode, "offline, disks copied: vda to default, left paused here, shut it off: busy") + + def test_disks_that_cannot_be_copied_are_refused_with_their_reasons(self): + cases = ( + (disk(f"{IMAGES}/x.iso", "vda", device="cdrom", fmt="raw"), {}, "eject it first"), + (disk(f"{IMAGES}/vm.qcow2", extra=""), {}, "read-only or encrypted"), + (disk(f"{IMAGES}/vm.vmdk", fmt="vmdk"), {}, "only raw and qcow2"), + (disk(f"{IMAGES}/vm.qcow2"), {"backing": f"{IMAGES}/base.qcow2"}, "backing file"), + ) + for xml, volume, reason in cases: + with self.subTest(reason=reason): + source = Source(1, domain(xml), **volume) + with self.assertRaisesRegex(util.OperationError, f"cannot be migrated: vda .*{reason}"): + migrate(destination(Pool("default", IMAGES)), source, {"vda": "default"}) + source.instance.migrate3.assert_not_called() + + def test_a_file_of_the_same_name_in_the_pool_is_never_overwritten(self): + source = Source(1, domain(disk(f"{IMAGES}/vm.qcow2"))) + with self.assertRaises(util.OperationError) as refused: + migrate(destination(Pool("fast", FAST, files={"vm.qcow2"})), source, {"vda": "fast"}) + self.assertEqual(str(refused.exception), "The disks cannot be migrated: " + f"vda ({IMAGES}/vm.qcow2): pool fast of the destination host already has a file vm.qcow2") + + def test_only_a_pool_of_disk_files_and_a_disk_of_the_vm_can_be_chosen(self): + source = Source(1, domain(disk(f"{IMAGES}/vm.qcow2"))) + dest = destination(Pool("vg", "/dev/vg", kind="logical")) + with self.assertRaisesRegex(util.OperationError, "has no pool vg for disk files; vdz: the VM has no disk"): + migrate(dest, source, {"vda": "vg", "vdz": "vg"}) + + def test_each_disk_keeps_its_own_new_path(self): + """Disks of one file name in two directories: neither copy takes the other's path.""" + xml = domain(disk(f"{IMAGES}/vm.qcow2"), disk(f"{FAST}/vm.qcow2", "vdb")) + source = Source(1, xml, pools=(IMAGES, FAST)) + dest = destination(Pool("default", IMAGES), Pool("fast", FAST), Pool("slow", "/srv/slow")) + migrate(dest, source, {"vda": "fast", "vdb": "slow"}) + dest_xml = source.instance.migrate3.call_args.args[1][libvirt.VIR_MIGRATE_PARAM_DEST_XML] + self.assertIn(f'', dest_xml) + self.assertIn('', dest_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.""" + source = Source(1, domain(disk(f"{IMAGES}/live.qcow2")), + inactive=domain(disk(f"{IMAGES}/next.qcow2"))) + with self.assertRaisesRegex(util.OperationError, f"vda \\({IMAGES}/next.qcow2\\) of the next start: " + "the destination host does not share it"): + migrate(destination(Pool("default", IMAGES)), source, {"vda": "default"}) + 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"): + migrate(destination(Pool("slow", "/srv/slow")), source, {"vda": "slow", "vdb": "slow"}) + + @staticmethod + def fail_after_copying(source, pool, message): + """libvirt made the copy here, then the migration failed.""" + def migrate3(*args): + pool.files.add("vm.qcow2") + raise libvirt.libvirtError(message) + source.instance.migrate3.side_effect = migrate3 + + def test_a_failed_migration_removes_the_partial_copies(self): + source, pool = Source(1, domain(disk(f"{IMAGES}/vm.qcow2"))), Pool("fast", FAST) + self.fail_after_copying(source, pool, "operation aborted: canceled by client") + with self.assertRaisesRegex(libvirt.libvirtError, "canceled by client"): + migrate(destination(pool), source, {"vda": "fast"}) + self.assertEqual(pool.deleted, ["vm.qcow2"]) + + def test_copies_a_vm_here_uses_are_kept(self): + source, pool = Source(1, domain(disk(f"{IMAGES}/vm.qcow2"))), Pool("fast", FAST) + self.fail_after_copying(source, pool, "lost reply") + with self.assertRaisesRegex(libvirt.libvirtError, "lost reply"): + migrate(destination(pool, vm_here=True), source, {"vda": "fast"}) + self.assertEqual(pool.deleted, []) + + +class DiskPlanTestCase(unittest.TestCase): + def test_each_file_disk_with_its_storage_and_reason(self): + xml = domain(disk(f"{IMAGES}/vm.qcow2"), disk(f"{IMAGES}/x.iso", "sda", device="cdrom", fmt="raw"), + disk("/srv/local/data.qcow2", "vdb")) + dest = destination(Pool("default", IMAGES), Pool("local", "/srv/local"), shared=True) + rows = dest.disk_plan(Source(1, ""), xml) + self.assertEqual([(r["target"], r["shared"], r["refusal"], r["capacity"]) for r in rows], [ + ("vda", True, None, 1 << 30), + ("sda", True, "an image is not copied; eject it first", 1 << 30), + ("vdb", False, None, 1 << 30), # no pool on the source: cannot be told + ]) + dest.same_directory.assert_called_once() # one probe per directory + + def test_a_probe_that_fails_means_not_shared(self): + dest = destination(Pool("default", IMAGES), shared=error(libvirt.VIR_ERR_SYSTEM_ERROR)) + rows = dest.disk_plan(Source(1, ""), domain(disk(f"{IMAGES}/vm.qcow2"))) + self.assertFalse(rows[0]["shared"]) + + +class SameDirectoryTestCase(unittest.TestCase): + def check(self, other_create): + pool, other = mock.Mock(), mock.Mock() + other.createXML.side_effect = other_create + try: + return wvmConnect.same_directory(pool, other) + finally: + pool.createXML.return_value.delete.assert_called_once() + + def test_a_file_the_other_host_has_already_means_one_directory(self): + for code in (libvirt.VIR_ERR_OPERATION_INVALID, libvirt.VIR_ERR_STORAGE_VOL_EXIST): + with self.subTest(code=code): + self.assertTrue(self.check(error(code))) + self.assertFalse(self.check(None)) + + def test_any_other_failure_tells_nothing(self): + with self.assertRaises(libvirt.libvirtError): + self.check(error(libvirt.VIR_ERR_SYSTEM_ERROR)) diff --git a/vrtManager/tests/test_migration.py b/vrtManager/tests/test_migration.py index 444053dc..72e9bf22 100644 --- a/vrtManager/tests/test_migration.py +++ b/vrtManager/tests/test_migration.py @@ -110,6 +110,9 @@ class FakePool: def isActive(self): return self.active + def XMLDesc(self, flags=0): + return "/elsewhere" + def refresh(self, flags=0): self.host.refreshes += 1 self.host.volumes |= self.host.unlisted @@ -291,7 +294,7 @@ class MigrationModeTestCase(unittest.TestCase): def migrate(self, state, **request): source = FakeSource(state) options = {"live": False, "unsafe": False, "offline": False, **request} - mode = destination().moveto(source, "vm", **options) + mode, _copied = destination().moveto(source, "vm", **options) return mode, source.instance.calls[0][0] if source.instance.calls else None def test_refused_requests(self): @@ -344,7 +347,14 @@ FILE_DISK = "