diff --git a/README.md b/README.md index 7bdefccd..4ddc3d63 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, its edit form, the QoS table, the title, description and CPU fields 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. Buttons shown as disabled while the VM runs can no longer be submitted with Enter. +- On a running VM the disk list, its edit form, the QoS table, the title, description, CPU, boot order, video and console fields show the settings as they will be at the next start, so saving an edit no longer reverts another pending change. A boot order that names a device missing from that definition is refused instead of clearing the order. A QoS edit keeps the inbound floor. Buttons shown as disabled while the VM runs can no longer be submitted with Enter. - 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 3ee0414d..f49d500b 100644 --- a/instances/models.py +++ b/instances/models.py @@ -185,6 +185,38 @@ class Instance(models.Model): def config_description(self) -> str: return self.proxy.get_description(config=True) + @cached_property + def config_media(self) -> list[dict]: + """CD-ROMs, boot menu and boot order of the persistent definition, + which the boot order editor changes.""" + return self.proxy.get_media_devices(config=True) + + @cached_property + def config_bootmenu(self) -> bool: + return self.proxy.get_bootmenu(config=True) + + @cached_property + def config_boot_order(self) -> dict: + return self.proxy.get_bootorder(config=True) + + @cached_property + def config_video_model(self) -> str | None: + """Video model and console settings of the persistent definition, + which their forms change.""" + return self.proxy.get_video_model(config=True) + + @cached_property + def config_console_listener_address(self) -> str: + return self.proxy.get_console_listener_addr(config=True) + + @cached_property + def config_console_passwd(self): + return self.proxy.get_console_passwd(config=True) + + @cached_property + def config_console_keymap(self) -> str: + return self.proxy.get_console_keymap(config=True) + @cached_property def telnet_port(self): return self.proxy.get_telnet_port() diff --git a/instances/templates/instance.html b/instances/templates/instance.html index aebf4da7..58326c60 100644 --- a/instances/templates/instance.html +++ b/instances/templates/instance.html @@ -255,21 +255,21 @@ }); $(document).ready(function () { // set current console keymap or fall back to default - var keymap = "{{ instance.console_keymap }}"; + var keymap = "{{ instance.config_console_keymap }}"; if (keymap != '') { $("#console_select_keymap option[value='" + keymap + "']").prop('selected', true); } }); $(document).ready(function () { // set current console listen address or fall back to default - var console_listener_address = "{{ instance.console_listener_address }}"; + var console_listener_address = "{{ instance.config_console_listener_address }}"; if (console_listener_address != '') { $("#console_select_listener_address option[value='" + console_listener_address + "']").prop('selected', true); } }); $(document).ready(function () { // get video model or fall back to default - let video_model = "{{ instance.video_model }}"; + let video_model = "{{ instance.config_video_model }}"; if (video_model != '') { $("#video_model_select option[value='" + video_model + "']").prop('selected', true); } diff --git a/instances/templates/instances/settings_tab.html b/instances/templates/instances/settings_tab.html index 0dcead10..58e00cea 100644 --- a/instances/templates/instances/settings_tab.html +++ b/instances/templates/instances/settings_tab.html @@ -99,7 +99,7 @@
{% if instance.status == 5 %}

{% trans "Enable Boot Menu for your instance when it starts up " %} - {% if instance.bootmenu == 0 %} + {% if instance.config_bootmenu == 0 %}

{% csrf_token %}
@@ -109,17 +109,17 @@ {% endif %} {% else %} - {% if instance.bootmenu == 0 %} + {% if instance.config_bootmenu == 0 %}

**** {% trans "Please shutdown instance to modify boot menu" %} ****

{% endif %} {% endif %}
- {% if instance.bootmenu == 1 %} + {% if instance.config_bootmenu == 1 %}
- {% for idx, val in instance.boot_order.items %} + {% for idx, val in instance.config_boot_order.items %} {% endfor %}
@@ -128,13 +128,13 @@
- {% for disk in instance.disks %} + {% for disk in instance.config_disks %} {% endfor %} - {% for cd in instance.media %} + {% for cd in instance.config_media %} {% endfor %} - {% for net in instance.networks %} + {% for net in instance.config_networks %} {% endfor %}
@@ -857,7 +857,7 @@ {% if instance.status == 5 %} diff --git a/instances/test_live_dataloss.py b/instances/test_live_dataloss.py index 1655e73b..55b61398 100644 --- a/instances/test_live_dataloss.py +++ b/instances/test_live_dataloss.py @@ -401,6 +401,34 @@ class LiveDataLossTestCase(TestCase): ("wvc-title", "wvc-description", "2")) self.assertEqual(dom.XMLDesc(0), live) + def test_pending_boot_order_and_video_are_shown_and_a_live_only_choice_changes_nothing(self): + from lxml import etree, html + + nic = "" + graphics = "" + dom, inst = self.vm("boot-pending", [livetest.create_volume(self.conn, P + "boot-pending")], + extra_devices=nic + graphics) + self.post("set_bootmenu", inst) + dom.create() + # pending order; a NIC the running VM has but its next start will not + self.post("set_bootorder", inst, {"bootorder": "network:52:54:00:aa:05:21,disk:vda"}) + dom.attachDeviceFlags(nic.replace("05:21", "05:22"), libvirt.VIR_DOMAIN_AFFECT_LIVE) + persistent = lambda: etree.fromstring(dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE)) # noqa: E731 + video = "virtio" if persistent().find("devices/video/model").get("type") != "virtio" else "vga" + self.post("set_video_model", inst, {"video_model": video}) + live = dom.XMLDesc(0) + + page = html.fromstring(self.client.get(reverse("instances:instance", args=[inst.id])).content) + (boot,) = page.xpath("//form[@action=$a]", a=reverse("instances:set_bootorder", args=[inst.id])) + self.assertEqual(boot.xpath(".//input[@type='checkbox']/@value"), ["disk:vda", "network:52:54:00:aa:05:21"]) + self.assertEqual(page.xpath("//select[@name='video_model']/option[@selected]/@value"), [video]) + + self.post("set_bootorder", inst, {"bootorder": "network:52:54:00:aa:05:22,disk:vda"}) + tree = persistent() + self.assertEqual(tree.xpath("devices/interface/boot/@order"), ["1"]) + self.assertEqual(tree.xpath("devices/disk[target/@dev='vda']/boot/@order"), ["2"]) + 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 diff --git a/instances/test_pending_edits.py b/instances/test_pending_edits.py index ff1f55ea..5237b0b4 100644 --- a/instances/test_pending_edits.py +++ b/instances/test_pending_edits.py @@ -6,10 +6,12 @@ from unittest.mock import PropertyMock, patch from computes.models import Compute from django.contrib.auth import get_user_model +from django.contrib.messages import get_messages from django.test import TestCase from django.urls import reverse from instances.models import Instance from lxml import html +from vrtManager import util def disk(dev, cache, serial): @@ -48,6 +50,7 @@ class PendingEditsTestCase(TestCase): 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_video_models.return_value = ["vga", "virtio"] proxy.get_cache_modes.return_value = {"default": "Default", "none": "Disabled"} response = self.client.get(reverse("instances:instance", args=[self.instance.id])) self.assertEqual(response.status_code, 200) @@ -102,3 +105,28 @@ class PendingEditsTestCase(TestCase): with self.subTest(action): (form,) = doc.xpath("//form[@action=$a]", a=reverse(f"instances:{action}", args=[self.instance.id])) self.assertTrue(all(b.get("disabled") is not None for b in form.xpath(".//button"))) + + def test_the_boot_video_and_console_forms_show_the_pending_definition(self): + cd = {"dev": "sda", "image": "pending.iso", "storage": "default", "path": "/pool/pending.iso", "bus": "sata"} + with patch("instances.models.Instance.config_networks", new_callable=PropertyMock, return_value=[]): + doc = self.page( + live=[disk("vda", "default", "")], config=[disk("vda", "default", ""), disk("vdb", "default", "")], + get_bootmenu=(False, True), get_bootorder=({}, {0: {"type": "file", "dev": "cdrom", "target": "sda"}}), + get_media_devices=([], [cd]), get_video_model=("vga", "virtio"), + get_console_listener_addr=("127.0.0.1", "0.0.0.0"), get_console_keymap=("", "de"), + get_console_passwd=("live-pw", "pending-pw"), + ) + (boot,) = doc.xpath("//form[@action=$a]", a=reverse("instances:set_bootorder", args=[self.instance.id])) + self.assertEqual(boot.xpath(".//input[@type='checkbox']/@value"), ["disk:vda", "disk:vdb", "cdrom:sda"]) + self.assertIn("1:sda", doc.xpath("string(//div[label[contains(., ':')]])")) + self.assertEqual(doc.xpath("//select[@name='video_model']/option[@selected]/@value"), ["virtio"]) + self.assertEqual(doc.xpath("//select[@name='keymap']/option[@selected]/@value"), ["de"]) + self.assertEqual(doc.xpath("//select[@name='listen_on']/option[@selected]/@value"), ["0.0.0.0"]) + self.assertEqual(doc.xpath("//input[@name='password']/@value"), ["pending-pw"]) + + def test_a_boot_order_choice_not_in_the_definition_is_refused(self): + with patch("instances.models.wvmInstance") as wvm: + wvm.return_value.set_bootorder.side_effect = util.OperationError("Boot device vdz is not in the definition") + response = self.client.post(reverse("instances:set_bootorder", args=[self.instance.id]), + {"bootorder": "disk:vda,disk:vdz"}) + self.assertEqual([str(m) for m in get_messages(response.wsgi_request)], ["Boot device vdz is not in the definition"]) diff --git a/instances/views.py b/instances/views.py index 2e40f523..7c67a3c3 100755 --- a/instances/views.py +++ b/instances/views.py @@ -121,9 +121,9 @@ def _instance_page(request, pk, instance, compute): if can_manage_console: console_form = ConsoleForm( initial={ - "listen_on": instance.console_listener_address, - "password": instance.console_passwd, - "keymap": instance.console_keymap, + "listen_on": instance.config_console_listener_address, + "password": instance.config_console_passwd, + "keymap": instance.config_console_keymap, }, can_set_listener=request.user.is_superuser, ) @@ -1501,7 +1501,11 @@ def set_bootorder(request, pk): for idx, val in enumerate(bootorder.split(",")): dev_type, dev = val.split(":", 1) order_list[idx] = {"type": dev_type, "dev": dev} - instance.proxy.set_bootorder(order_list) + try: + instance.proxy.set_bootorder(order_list) + except util.OperationError as err: + messages.error(request, err) + return _back(request, pk, "boot_opt") msg = _("Set boot order") if not instance.proxy.get_status() == 5: diff --git a/vrtManager/instance.py b/vrtManager/instance.py index d6de968a..de19e26c 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -732,7 +732,7 @@ class wvmInstance(wvmConnect): return self._cached(("volume", path), read) - def get_media_devices(self): + def get_media_devices(self, config=False): def disks(doc): result = [] for media in doc.xpath("/domain/devices/disk[@device='cdrom']"): @@ -753,10 +753,10 @@ class wvmInstance(wvmConnect): ) return result - return util.get_xml_path(self._XMLDesc(0), func=disks) + return util.get_xml_path(self._XMLDesc(PERSISTENT_XML if config else 0), func=disks) - def get_bootmenu(self): - menu = util.get_xml_path(self._XMLDesc(0), "/domain/os/bootmenu/@enable") + def get_bootmenu(self, config=False): + menu = util.get_xml_path(self._XMLDesc(PERSISTENT_XML if config else 0), "/domain/os/bootmenu/@enable") return menu == "yes" def set_bootmenu(self, flag): @@ -783,10 +783,10 @@ class wvmInstance(wvmConnect): xmldom = etree.tostring(tree).decode() self._defineXML(xmldom) - def get_bootorder(self): + def get_bootorder(self, config=False): boot_order = {} dev_type = target = None - tree = ElementTree.fromstring(self._XMLDesc(0)) + tree = ElementTree.fromstring(self._XMLDesc(PERSISTENT_XML if config else 0)) os = tree.find("os") boot = os.findall("boot") @@ -858,7 +858,10 @@ class wvmInstance(wvmConnect): elif dev["type"] == "network": # any NIC type: network, bridge, direct devices = tree.xpath("./devices/interface[mac/@address=$mac]", mac=dev["dev"]) else: - raise Exception("Invalid Device Type for boot order") + raise util.OperationError("Invalid Device Type for boot order") + # nothing is defined until every choice is found: the order above is cleared + if not devices: + raise util.OperationError(f"Boot device {dev['dev']} is not in the definition") for d in devices: d.append(order) self._defineXML(etree.tostring(tree).decode()) @@ -1252,13 +1255,13 @@ class wvmInstance(wvmConnect): telnet_port = service_port return telnet_port - def get_console_listener_addr(self): + def get_console_listener_addr(self, config=False): listener_addr = util.get_xml_path( - self._XMLDesc(0), "/domain/devices/graphics/@listen" + self._XMLDesc(PERSISTENT_XML if config else 0), "/domain/devices/graphics/@listen" ) if listener_addr is None: listener_addr = util.get_xml_path( - self._XMLDesc(0), "/domain/devices/graphics/listen/@address" + self._XMLDesc(PERSISTENT_XML if config else 0), "/domain/devices/graphics/listen/@address" ) if listener_addr is None: return "127.0.0.1" @@ -1341,9 +1344,9 @@ class wvmInstance(wvmConnect): ) return websocket_port - def get_console_passwd(self): + def get_console_passwd(self, config=False): return util.get_xml_path( - self._XMLDesc(VIR_DOMAIN_XML_SECURE), "/domain/devices/graphics/@passwd" + self._XMLDesc(PERSISTENT_XML if config else VIR_DOMAIN_XML_SECURE), "/domain/devices/graphics/@passwd" ) def set_console_passwd(self, passwd): @@ -1374,17 +1377,17 @@ class wvmInstance(wvmConnect): newxml = etree.tostring(root).decode() self._defineXML(newxml) - def get_console_keymap(self): + def get_console_keymap(self, config=False): return ( util.get_xml_path( - self._XMLDesc(VIR_DOMAIN_XML_SECURE), "/domain/devices/graphics/@keymap" + self._XMLDesc(PERSISTENT_XML if config else VIR_DOMAIN_XML_SECURE), "/domain/devices/graphics/@keymap" ) or "" ) - def get_video_model(self): + def get_video_model(self, config=False): """:return only primary video card""" - xml = self._XMLDesc(VIR_DOMAIN_XML_SECURE) + xml = self._XMLDesc(PERSISTENT_XML if config else VIR_DOMAIN_XML_SECURE) tree = etree.fromstring(xml) video_models = tree.xpath("/domain/devices/video/model") for model in video_models: diff --git a/vrtManager/test_device_edits.py b/vrtManager/test_device_edits.py index 601a25f9..0764f088 100644 --- a/vrtManager/test_device_edits.py +++ b/vrtManager/test_device_edits.py @@ -1,6 +1,7 @@ """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, a QoS edit keeps the floor, and the CPU, title and description are read from either definition. An active VM's memory is resized +definition changes go through _defineXML, a QoS edit keeps the floor, and what the edit forms show is read from either definition. A boot order with a +missing device changes nothing. 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.""" @@ -18,15 +19,17 @@ from vrtManager import util from vrtManager.instance import PERSISTENT_XML, wvmInstance from libvirt import VIR_DOMAIN_AFFECT_CONFIG, VIR_DOMAIN_AFFECT_LIVE -PERSISTENT = """pending4hvm +PERSISTENT = """pending4hvm + + """ -LIVE = """1 +LIVE = """1hvm """ @@ -120,6 +123,25 @@ class DeviceEditsTestCase(unittest.TestCase): self.assertEqual((p.get_vcpu(), p.get_cur_vcpu(), p.get_title(), p.get_description()), (1, 1, "", "")) self.assertEqual((p.get_vcpu(config=True), p.get_cur_vcpu(config=True), p.get_title(config=True)), (4, 2, "pending")) + def test_boot_video_and_console_are_read_from_either_definition(self): + p = proxy() + live = (p.get_bootmenu(), p.get_bootorder(), p.get_media_devices(), p.get_video_model(), + p.get_console_listener_addr(), p.get_console_passwd(), p.get_console_keymap()) + self.assertEqual(live, (False, {}, [], None, "127.0.0.1", None, "")) + self.assertEqual((p.get_bootmenu(config=True), p.get_bootorder(config=True), p.get_video_model(config=True)), + (True, {0: {"type": "file", "dev": "cdrom", "target": "sda"}}, "vga")) + self.assertEqual([m["dev"] for m in p.get_media_devices(config=True)], ["sda"]) + self.assertEqual((p.get_console_listener_addr(config=True), p.get_console_passwd(config=True), + p.get_console_keymap(config=True)), ("0.0.0.0", "pending", "de")) + + def test_a_boot_order_with_a_device_not_in_the_definition_changes_nothing(self): + p = proxy() + for choice in ({"type": "disk", "dev": "vdz"}, {"type": "network", "dev": "52:54:10:00:00:99"}, + {"type": "floppy", "dev": "fda"}): + with self.subTest(choice), self.assertRaises(util.OperationError): + p.set_bootorder({0: {"type": "disk", "dev": "vda"}, 1: choice}) + p._defineXML.assert_not_called() + 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", "")