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.

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):

instances/templates/instances/settings_tab.html

@@ -511,7 +511,7 @@
             </div>
             </div>
 
-            {% if instance.qos %}
+            {% if instance.config_qos %}
             <div class="row mt-3">
                 <div class="col-sm-10">
                     <p><strong>{% trans "QoS Configuration" %}</strong></p>
@@ -528,7 +528,7 @@
                         </tr>
                         </thead>
                         <tbody>
-                        {% for q, attrs in instance.qos.items %}
+                        {% for q, attrs in instance.config_qos.items %}
                             {% for att in attrs  %}
                                 <tr class="d-flex">
                                     <form action="{% url 'instances:set_qos' instance.id %}" method="post" role="form" aria-label="Instance QoS configuration form">

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"<interface type='network'><mac address='{mac}'/><source network='default'/>"
+               "<bandwidth><inbound average='500' peak='0' burst='0'/></bandwidth></interface>")
+        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)
+

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:

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()