From f8e30d76686af3e574dc275135083353b715d9a9 Mon Sep 17 00:00:00 2001 From: catborise Date: Fri, 30 Oct 2026 18:16:11 +0300 Subject: [PATCH] feat(instances): a VM's resize tab as a ledger, its vCPU changes kept The Resize tab's CPU, Memory and Disk panes were horizontal forms with bold host figures. The owner chose the ledger layout from three rendered options: each section with its heading, the VM's figures and the host's on the left, its fields on the right, and one line on what applies now and what at the next start. Codex reviewed resizing end to end; these are fixed: - A vCPU turned on or off while the VM runs is kept at its next start: libvirt was asked without flags, which changes the running VM only. - A paused VM offers to turn vCPUs on only: turning one off needs the guest, which does not run. - A smaller disk size is refused, and so is the rest of the form, before anything grows; it was skipped without a word. - vCPUs and memory below 1 are refused. - The memory fields are numbers filled with the VM's values: the "Custom value" box, its script and the preset list are gone; an empty custom field silently used the preset. - Units and words: MiB, not MB; a disk's field is its new size, with its size now above it; the messages say what the VM gets. --- instances/templates/instance.html | 11 - instances/templates/instances/resize_tab.html | 297 +++++++++--------- instances/tests/test_live_guest.py | 3 + instances/tests/test_missing_fields.py | 2 +- instances/tests/test_pending_edits.py | 13 + instances/tests/test_resize_input.py | 26 +- instances/tests/test_snapshot_views.py | 5 +- instances/tests/test_views.py | 2 +- instances/views.py | 40 +-- vrtManager/instance.py | 3 +- vrtManager/tests/test_device_edits.py | 5 + 11 files changed, 214 insertions(+), 193 deletions(-) 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]