diff --git a/instances/jobs.py b/instances/jobs.py index ca5df964..547fdce0 100644 --- a/instances/jobs.py +++ b/instances/jobs.py @@ -825,13 +825,16 @@ class CreateInstance(Kind): MIGRATE_OPTIONS = ("live", "unsafe", "offline", "autoconverge", "compress", "delete_source") -def _target(instance, target_pk): - """The destination; refused when the VM is already there or it cannot be reached.""" +def migration_target(instance, target_pk): + """The destination; refused when the VM is already there, also under + another record of its host, or it cannot be reached.""" target = Compute.objects.get(pk=target_pk) if target.pk == instance.compute_id: raise util.OperationError(_("The instance is already on %(compute)s") % {"compute": target.name}) if not target.status: raise util.OperationError(_("%(compute)s is not reachable") % {"compute": target.name}) + if target.proxy.host_uuid() == instance.proxy.host_uuid(): + raise util.OperationError(_("%(compute)s is the VM's own host under another name") % {"compute": target.name}) return target @@ -862,7 +865,7 @@ class MigrateInstance(Kind): def validate(self, user, params): if not user.is_superuser: raise PermissionDenied - _target(_instance(params["instance"]), params["target"]) + migration_target(_instance(params["instance"]), params["target"]) def plan(self, user, params): instance = _instance(params["instance"]) @@ -887,7 +890,7 @@ class MigrateInstance(Kind): instance = _instance(op.params["instance"]) if instance.compute_id != op.intent["compute"]: raise util.OperationError("The VM moved to another compute after the job was queued") - _target(instance, op.params["target"]) + migration_target(instance, op.params["target"]) def run(self, op, ctx): p = op.params diff --git a/instances/templates/instance.html b/instances/templates/instance.html index 9121cc13..f9b6a644 100644 --- a/instances/templates/instance.html +++ b/instances/templates/instance.html @@ -425,11 +425,40 @@ submit.disabled = false; // the migration checks them again }); } + // the destinations that cannot be chosen (unreachable, or this host under + // another record) are marked first; then the disks of the first one left + function loadMigrateTargets() { + var submit = migratePlan.closest("form").querySelector('button[name="migrate"]'); + submit.disabled = true; + migrateCompute.disabled = true; // no disks are checked for a host before the hosts are + migratePlan.textContent = "{{ _("Checking the hosts…")|escapejs }}"; + fetch(migratePlan.dataset.targetsUrl).then(function (response) { + if (!response.ok) throw new Error(response.statusText); + return response.json(); + }).then(function (refused) { + migrateCompute.disabled = false; + Array.prototype.forEach.call(migrateCompute.options, function (option) { + if (refused[option.value]) { + option.disabled = true; + option.textContent = refused[option.value]; // it names the host + } + }); + var first = Array.prototype.find.call(migrateCompute.options, function (option) { return !option.disabled; }); + if (!first) { + migratePlan.textContent = "{{ _("No host can take the VM now.")|escapejs }}"; + return; + } + if (migrateCompute.selectedOptions[0].disabled) migrateCompute.value = first.value; + loadMigratePlan(); + }).catch(function () { + migratePlan.textContent = "{{ _("The hosts could not be checked.")|escapejs }}"; + }); + } if (migratePlan && migrateCompute) { document.querySelector('[data-bs-target="#migrate"]').addEventListener("shown.bs.tab", function () { if (!migratePlan.dataset.loaded) { migratePlan.dataset.loaded = "true"; - loadMigratePlan(); + loadMigrateTargets(); } }); migrateCompute.addEventListener("change", loadMigratePlan); diff --git a/instances/templates/instances/migrate_plan.html b/instances/templates/instances/migrate_plan.html index 703b2599..bcf39c4d 100644 --- a/instances/templates/instances/migrate_plan.html +++ b/instances/templates/instances/migrate_plan.html @@ -2,30 +2,50 @@ {# the disks of the Migrate tab for the chosen destination: where each goes; data-refused keeps the form from being sent #} {% if error %}
{% blocktrans %}The disks could not be checked: {{ error }}{% endblocktrans %}
-{% elif not disks %} +{% elif not disks and not pending %}

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

{% else %} {% for disk in disks %}

{{ disk.target }} · {{ disk.capacity|filesizeformat }}

{{ disk.path }}

- {% if disk.shared or not disk.refusal and pools %} + {% if disk.shared or not disk.refusal and disk.free_pools %} {% elif disk.refusal %}

{% blocktrans with reason=disk.refusal %}Cannot be copied: {{ reason }}{% endblocktrans %}

+ {% elif pools %} +

{% trans "Every pool of the destination host has a file of this name" %}

{% else %}

{% trans "No pool of the destination host can take it" %}

{% endif %} {% endfor %} +{% if pending %} +
+

{% trans "Disks of the next start" %}

+

{% trans "The VM gets these at its next start; the migration does not copy them." %}

+ +
+{% 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 %} diff --git a/instances/templates/instances/settings_tab.html b/instances/templates/instances/settings_tab.html index 02aa6407..093af3d9 100644 --- a/instances/templates/instances/settings_tab.html +++ b/instances/templates/instances/settings_tab.html @@ -514,7 +514,7 @@

{% bs_icon 'device-hdd' %} {% trans "Storage" %}

-
+
diff --git a/instances/tests/test_migrate_job.py b/instances/tests/test_migrate_job.py index dcd8b37b..00037b7d 100644 --- a/instances/tests/test_migrate_job.py +++ b/instances/tests/test_migrate_job.py @@ -11,7 +11,7 @@ from appsettings.settings import get_settings from computes.models import Compute from instances.jobs import MigrateInstance from instances.models import Instance -from libvirt import libvirtError +from libvirt import VIR_DOMAIN_XML_INACTIVE, libvirtError from operations.jobs import Cancelled, Context, get_kind from operations.models import Claim, Operation, acquire @@ -30,6 +30,8 @@ class MigrateJobTestCase(TransactionTestCase): proxy.get_nvram.return_value = None patch.object(Compute, "status", True).start() patch.object(Instance, "proxy", proxy).start() + # another host than the VM's + patch.object(Compute, "proxy", MagicMock(**{"host_uuid.return_value": "dddddddd-0000-0000-0000-000000000051"})).start() self.dest_conn = patch("instances.utils.wvmInstances").start().return_value self.dest_conn.moveto.return_value = ("live", []) self.addCleanup(patch.stopall) @@ -97,26 +99,53 @@ class MigrateJobTestCase(TransactionTestCase): 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}, + dest.migration_plan.return_value = ([ + {"target": "vda", "path": "/i/mig-vm.qcow2", "capacity": 1 << 30, "shared": False, "refusal": None, + "free_pools": ["default", "fast"], "default": "default"}, + {"target": "vdb", "path": "/i/data.qcow2", "capacity": 1 << 30, "shared": False, "refusal": None, + "free_pools": ["default"], "default": "default"}, {"target": "sda", "path": "/i/os.iso", "capacity": 1 << 20, "shared": False, - "refusal": "an image is not copied; eject it first"}] + "refusal": "an image is not copied; eject it first", "free_pools": [], "default": None}], + # the next start's disks the migration does not carry + [{"target": "vdc", "path": "/i/new.qcow2", "shared": False}, + {"target": "vdd", "path": "/nfs/new.qcow2", "shared": True}]) 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 + # the definitions it plans with: the one it carries and the next start's + self.assertEqual(dest.migration_plan.call_args.args[1:], ("", "")) + self.assertEqual(Instance.proxy.instance.XMLDesc.call_args_list[-1].args, (VIR_DOMAIN_XML_INACTIVE,)) + # the pool on the disk's directory is chosen; one that has a file of the name cannot be self.assertIn('', page) 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.assertIn("/nfs/new.qcow2:\n on storage both hosts share", page) # a disk that cannot go keeps the form from being sent, as does a destination that does not answer - self.assertEqual(page.count("data-refused"), 1) + self.assertEqual(page.count("data-refused"), 2) + dest.close.assert_called_once_with() with patch("instances.views.wvmInstances", side_effect=libvirtError("no answer")): page = self.client.get(reverse("instances:migrate_plan", args=[self.vm.pk]), {"compute_id": self.dest.pk}).content.decode() self.assertIn("data-refused", page) self.assertEqual(self.client.get(reverse("instances:migrate_plan", args=[self.vm.pk])).status_code, 404) + def test_the_destinations_that_cannot_be_chosen(self): + down = Compute.objects.create(name="mig-down", hostname="192.0.2.52", login="root", password="", type=1) + alias = Compute.objects.create(name="mig-alias", hostname="192.0.2.53", login="root", password="", type=1) + Instance.proxy.host_uuid.return_value = "aaaaaaaa-0000-0000-0000-000000000050" + # the alias is the source host under another record: the same libvirt host UUID + uuids = {alias.pk: "aaaaaaaa-0000-0000-0000-000000000050"} + with patch.object(Compute, "status", property(lambda c: c.pk != down.pk)), patch.object( + Compute, "proxy", property(lambda c: MagicMock(**{"host_uuid.return_value": uuids.get(c.pk, str(c.pk))}))): + refused = self.client.get(reverse("instances:migrate_targets", args=[self.vm.pk])).json() + self.assertEqual(refused, {str(down.pk): "mig-down is not reachable", + str(alias.pk): "mig-alias is the VM's own host under another name"}) + # a request that names it anyway is refused too + self.client.post(reverse("instances:migrate", args=[self.vm.pk]), + {"compute_id": alias.pk, "submission_key": "key-mig00002"}) + self.assertFalse(Operation.objects.exists()) + 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/urls.py b/instances/urls.py index b3848d10..ac7170da 100755 --- a/instances/urls.py +++ b/instances/urls.py @@ -61,6 +61,7 @@ urlpatterns = [ 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("/migrate_targets/", views.migrate_targets, name="migrate_targets"), path("/move_disk/", views.move_disk, name="move_disk"), path("/add_usb_hostdev/", views.add_usb_hostdev, name="add_usb_hostdev"), path("/delete_usb_hostdev/", views.delete_usb_hostdev, name="delete_usb_hostdev"), diff --git a/instances/views.py b/instances/views.py index 6631f91e..4a806e5c 100755 --- a/instances/views.py +++ b/instances/views.py @@ -41,6 +41,7 @@ from libvirt import (VIR_DOMAIN_PAUSED, VIR_DOMAIN_UNDEFINE_MANAGED_SAVE, VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, VIR_DOMAIN_UNDEFINE_NVRAM, + VIR_DOMAIN_XML_INACTIVE, VIR_ERR_NO_SUPPORT, libvirtError) from logs.audit import audit, audit_vm, changed_fields, failed_vm @@ -910,6 +911,23 @@ def move_disk(request, pk): return _back(request, pk, "disks") +@superuser_only +def migrate_targets(request, pk): + """The Migrate tab's destinations that cannot be chosen, {id: why}: one + that cannot be reached, or the VM's own host under another record. + Loaded when the tab opens, so the page does not wait for the hosts.""" + from instances.jobs import migration_target + + instance = get_instance(request.user, pk) + refused = {} + for target in Compute.objects.exclude(pk=instance.compute_id): + try: + migration_target(instance, target.pk) + except libvirtError as err: + refused[target.pk] = error_text(request, err) + return JsonResponse(refused) + + @superuser_only def migrate_plan(request, pk): """The disks of the Migrate tab for the chosen destination, loaded when @@ -923,15 +941,10 @@ def migrate_plan(request, pk): 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} + pools = [{"name": name, "free": pool.info()[3]} for name, (pool, directory) in sorted(dest.file_pools().items())] + domain = instance.proxy.instance + disks, pending = dest.migration_plan(instance.proxy, domain.XMLDesc(0), domain.XMLDesc(VIR_DOMAIN_XML_INACTIVE)) + context = {"disks": disks, "pending": pending, "pools": pools, "shut_off": instance.proxy.get_status() == 5} except libvirtError as err: context = {"error": error_text(request, err)} finally: diff --git a/vrtManager/instance.py b/vrtManager/instance.py index a48907b5..99aeea86 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -276,6 +276,29 @@ class wvmInstances(wvmConnect): "refusal": self.copy_refusal(disk, volume)}) return rows + def migration_plan(self, conn, live_xml, next_xml): + """The disks of the Migrate tab for a VM coming here (disk_plan of + live_xml), each with the pools here it can be copied into, as + disk_copies takes them: {free_pools, default} added, and the file + disks of next_xml (the next start's definition) that the migration + does not carry, which must be on shared storage.""" + here = self.file_pools() + for pool, directory in here.values(): + pool.refresh(0) + disks = self.disk_plan(conn, live_xml) + for disk in disks: + file = os.path.basename(disk["path"]) + # a copy never replaces a file: a pool that has one of its name cannot take it + disk["free_pools"] = [name for name, (pool, directory) in sorted(here.items()) + if self._find_volume(pool, file) is None] + # a copy goes by default to the pool on the same directory, else the first + names = [name for name in disk["free_pools"] if here[name][1] == os.path.dirname(disk["path"])] + names = names or disk["free_pools"] + disk["default"] = names[0] if names else None + carried = {disk["path"] for disk in disks} + pending = [row for row in self.disk_plan(conn, next_xml) if row["path"] not in carried] + return disks, pending + @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.""" diff --git a/vrtManager/tests/test_migration.py b/vrtManager/tests/test_migration.py index 72e9bf22..d2acee55 100644 --- a/vrtManager/tests/test_migration.py +++ b/vrtManager/tests/test_migration.py @@ -499,3 +499,24 @@ class NvramCopyTestCase(unittest.TestCase): self.assertIs(raised.exception, error) self.dest_pool.storageVolLookupByName.assert_called_once_with("vm_VARS.fd") self.dest_pool.storageVolLookupByName.return_value.delete.assert_called_once_with(0) + + +class MigrationPlanTestCase(unittest.TestCase): + """The Migrate tab's plan: the pools each disk can be copied into, and the + next start's disks the migration does not carry.""" + + def test_pools_with_a_file_of_the_name_and_the_next_start_disks(self): + dest = wvmInstances.__new__(wvmInstances) + pools = {"default": (mock.MagicMock(), "/i"), "fast": (mock.MagicMock(), "/srv/fast"), + "slow": (mock.MagicMock(), "/srv/slow")} + dest.file_pools = lambda: pools + vda = {"target": "vda", "path": "/i/vm.qcow2", "shared": False, "refusal": None} + vdb = {"target": "vdb", "path": "/i/new.qcow2", "shared": False, "refusal": None} + dest.disk_plan = lambda conn, xml: [dict(row) for row in {"live": [vda], "next": [vda, vdb]}[xml]] + # the pool on the disk's directory has a file of its name + dest._find_volume = lambda pool, name: object() if pool is pools["default"][0] else None + disks, pending = dest.migration_plan(None, "live", "next") + self.assertEqual((disks[0]["free_pools"], disks[0]["default"]), (["fast", "slow"], "fast")) + self.assertEqual(pending, [vdb]) + for pool, directory in pools.values(): + pool.refresh.assert_called_once_with(0) # a file made since is seen