diff --git a/instances/templates/instance.html b/instances/templates/instance.html index 1af69283..a4691e91 100644 --- a/instances/templates/instance.html +++ b/instances/templates/instance.html @@ -258,17 +258,6 @@ }); }); - diff --git a/instances/templates/instances/resize_tab.html b/instances/templates/instances/resize_tab.html index 73a9f3c7..279cef30 100644 --- a/instances/templates/instances/resize_tab.html +++ b/instances/templates/instances/resize_tab.html @@ -1,4 +1,5 @@ {% load i18n %} +{% load bootstrap_icons %}
@@ -25,172 +26,180 @@
{% if can_change %} - {% if instance.status == 5 or not instance.vcpus %} -
{% csrf_token %} -

{% trans "Logical host CPUs" %} : {{ vcpu_host }}

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

{% bs_icon 'cpu' %} {% trans "CPU allocation" %}

+

{% blocktrans with cur=instance.cur_vcpu max=instance.vcpu %}Now {{ cur }} of {{ max }} vCPUs.{% endblocktrans %}

+

{% trans "Host" %}: {% blocktrans with count=vcpu_host %}{{ count }} logical CPUs{% endblocktrans %}

+
+
+

+ {% if instance.status == 5 %}{% trans "The VM gets these at its next start." %} + {% else %}{% trans "Shut the VM down to change its vCPUs." %}{% if instance.vcpus %} {% trans "While it runs, turn vCPUs on and off below." %}{% endif %}{% endif %} +

+
+
+ + +
+
+ + +
+
+
+ +
-
- -
- -
-
- - {% if instance.status == 5 %} - - {% else %} - - {% endif %} -
- {% else %} -

{% trans "Logical Instance Active/Maximum CPUs" %} : {{ instance.cur_vcpu }} / {{ instance.vcpu }}

-
-
- {% if request.user.is_superuser %} - {% for id, vcpu in instance.vcpus.items %} -
{% csrf_token %} -
- - {% if vcpu.enabled == 'yes' and vcpu.hotpluggable == "yes" %} - - {% elif vcpu.enabled == 'yes' and vcpu.hotpluggable == "no" %} - - {% else %} - - {% endif %} + {% if request.user.is_superuser and instance.vcpus and instance.status != 5 %} +
+
+
+

{% trans "Live vCPUs" %}

+

+ {% trans "Turn a vCPU on or off now; the next start keeps it so. vCPU 0 stays on." %} + {% if instance.status == 3 %}{% trans "A paused VM cannot let one go: resume it to turn vCPUs off." %}{% endif %} +

+
+
+
+ {% for id, vcpu in instance.vcpus.items %} + {% csrf_token %} + + {% if vcpu.hotpluggable != "yes" %} + + {% elif vcpu.enabled == "yes" %} + + {% else %} + + {% endif %} + + {% endfor %}
- - {% endfor %} - {% endif %} +
-
+
{% endif %} - {% else %} - {% trans "You don't have permission for resizing instance" %} - - {% endif %} -
- {% if request.user.is_superuser %} -
{% csrf_token %} -
-
- -

- {% trans "Lets vCPUs up to the maximum be turned on and off while the VM runs." %} - {% if instance.status != 5 %}{% trans "Shut the VM down to change this." %}{% endif %} -

-
-
-
-
- + {% if request.user.is_superuser %} + {% csrf_token %} +
+
+ +

+ {% trans "Lets vCPUs up to the maximum be turned on and off while the VM runs." %} + {% if instance.status != 5 %}{% trans "Shut the VM down to change this." %}{% endif %} +

+
+
+
+
+ +
+
+ +
-
- +
+
+ + {% endif %} +
+ {% else %} +

{% trans "You don't have permission for resizing instance" %}

+ {% endif %} +
+
+ {% if can_change %} +
{% csrf_token %} +
+
+
+

{% bs_icon 'memory' %} {% trans "Memory allocation" %}

+

{% blocktrans with cur=instance.cur_memory max=instance.memory %}Now {{ cur }} MiB of maximum {{ max }} MiB.{% endblocktrans %}

+

{% trans "Host" %}: {% blocktrans with total=memory_host|filesizeformat %}{{ total }} memory{% endblocktrans %}

+
+
+

+ {% if instance.status == 5 %}{% trans "The VM gets these at its next start." %} + {% else %}{% trans "The current memory changes now, up to the maximum; shut the VM down to change the maximum." %}{% endif %} +

+
+
+ + +
+
+ + {# a running VM keeps its maximum: only the current memory changes #} + +
+
+
+
- {% endif %} -
-
- {% if can_change %} -
- {% csrf_token %} -

{% trans "Total host memory" %}: {{ memory_host|filesizeformat }}

-
- -
- - - {% trans "Custom value" %} -
-
-
- -
- {# a running VM keeps its maximum: only the current memory changes #} - - {% if instance.status == 5 %} - - {% trans "Custom value" %} - {% else %} - {% trans "Shut off the VM to change the maximum" %} - {% endif %} -
-
- -
{% else %} - {% trans "You don't have permission for resizing instance" %} - +

{% trans "You don't have permission for resizing instance" %}

{% endif %} -
{% if can_change %} -
- {% csrf_token %} -

{% trans "New disk size (GiB); leave empty to keep a disk as it is" %}:

- {% for disk in instance.disks %} -
- - {% if disk.storage is None %} -
-
- {% trans "Error getting disk info" %} + {% csrf_token %} +
+
+
+

{% bs_icon 'device-hdd' %} {% trans "Grow disks" %}

+

{% trans "A new size in GiB, larger than the disk's; empty keeps a disk as it is. Disks grow while the VM runs; the guest then grows its partition and file system." %}

+
+
+
+ {% for disk in instance.disks %} +
+ + {% if disk.storage is None %} +

{% trans "Error getting disk info" %}

+ {% else %} +

{% blocktrans with size=disk.size|filesizeformat %}Now {{ size }}{% endblocktrans %}

+
+ + GiB +
+ {% endif %} +
+ {% endfor %} +
+
+ {% if instance.status == 1 or instance.status == 3 or instance.status == 5 %} + + {% else %} + + {% endif %}
- {% else %} -
- {# empty: the disk stays as it is #} - -
- {% endif %}
- {% endfor %} - {% if instance.status == 1 or instance.status == 3 or instance.status == 5 %} - - {% else %} - - {% endif %} - +
+ {% else %} - {% trans "You don't have permission for resizing instance" %} - +

{% trans "You don't have permission for resizing instance" %}

{% endif %} -
diff --git a/instances/tests/test_live_guest.py b/instances/tests/test_live_guest.py index 4a3fdf7c..449d93de 100644 --- a/instances/tests/test_live_guest.py +++ b/instances/tests/test_live_guest.py @@ -252,6 +252,9 @@ class LiveGuestTestCase(TestCase): self.assertEqual(online(), 1) self.post("set_vcpu", inst, {"id": "1", "set_vcpu": "True"}) self.assertTrue(wait_for(lambda: online() == 2, 60), "guest did not bring the new vCPU online") + # the next start keeps it + persistent = etree.fromstring(dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE)) + self.assertEqual(persistent.xpath("vcpus/vcpu[@id='1']/@enabled"), ["yes"]) def test_agent_reports_the_guest_os_and_nic(self): dom, inst = self.boot("gagent") diff --git a/instances/tests/test_missing_fields.py b/instances/tests/test_missing_fields.py index cb5705b3..8d27534c 100644 --- a/instances/tests/test_missing_fields.py +++ b/instances/tests/test_missing_fields.py @@ -65,7 +65,7 @@ class MissingFieldTestCase(TestCase): self.assert_reported(self.post("resizevm_cpu", {"vcpu": "²", "cur_vcpu": "1"})) def test_resize_memory_with_a_unicode_digit(self): - self.assert_reported(self.post("resize_memory", {"memory": "512", "cur_memory_custom": "²"})) + self.assert_reported(self.post("resize_memory", {"memory": "512", "cur_memory": "²"})) def test_set_qos_with_non_numeric_rate(self): self.assert_reported(self.post("set_qos", {"qos_direction": "inbound", "net-mac-0": "52:54:00:00:00:01", "qos_average": "fast"})) diff --git a/instances/tests/test_pending_edits.py b/instances/tests/test_pending_edits.py index ce6e0cc5..36f29b29 100644 --- a/instances/tests/test_pending_edits.py +++ b/instances/tests/test_pending_edits.py @@ -168,6 +168,19 @@ class PendingEditsTestCase(TestCase): # not an owner yet and not an administrator, who can use every VM anyway self.assertEqual(doc.xpath("//select[@id='add-owner-user']/option/text()"), ["pe-new"]) + def test_a_paused_vm_offers_to_turn_vcpus_on_only(self): + # unplugging a vCPU needs the guest, which a paused VM does not run + vcpus = {"0": {"enabled": "yes", "hotpluggable": "no", "order": "1"}, + "1": {"enabled": "yes", "hotpluggable": "yes", "order": "2"}, + "2": {"enabled": "no", "hotpluggable": "yes", "order": "3"}} + for status, off_disabled in ((1, False), (3, True)): + with self.subTest(status=status): + doc = self.page(status=status, get_vcpus=(vcpus, vcpus)) + buttons = {b.get("value"): b.get("disabled") is not None + for b in doc.xpath("//form[@action=$a]//button[@name='set_vcpu']", + a=reverse("instances:set_vcpu", args=[self.instance.id]))} + self.assertEqual(buttons, {"False": off_disabled, "True": False}) + def test_buttons_shown_as_disabled_cannot_submit(self): # running: the CPU resize, vCPU hot plug, template and disk removal forms wait for a shutdown; # the title, description and video model are of the next start, saved at once diff --git a/instances/tests/test_resize_input.py b/instances/tests/test_resize_input.py index adc7bf07..5f29118b 100644 --- a/instances/tests/test_resize_input.py +++ b/instances/tests/test_resize_input.py @@ -48,6 +48,26 @@ class ResizeInputTests(TestCase): self.assertEqual([(d["dev"], d["size_new"]) for d in proxy.resize_disk.call_args.args[0]], [("vdb", 30 * GIB)]) self.assertEqual(messages, ["Disk is resized: vdb"]) + def test_a_smaller_disk_size_refuses_every_change(self): + # a disk does not shrink: one smaller size leaves the others as they are too + disks = [{"dev": "vda", "size": 10 * GIB}, {"dev": "vdb", "size": 20 * GIB}] + messages, proxy = self.post("instances:resize_disk", {"disk_size_vda": "5", "disk_size_vdb": "30"}, disks=disks) + proxy.resize_disk.assert_not_called() + self.assertEqual(messages, ["vda is 10.0\xa0GB: a disk cannot shrink"]) + + def test_vcpus_and_memory_must_be_at_least_one(self): + for view, data, setter in (("instances:resizevm_cpu", {"vcpu": "2", "cur_vcpu": "0"}, "resize_cpu"), + ("instances:resize_memory", {"memory": "2048", "cur_memory": "-1"}, "resize_mem")): + with self.subTest(view): + messages, proxy = self.post(view, data) + getattr(proxy, setter).assert_not_called() + self.assertTrue(messages[-1].startswith("Enter a whole number"), messages) + + def test_a_cpu_resize_says_what_the_vm_gets(self): + messages, proxy = self.post("instances:resizevm_cpu", {"vcpu": "4", "cur_vcpu": "2"}) + proxy.resize_cpu.assert_called_once_with("2", "4") + self.assertEqual(messages, ["vCPUs set to 2 (maximum 4)"]) + def test_a_disk_size_that_is_not_a_whole_number_is_refused(self): messages, proxy = self.post("instances:resize_disk", {"disk_size_vda": "4.5"}, disks=[{"dev": "vda", "size": GIB}]) proxy.resize_disk.assert_not_called() @@ -72,7 +92,5 @@ class ResizeInputTests(TestCase): with patch("instances.models.wvmInstance"): self.vm.__dict__["status"] = VIR_DOMAIN_RUNNING from django.template.loader import render_to_string - html = render_to_string("instances/resize_tab.html", {"instance": self.vm, "can_change": True, - "memory_range": [1024, 2048]}) - self.assertRegex(html, r']*name="memory"[^>]*disabled>') diff --git a/instances/tests/test_snapshot_views.py b/instances/tests/test_snapshot_views.py index 8c43391a..c71460d7 100644 --- a/instances/tests/test_snapshot_views.py +++ b/instances/tests/test_snapshot_views.py @@ -165,8 +165,9 @@ class SnapshotTabTestCase(TestCase): self.assertEqual(box.get("disabled") is None, offered) def test_memory_checkbox_only_for_a_running_vm(self): - self.assertFalse(self.page(status=1).xpath("//input[@name='memory']")[0].get("disabled")) - self.assertIsNotNone(self.page(status=5).xpath("//input[@name='memory']")[0].get("disabled")) + checkbox = "//div[@id='snapshots']//input[@name='memory']" # the Resize tab has a memory field too + self.assertFalse(self.page(status=1).xpath(checkbox)[0].get("disabled")) + self.assertIsNotNone(self.page(status=5).xpath(checkbox)[0].get("disabled")) def test_a_running_job_disables_actions(self): # its progress shows in the job panel, not in the tab; the tab says why the actions are off diff --git a/instances/tests/test_views.py b/instances/tests/test_views.py index e621f1fc..63a76c3d 100644 --- a/instances/tests/test_views.py +++ b/instances/tests/test_views.py @@ -321,7 +321,7 @@ class InstancesTestCase(TestCase): response = self.client.post( reverse("instances:resize_memory", args=[self.instance.id]), - {"memory_custom": 500, "cur_memory_custom": 200}, + {"memory": 500, "cur_memory": 200}, ) self.assertRedirects( response, reverse("instances:instance", args=[self.instance.id]) + "#resize" diff --git a/instances/views.py b/instances/views.py index dbda8ea6..f4b9fd52 100755 --- a/instances/views.py +++ b/instances/views.py @@ -10,7 +10,6 @@ import re import socket import subprocess import time -from bisect import insort from ipaddress import ip_address from accounts.models import UserInstance, UserSSHKey @@ -30,6 +29,7 @@ from django.http import Http404, HttpResponse, JsonResponse from django.db.models import Prefetch, prefetch_related_objects from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse +from django.template.defaultfilters import filesizeformat from django.utils import timezone from django.utils.translation import gettext_noop as _ from django.views.decorators.http import require_POST @@ -278,11 +278,6 @@ def _instance_page(request, pk, instance, compute): request.user.is_superuser or can_manage_console or can_clone or can_change or instance.guest_agent_ready ) - memory_range = [256, 512, 768, 1024, 2048, 3072, 4096, 6144, 8192, 16384] - if instance.memory not in memory_range: - insort(memory_range, instance.memory) - if instance.cur_memory not in memory_range: - insort(memory_range, instance.cur_memory) clone_free_names = utils.get_clone_free_names() default_bus = app_settings.INSTANCE_VOLUME_DEFAULT_BUS @@ -1050,7 +1045,7 @@ def resizevm_cpu(request, pk): new_vcpu = request.POST.get("vcpu", "") new_cur_vcpu = request.POST.get("cur_vcpu", "") - if not _is_int(new_vcpu) or not _is_int(new_cur_vcpu): + if not _is_int(new_vcpu) or not _is_int(new_cur_vcpu) or int(new_cur_vcpu) < 1: messages.error(request, _("Enter a whole number of vCPUs")) return redirect(reverse("instances:instance", args=[instance.id]) + "#resize") # a running VM would get them only at its next start, past the quota counted now @@ -1077,11 +1072,7 @@ def resizevm_cpu(request, pk): vcpu = new_vcpu with audit_vm(request, instance, f"set vCPUs to {cur_vcpu} (maximum {vcpu})"): instance.proxy.resize_cpu(cur_vcpu, vcpu) - msg = _("CPU is resized: %(old)s to %(new)s") % { - "old": cur_vcpu, - "new": vcpu, - } - messages.success(request, msg) + messages.success(request, _("vCPUs set to %(cur)s (maximum %(max)s)") % {"cur": cur_vcpu, "max": vcpu}) return redirect(reverse("instances:instance", args=[instance.id]) + "#resize") @@ -1091,22 +1082,15 @@ def resizevm_cpu(request, pk): def resize_memory(request, pk): instance = get_instance(request.user, pk, perm_type="change") memory = instance.proxy.get_memory() - cur_memory = instance.proxy.get_cur_memory() new_memory = request.POST.get("memory", "") - new_memory_custom = request.POST.get("memory_custom", "") - if new_memory_custom: - new_memory = new_memory_custom new_cur_memory = request.POST.get("cur_memory", "") - new_cur_memory_custom = request.POST.get("cur_memory_custom", "") - if new_cur_memory_custom: - new_cur_memory = new_cur_memory_custom # The quota charges the maximum. An active VM keeps it (resize_mem changes # only the current memory there); pass the current maximum, so a VM that # shuts down meanwhile does not get an unchecked one. if instance.proxy.instance.isActive(): new_memory = memory - if not _is_int(new_memory) or not _is_int(new_cur_memory): + if not _is_int(new_memory) or not _is_int(new_cur_memory) or int(new_cur_memory) < 1: messages.error(request, _("Enter a whole number for memory")) return redirect(reverse("instances:instance", args=[instance.id]) + "#resize") if int(new_cur_memory) > int(new_memory): @@ -1124,15 +1108,8 @@ def resize_memory(request, pk): else: with audit_vm(request, instance, f"set memory to {new_cur_memory} MiB (maximum {new_memory} MiB)"): instance.proxy.resize_mem(new_cur_memory, new_memory) - msg = _( - "Memory is resized: current/max: %(old_cur)s/%(old_max)s to %(new_cur)s/%(new_max)s" - ) % { - "old_cur": cur_memory, - "old_max": memory, - "new_cur": new_cur_memory, - "new_max": new_memory, - } - messages.success(request, msg) + messages.success(request, _("Memory set to %(cur)s MiB (maximum %(max)s MiB)") % { + "cur": new_cur_memory, "max": new_memory}) return redirect(reverse("instances:instance", args=[instance.id]) + "#resize") @@ -1153,6 +1130,11 @@ def resize_disk(request, pk): if not _is_int(size): messages.error(request, _("Enter disk sizes as whole GiB")) return redirect(reverse("instances:instance", args=[instance.id]) + "#resize") + # a disk does not shrink: one smaller size refuses them all + if int(size) << 30 < disk["size"]: + messages.error(request, _("%(dev)s is %(size)s: a disk cannot shrink") % { + "dev": disk["dev"], "size": filesizeformat(disk["size"])}) + return redirect(reverse("instances:instance", args=[instance.id]) + "#resize") if int(size) << 30 > disk["size"]: disk["size_new"] = int(size) << 30 disks_new.append(disk) diff --git a/vrtManager/instance.py b/vrtManager/instance.py index 3d07ff53..381efc59 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -1307,7 +1307,8 @@ class wvmInstance(wvmConnect): self._defineXML(etree.tostring(tree).decode()) def set_vcpu(self, cpu_id, enabled): - self.instance.setVcpu(str(cpu_id), enabled) + """Turns a hotpluggable vCPU of the active VM on or off, now and for its next start.""" + self.instance.setVcpu(str(cpu_id), enabled, VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG) def set_vcpu_hotplug(self, status, vcpus_hotplug=0): """vcpus_hotplug: the vCPUs enabled at boot, 0 keeps the current ones; diff --git a/vrtManager/tests/test_device_edits.py b/vrtManager/tests/test_device_edits.py index a653121b..0d0a9fbb 100644 --- a/vrtManager/tests/test_device_edits.py +++ b/vrtManager/tests/test_device_edits.py @@ -70,6 +70,11 @@ class DetachDiskTestCase(unittest.TestCase): class VcpuHotplugTestCase(unittest.TestCase): + def test_a_vcpu_turned_on_or_off_stays_so_at_the_next_start(self): + p = proxy() + p.set_vcpu("1", 1) + p.instance.setVcpu.assert_called_once_with("1", 1, VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG) + def test_enabling_hotplug_keeps_the_current_vcpus_enabled(self): p = proxy(active=False) p.instance.info.return_value = [5]