diff --git a/instances/templates/instance.html b/instances/templates/instance.html index 955df7eb..0a0adc70 100644 --- a/instances/templates/instance.html +++ b/instances/templates/instance.html @@ -409,7 +409,8 @@ return response.text(); }).then(function (html) { migratePlan.innerHTML = html; - submit.disabled = false; + // a destination that did not answer, or a disk that cannot go there + submit.disabled = migratePlan.querySelector("[data-refused]") !== null; }).catch(function () { if (loading.signal.aborted) return; // another destination was chosen migratePlan.textContent = "{{ _("The disks could not be checked.")|escapejs }}"; diff --git a/instances/templates/instances/migrate_plan.html b/instances/templates/instances/migrate_plan.html index ba2e90a8..703b2599 100644 --- a/instances/templates/instances/migrate_plan.html +++ b/instances/templates/instances/migrate_plan.html @@ -1,42 +1,32 @@ {% load i18n %} -{# the disks of the Migrate tab for the chosen destination: where each goes #} +{# 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 %}
+
{% 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 %} -
+{% for disk in disks %} + +

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

+

{{ disk.path }}

+ {% 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 %}
-

+{% endfor %} +

{% 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 d00e75d9..b2f3305d 100644 --- a/instances/templates/instances/settings_tab.html +++ b/instances/templates/instances/settings_tab.html @@ -484,86 +484,98 @@ {% endif %}
-

{% trans "For migration both host servers must have equal settings and OS type" %}

-
- {% csrf_token %}{% submission_key %} -
- -
- -
-
-
- -
- +
+ +
+ + +
+
+

+ {% if instance.status == 1 %}{% trans "The VM is running: it migrates live, or paused for the transfer when Live migration is off." %} + {% elif instance.status == 3 %}{% trans "The VM is paused: it stays paused on the destination." %} + {% elif instance.status == 5 %}{% trans "The VM is shut off: it moves offline." %}{% endif %} + {% trans "Both hosts need the same emulator; the VM's definition leaves the source host." %} +

+ + {# a VM that cannot go anywhere gets the reason, not the disks and settings #} + {% if not migrate_refusal %} +
+
+

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

+
+
+
+
+
+

{% bs_icon 'arrow-left-right' %} {% trans "Transfer settings" %}

+
+ {% if instance.status == 1 or instance.status == 3 %} +
+
+ + +
+

{% trans "The VM keeps running while its memory is copied. Off, it is paused for the transfer." %}

+
{% endif %} - -
-
-
- -
-
-
-
-
- - + {% if instance.status == 1 %} +
+
+ + +
+

{% trans "Slows the guest's CPUs so that a busy VM's live migration can finish. Only with Live migration." %}

+
+
+
+ + +
+

{% trans "Compresses the memory sent: less network, more CPU on both hosts." %}

+
+
+
+ + +
+

{% trans "Skips libvirt's safety checks, such as a disk cache mode that can corrupt data in a migration." %}

+
+ {% endif %} +
+
+ + +
+

{% trans "After a migration that copied disks, their files on the source host go. Disks on shared storage stay." %}

+
-
-
-
-
- - -
-
-
-
-
-
- - -
-
-
-
-
-
- - -
-
-
-
-
-
- - -
-
-
-
-
-
- - -
-
-
- {% if computes_count != 1 %} - - {% else %} - +
{% endif %} +
+ {% if migrate_refusal %}

{{ migrate_refusal }}

{% endif %} +
+

{% trans "Migration runs as a background job." %}

+ {% if migrate_refusal %} + + {% else %} + {# enabled once the disks of the chosen destination are shown #} + + {% endif %} +
+
diff --git a/instances/tests/test_migrate_form.py b/instances/tests/test_migrate_form.py index a188395f..54ac4840 100644 --- a/instances/tests/test_migrate_form.py +++ b/instances/tests/test_migrate_form.py @@ -1,7 +1,8 @@ -"""The migrate form ticks auto converge for a running VM, and the API uses -the same default: it only slows a VM that the migration cannot keep up with.""" +"""The migrate form shows the switches the VM's state uses and ticks auto +converge for a running VM, and the API uses the same default: it only slows +a VM that the migration cannot keep up with. A VM that cannot migrate gets +the reason instead.""" -import re from unittest.mock import patch import libvirt @@ -10,6 +11,7 @@ from django.contrib.auth import get_user_model from django.test import TestCase from django.urls import reverse from instances.models import Instance +from lxml import html from operations.models import Operation from rest_framework.test import APIClient @@ -37,20 +39,34 @@ class MigrateFormTests(TestCase): self.domain.destroy() self.domain.undefine() - def autoconverge_input(self): + def migrate_form(self): self.client.force_login(self.admin) with patch("vrtManager.connection.connection_manager.get_connection", return_value=self.conn): - page = self.client.get(reverse("instances:instance", args=[self.vm.id])).content.decode() - return re.search(r']*id="autoconverge"[^>]*>', page).group(0) + page = self.client.get(reverse("instances:instance", args=[self.vm.id])).content + (form,) = html.fromstring(page).xpath("//form[@action=$a]", a=reverse("instances:migrate", args=[self.vm.id])) + return form def test_auto_converge_is_ticked_for_a_running_vm(self): self.domain.create() - self.assertIn("checked", self.autoconverge_input()) + self.assertEqual(self.migrate_form().xpath(".//input[@name='autoconverge']/@checked"), ["checked"]) - def test_a_shut_off_vm_has_no_auto_converge(self): - field = self.autoconverge_input() - self.assertIn("disabled", field) - self.assertNotIn("checked", field) + def test_each_state_shows_the_switches_it_uses(self): + # the mode follows the state: no Offline switch; unsafe, auto converge and compressed apply to a running VM + for state, switches in (("shut off", ["delete_source"]), + ("running", ["live_migrate", "autoconverge", "compress", "unsafe_migrate", "delete_source"]), + ("paused", ["live_migrate", "delete_source"])): + with self.subTest(state): + if state == "running": + self.domain.create() + elif state == "paused": + self.domain.suspend() + self.assertEqual(self.migrate_form().xpath(".//input[@type='checkbox']/@name"), switches) + + def test_a_vm_with_no_other_host_gets_the_reason(self): + self.other.delete() + form = self.migrate_form() + self.assertIn("There is no other host to migrate to.", form.text_content()) + self.assertEqual(form.xpath(".//button[@name='migrate']"), []) def test_the_api_uses_auto_converge_unless_told_otherwise(self): api = APIClient() diff --git a/instances/tests/test_migrate_job.py b/instances/tests/test_migrate_job.py index a86f338d..42206289 100644 --- a/instances/tests/test_migrate_job.py +++ b/instances/tests/test_migrate_job.py @@ -109,6 +109,12 @@ class MigrateJobTestCase(TransactionTestCase): 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) + # 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) + 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_a_cancelled_migration_ends_cancelled_and_the_vm_stays(self): diff --git a/instances/views.py b/instances/views.py index 117bfdcb..94d38c46 100755 --- a/instances/views.py +++ b/instances/views.py @@ -322,6 +322,18 @@ def _instance_page(request, pk, instance, compute): show_hostdevs = request.user.is_superuser or bool(usb_hostdevs or pci_hostdevs or other_hostdevs) show_settings = show_settings or show_hostdevs + # what keeps the Migrate tab from migrating the VM to any destination + if computes_count == 1: + migrate_refusal = _("There is no other host to migrate to.") + elif instance.status not in (1, 3, 5): + migrate_refusal = _("A VM migrates when it is running, paused or shut off.") + elif instance.snapshots: + migrate_refusal = _("A VM with snapshots cannot be migrated: its snapshots stay on this host. Delete them first.") + elif usb_hostdevs or pci_hostdevs or other_hostdevs: + migrate_refusal = _("A VM with host devices cannot be migrated: the destination may not have them. Remove them first.") + else: + migrate_refusal = None + job_resource = claims.instance_key(instance.uuid) return render(request, "instance.html", locals(),) @@ -849,7 +861,7 @@ def migrate(request, pk): raise Http404 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"), + "unsafe": request.POST.get("unsafe_migrate"), "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()