From ea9424ee1ffe9488effde5e20cbda3d5bb2ad96b Mon Sep 17 00:00:00 2001 From: catborise Date: Sat, 10 Oct 2026 22:59:14 +0300 Subject: [PATCH] fix(instances): a running VM's QoS edit keeps the pending QoS changes The QoS table showed the running VM's bandwidth while an edit changes the definition for the next start. Each row posts average, peak and burst, so an edit wrote the running values back over a pending change, and a direction added while the VM ran had no row. The table shows the persistent definition now. An edit also kept only the posted values and dropped an inbound floor; it now updates them and keeps the rest. --- README.md | 2 +- instances/models.py | 5 ++-- .../templates/instances/settings_tab.html | 4 +-- instances/test_live_dataloss.py | 25 +++++++++++++++++++ instances/test_pending_edits.py | 12 ++++++++- instances/tests.py | 10 ++++---- vrtManager/instance.py | 14 ++++------- vrtManager/test_device_edits.py | 12 +++++++-- 8 files changed, 62 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 4133f19a..9f0c7b22 100644 --- a/README.md +++ b/README.md @@ -540,7 +540,7 @@ A rollback leaves the clone on a detached HEAD at the restored commit; the updat - A VM's stats are polled every 5 s like the host graph, the first point as soon as the tab opens; each point is the average since the previous one, disk rates in MB/s (they were MiB/s labelled Mb/s); a counter the host does not report shows a gap instead of 0. - An ISO uploads to an SSH compute about 3.5 times faster (SFTP writes no longer wait for each 32 KiB); a failed write still fails the upload. - On a host whose libvirt cannot change network interfaces (the udev backend: RHEL 9 and later, Debian 12), the interface pages offer no create, start, stop or delete; they only failed there. -- On a running VM the disk list and its edit form show the disks as they will be at the next start, so saving an edit no longer reverts another pending disk change. +- On a running VM the disk list, its edit form and the QoS table show the settings as they will be at the next start, so saving an edit no longer reverts another pending change. A QoS edit keeps the inbound floor. - The VM page shows each NIC's own QoS and each CD-ROM's own pool (they showed the previous device's), and a NIC without a source or model no longer breaks it. - A volume of an LVM or iSCSI pool can be attached to a VM, and bridge and direct NICs can be boot devices. The NIC model `default` lets libvirt choose, and `rt18139` is `rtl8139`: a VM with either did not start; a migration corrects the default NIC type setting. - A paused VM's memory is resized as a running VM's: the current memory changes and the guest takes it once resumed, the maximum stays (both only reached the VM at its next boot). diff --git a/instances/models.py b/instances/models.py index 06cd112d..a516f835 100644 --- a/instances/models.py +++ b/instances/models.py @@ -162,8 +162,9 @@ class Instance(models.Model): return self.proxy.get_disk_devices(config=True) @cached_property - def qos(self): - return self.proxy.get_all_qos() + def config_qos(self): + """QoS of the persistent definition, which the QoS form changes.""" + return self.proxy.get_all_qos(config=True) @cached_property def telnet_port(self): diff --git a/instances/templates/instances/settings_tab.html b/instances/templates/instances/settings_tab.html index 5f92a734..40405f41 100644 --- a/instances/templates/instances/settings_tab.html +++ b/instances/templates/instances/settings_tab.html @@ -511,7 +511,7 @@ - {% if instance.qos %} + {% if instance.config_qos %}

{% trans "QoS Configuration" %}

@@ -528,7 +528,7 @@ - {% for q, attrs in instance.qos.items %} + {% for q, attrs in instance.config_qos.items %} {% for att in attrs %}
diff --git a/instances/test_live_dataloss.py b/instances/test_live_dataloss.py index 604a141a..0c9f44ef 100644 --- a/instances/test_live_dataloss.py +++ b/instances/test_live_dataloss.py @@ -376,3 +376,28 @@ class LiveDataLossTestCase(TestCase): self.assertEqual((disk.find("driver").get("cache"), disk.findtext("serial")), ("none", "wvc-serial")) self.assertEqual(dom.XMLDesc(0), live) + def test_pending_qos_change_is_shown_and_kept_by_the_next_edit(self): + from lxml import etree, html + + mac = "52:54:00:aa:05:11" + nic = (f"" + "") + dom, inst = self.vm("qos-pending", [], extra_devices=nic) + dom.create() + live = dom.XMLDesc(0) + + def row(): + page = html.fromstring(self.client.get(reverse("instances:instance", args=[inst.id])).content) + (label,) = page.xpath("//label[@class='col-form-label'][contains(., 'Inbound')]") + fields = label.xpath("ancestor::tr[1]//input[@name]") # the browser posts the row's inputs + return {f.get("name"): f.get("value") for f in fields} + + self.post("set_qos", inst, {**row(), "qos_average": "1000"}) + form = row() + self.assertEqual(form["qos_average"], "1000") + self.post("set_qos", inst, {**form, "qos_peak": "600"}) + + inbound = etree.fromstring(dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE)).find("devices/interface/bandwidth/inbound") + self.assertEqual((inbound.get("average"), inbound.get("peak")), ("1000", "600")) + self.assertEqual(dom.XMLDesc(0), live) + diff --git a/instances/test_pending_edits.py b/instances/test_pending_edits.py index 054226ca..1f9a39bf 100644 --- a/instances/test_pending_edits.py +++ b/instances/test_pending_edits.py @@ -26,7 +26,7 @@ class PendingEditsTestCase(TestCase): compute = Compute.objects.create(name="pe", hostname="127.0.0.1:1", login="root", password="", type=1) self.instance = Instance.objects.create(compute=compute, name="pe-vm", uuid="aaaaaaaa-1111-2222-3333-444444444444") - def page(self, live, config): # disks of the live and the persistent definition + def page(self, live=(), config=(), qos=({}, {})): # disks and QoS of the live and the persistent definition with patch("instances.models.wvmInstance") as wvm, patch.object( Compute, "status", new_callable=PropertyMock, return_value=True ): @@ -41,6 +41,7 @@ class PendingEditsTestCase(TestCase): "get_snapshot", "get_external_snapshots"): getattr(proxy, getter).return_value = [] proxy.get_disk_devices.side_effect = lambda **kw: config if kw.get("config") else live + proxy.get_all_qos.side_effect = lambda **kw: qos[1] if kw.get("config") else qos[0] proxy.get_console_type.return_value = "vnc" proxy.get_cache_modes.return_value = {"default": "Default", "none": "Disabled"} response = self.client.get(reverse("instances:instance", args=[self.instance.id])) @@ -59,6 +60,15 @@ class PendingEditsTestCase(TestCase): self.assertEqual(vda.xpath(".//select[@name='vol_cache']/option[@selected]/@value"), ["none"]) self.assertEqual(vda.xpath("string(.//input[@name='vol_serial']/@value)"), "") + def test_the_qos_form_shows_the_pending_definition(self): + mac = "52:54:10:00:00:01" + inbound = {"direction": "inbound", "average": "500", "peak": "0", "burst": "0", "floor": None} + doc = self.page(qos=({mac: [inbound]}, {mac: [{**inbound, "average": "1000"}, {**inbound, "direction": "outbound"}]})) + self.assertEqual(doc.xpath("//input[@name='qos_average']/@value"), ["1000", "500"]) + # the pending outbound has a row + self.assertEqual([" ".join(t.split()) for t in doc.xpath("//label[@class='col-form-label']/text()")], + [f"{mac} Inbound", f"{mac} Outbound"]) + def test_a_disk_with_a_pending_source_is_not_deleted(self): # running: vda is A, its definition for the next start uses B with patch("instances.models.wvmInstance") as wvm, patch("instances.views.wvmStorage") as storage: diff --git a/instances/tests.py b/instances/tests.py index bd7ebf89..f1903cd3 100644 --- a/instances/tests.py +++ b/instances/tests.py @@ -1027,7 +1027,7 @@ class InstancesTestCase(TestCase): self.assertEqual(self.instance.networks[0]["state"], "down") def test_set_unset_qos(self): - self.assertEqual(len(self.instance.qos.keys()), 0) + self.assertEqual(len(self.instance.config_qos.keys()), 0) net_mac = self.instance.networks[0]["mac"] response = self.client.post( reverse("instances:set_qos", args=[self.instance.id]), @@ -1042,8 +1042,8 @@ class InstancesTestCase(TestCase): ) self.assertEqual(response.status_code, 302) - del self.instance.qos - self.assertEqual(len(self.instance.qos.keys()), 1) + del self.instance.config_qos + self.assertEqual(len(self.instance.config_qos.keys()), 1) response = self.client.post( reverse("instances:unset_qos", args=[self.instance.id]), @@ -1054,8 +1054,8 @@ class InstancesTestCase(TestCase): HTTP_REFERER=reverse("index"), ) self.assertEqual(response.status_code, 302) - del self.instance.qos - self.assertEqual(len(self.instance.qos.keys()), 0) + del self.instance.config_qos + self.assertEqual(len(self.instance.config_qos.keys()), 0) # test on running instance # self.instance.proxy.start() diff --git a/vrtManager/instance.py b/vrtManager/instance.py index 8883c7d2..0e50de3e 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -1979,9 +1979,9 @@ class wvmInstance(wvmConnect): def set_memory(self, size, flags=0): self.instance.setMemoryFlags(size, flags) - def get_all_qos(self): + def get_all_qos(self, config=False): qos_values = dict() - tree = etree.fromstring(self._XMLDesc(0)) + tree = etree.fromstring(self._XMLDesc(PERSISTENT_XML if config else 0)) qos = tree.xpath("/domain/devices/interface") for q in qos: @@ -2029,10 +2029,7 @@ class wvmInstance(wvmConnect): peak = int(peak) burst = int(burst) - if direction == "inbound": - xml = f"" - else: - xml = f"" + xml = f"<{direction} average='{average}' peak='{peak}' burst='{burst}'/>" tree = etree.fromstring(self._XMLDesc(PERSISTENT_XML)) @@ -2048,9 +2045,8 @@ class wvmInstance(wvmConnect): else: direct = band.find(direction) if direct is not None: - parent = direct.getparent() - parent.remove(direct) - parent.append(etree.fromstring(xml)) + # keep what the form does not post (an inbound floor) + direct.attrib.update(etree.fromstring(xml).attrib) else: band.append(etree.fromstring(xml)) new_xml = etree.tostring(tree).decode() diff --git a/vrtManager/test_device_edits.py b/vrtManager/test_device_edits.py index 44512ad4..85773c04 100644 --- a/vrtManager/test_device_edits.py +++ b/vrtManager/test_device_edits.py @@ -1,6 +1,6 @@ """Device edits of a VM: each definition is changed as it describes the device, every NIC type can be a boot device, posted values stay values, and -definition changes go through _defineXML. An active VM's memory is resized +definition changes go through _defineXML, a QoS edit keeps the floor. An active VM's memory is resized by balloon, a stopped one's in its definition. Enabling vCPU hot plug keeps the vCPUs enabled at boot.""" @@ -23,7 +23,7 @@ PERSISTENT = """hvm - + """ LIVE = """ @@ -107,6 +107,14 @@ class DeviceEditsTestCase(unittest.TestCase): p._defineXML.assert_called_once() p.wvm.defineXML.assert_not_called() + def test_qos_is_read_from_either_definition_and_an_edit_keeps_the_floor(self): + p = proxy() + self.assertEqual(p.get_all_qos(config=True)["52:54:10:00:00:01"][0]["floor"], "4") + self.assertEqual(p.get_all_qos(), {}) # the running VM has none + p.set_qos("52:54:10:00:00:01", "inbound", 10, 20, 30) + inbound = defined(p).find("devices/interface[@type='network']/bandwidth/inbound") + self.assertEqual(inbound.attrib, {"average": "10", "peak": "20", "burst": "30", "floor": "4"}) + def test_the_default_nic_model_leaves_the_choice_to_libvirt(self): p = proxy() p.change_network("52:54:10:00:00:01", "52:54:10:00:00:01", "default", "net", "default", "")