j3d1/webvirtcloud
1
0
Fork 0
mirror of https://github.com/retspen/webvirtcloud synced 2026-11-09 03:36:48 +00:00
Code Issues Projects Releases Wiki Activity

fix(instances): device edits fit every definition and device type

Browse source 
A volume of an LVM or iSCSI pool can be attached: by its own path, with
the format its volume reports (raw when the pool knows none). Detaching
a disk takes each definition's own description of it, so a disk only in
the persistent definition goes too. Every NIC type can be a boot device;
bridge and direct NICs were dropped from the boot order. The video model
is a value, never parsed as XML. QoS changes clear the page's read cache.

The NIC model "default" leaves the choice to libvirt instead of writing
model "default", and rt18139 is rtl8139: a VM with either did not start
(checked on a RHEL 10 host). A migration corrects the default NIC type
setting.
This commit is contained in:
catborise 2026-10-09 17:33:09 +03:00
parent 769a0dc96f
commit 11fe2409f9
12 changed files with 250 additions and 48 deletions
Download patch file Download diff file Expand all files Collapse all files

1
README.md
View file

@ -538,6 +538,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.
- 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 running VM without a guest agent shows the IPv4 address the host learned by ARP (the lookup always failed before).
- After a live migration the VM's NVRAM file is deleted on the source, when it is in libvirt's `/var/lib/libvirt/qemu/nvram` and the destination does not share that directory (a VM created there later with the same name would start with those UEFI variables).
- gstfsd finds the guest's operating system the way libguestfs inspects it, so a root on LVM works, and it always answers (an error when it finds no or several operating systems). Update `/usr/local/bin/gstfsd` on every compute (see [Adding a Compute Node](#adding-a-compute-node), step 3).

22
appsettings/migrations/0016_nic_type_rtl8139.py Normal file
View file

@ -0,0 +1,22 @@
from django.db import migrations
def fix_rtl8139(apps, schema_editor):
"""The NIC type is rtl8139: a VM with model rt18139 does not start. Not
undone on the way back, as nothing needs the misspelling."""
AppSettings = apps.get_model("appsettings", "AppSettings")
for setting in AppSettings.objects.using(schema_editor.connection.alias).filter(key="INSTANCE_NIC_DEFAULT_TYPE"):
setting.choices = setting.choices.replace("rt18139", "rtl8139")
if setting.value == "rt18139":
setting.value = "rtl8139"
setting.save()
class Migration(migrations.Migration):
dependencies = [
("appsettings", "0015_remove_clone_instance_auto_migrate"),
]
operations = [
migrations.RunPython(fix_rtl8139, migrations.RunPython.noop),
]

16
appsettings/test_migrations.py
View file

@ -4,10 +4,11 @@ an id must not collide with them."""
import importlib
import unittest
from types import SimpleNamespace
from django.apps import apps
from django.db import connection
from django.test import TransactionTestCase
from django.test import TestCase, TransactionTestCase
from appsettings.models import AppSettings
@ -26,3 +27,16 @@ class SequenceAfterFixedIdsTestCase(TransactionTestCase):
cursor.execute("SELECT setval(pg_get_serial_sequence('appsettings_appsettings', 'id'), 1, false)")
login_lockout.add_settings(apps, schema_editor)
self.assertEqual(AppSettings.objects.filter(key__startswith="LOGIN_").count(), 2)
nic_type = importlib.import_module("appsettings.migrations.0016_nic_type_rtl8139")
class NicTypeTestCase(TestCase):
def test_the_misspelt_nic_type_becomes_rtl8139(self):
setting = AppSettings.objects.get(key="INSTANCE_NIC_DEFAULT_TYPE")
self.assertEqual(setting.choices, "default,e1000,e1000e,rtl8139,virtio")
AppSettings.objects.filter(pk=setting.pk).update(value="rt18139", choices="default,e1000,e1000e,rt18139,virtio")
nic_type.fix_rtl8139(apps, SimpleNamespace(connection=connection))
setting.refresh_from_db()
self.assertEqual((setting.value, setting.choices), ("rtl8139", "default,e1000,e1000e,rtl8139,virtio"))

47
instances/test_disk_isolation.py
View file

@ -191,6 +191,34 @@ class DiskViewsTenantIsolationTestCase(TestCase):
self.assertEqual(res.status_code, 302)
proxy.attach_disk.assert_not_called()
def test_add_existing_vol_attaches_the_volume_by_its_own_path(self):
self.client.force_login(self.superuser)
with patch("instances.models.wvmInstance") as mock_wvm, patch(
"instances.views.wvmStorage"
) as mock_storage:
proxy = mock_wvm.return_value
proxy.get_disk_devices.return_value = [VM_DISK]
proxy.get_media_devices.return_value = []
proxy.get_disk_bus_types.return_value = ["virtio", "sata", "scsi"]
proxy.get_cache_modes.return_value = {"default": "", "none": ""}
pool = mock_storage.return_value
pool.get_volumes.return_value = ["unit:0:0:1"]
pool.get_type.return_value = "iscsi"
pool.get_volume_type.return_value = "block"
pool.get_volume_format_type.return_value = None # an iSCSI pool knows no format
pool.get_target_path.return_value = "/dev/disk/by-path"
pool.get_volume.return_value.path.return_value = "/dev/disk/by-path/ip-192.0.2.1:3260-iscsi-iqn.example-lun-1"
res = self._post(
"add_existing_vol",
{"selected_storage": "iscsi", "vols": "unit:0:0:1", "bus": "virtio", "cache": "default"},
)
self.assertEqual(res.status_code, 302)
pool.get_volume.assert_called_once_with("unit:0:0:1")
self.assertEqual(
proxy.attach_disk.call_args.args[1], "/dev/disk/by-path/ip-192.0.2.1:3260-iscsi-iqn.example-lun-1"
)
self.assertEqual(proxy.attach_disk.call_args.kwargs["format_type"], "raw")
def _mock_disk_options(self, proxy):
proxy.get_disk_devices.return_value = [VM_DISK]
proxy.get_media_devices.return_value = []
@ -299,6 +327,25 @@ class DiskViewsTenantIsolationTestCase(TestCase):
)
mock_create.return_value.create_volume.assert_not_called()
def test_add_new_vol_attaches_the_format_the_pool_created(self):
self.client.force_login(self.superuser)
with patch("instances.models.wvmInstance") as mock_wvm, patch(
"instances.views.wvmCreate"
) as mock_create, patch("instances.views.wvmStorage") as mock_storage:
proxy = mock_wvm.return_value
self._mock_disk_options(proxy)
mock_create.return_value.create_volume.return_value = "/mnt/nfs/disk.img"
pool = mock_storage.return_value
pool.get_type.return_value = "netfs"
pool.get_volume_type.return_value = "file"
pool.get_volume_format_type.return_value = "raw" # a netfs pool creates raw
self._post(
"add_new_vol",
{"storage": "nfs", "name": "disk", "format": "qcow2", "size": "1", "bus": "virtio", "cache": "default"},
)
pool.get_volume_format_type.assert_called_once_with("disk.img")
self.assertEqual(proxy.attach_disk.call_args.kwargs["format_type"], "raw")
def test_edit_volume_accepts_a_disk_without_any_driver_format(self):
self.client.force_login(self.superuser)
no_format_disk = dict(VM_DISK, format=None)

11
instances/views.py
View file

@ -1013,6 +1013,8 @@ def add_new_vol(request, pk):
pool_type = conn_pool.get_type()
disk_type = conn_pool.get_volume_type(os.path.basename(source))
# the pool decides: only a dir pool keeps the asked format, the others create raw
format = util.vol_driver_type(conn_pool.get_volume_format_type(os.path.basename(source)), pool_type)
if pool_type == "rbd":
source_info = conn_pool.get_rbd_source()
@ -1067,18 +1069,19 @@ def add_existing_vol(request, pk):
if name not in conn_create.get_volumes():
return reject_disk_options(request, pk, ["vols"])
format_type = conn_create.get_volume_format_type(name)
disk_type = conn_create.get_volume_type(name)
pool_type = conn_create.get_type()
format_type = util.vol_driver_type(conn_create.get_volume_format_type(name), pool_type)
disk_type = conn_create.get_volume_type(name)
if pool_type == "rbd":
source_info = conn_create.get_rbd_source()
path = conn_create.get_source_name()
source = f"{path}/{name}"
else:
source_info = None
path = conn_create.get_target_path()
# an iSCSI volume's path is not the pool target plus its name
source = conn_create.get_volume(name).path()
target_dev = utils.get_new_disk_dev(media, disks, bus)
source = f"{path}/{name}"
instance.proxy.attach_disk(
target_dev,

2
vrtManager/connection.py
View file

@ -811,7 +811,7 @@ class wvmConnect(object):
"""
:return: network card models
"""
return ["default", "e1000", "e1000e","rt18139", "virtio"]
return ["default", "e1000", "e1000e", "rtl8139", "virtio"]
def get_image_formats(self):
"""

3
vrtManager/create.py
View file

@ -87,8 +87,7 @@ class wvmCreate(wvmConnect):
def get_volume_format_type(self, path):
vol = self.get_volume_by_path(path)
vol_type = util.get_xml_path(vol.XMLDesc(0), "/volume/target/format/@type")
return "raw" if vol_type in ["unknown", "iso"] else vol_type or "raw"
return util.vol_driver_type(util.get_xml_path(vol.XMLDesc(0), "/volume/target/format/@type"))
def get_volume_path(self, volume, pool=None):
storages = [pool] if pool else self.get_storages(only_actives=True)

67
vrtManager/instance.py
View file

@ -950,26 +950,14 @@ class wvmInstance(wvmConnect):
for idx, dev in devorder.items():
order = etree.fromstring("<boot order='{}'/>".format(idx + 1))
if dev["type"] == "disk":
devices = tree.findall("./devices/disk[@device='disk']")
for d in devices:
device = d.find("./target[@dev='{}']".format(dev["dev"]))
if device is not None:
d.append(order)
elif dev["type"] == "cdrom":
devices = tree.findall("./devices/disk[@device='cdrom']")
for d in devices:
device = d.find("./target[@dev='{}']".format(dev["dev"]))
if device is not None:
d.append(order)
elif dev["type"] == "network":
devices = tree.findall("./devices/interface[@type='network']")
for d in devices:
device = d.find("mac[@address='{}']".format(dev["dev"]))
if device is not None:
d.append(order)
if dev["type"] in ("disk", "cdrom"):
devices = tree.xpath("./devices/disk[@device=$device][target/@dev=$dev]", device=dev["type"], dev=dev["dev"])
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")
for d in devices:
d.append(order)
self._defineXML(etree.tostring(tree).decode())
def _set_cdrom_media(self, dev, path):
@ -1068,6 +1056,8 @@ class wvmInstance(wvmConnect):
if disk_type == "file":
xml_disk += f"<source file='{source}'/>"
elif disk_type == "block": # a volume of an LVM, disk or iSCSI pool
xml_disk += f"<source dev='{source}'/>"
elif disk_type == "network":
if pool_type == "rbd":
auth_type = source_info.get("auth_type")
@ -1100,17 +1090,16 @@ class wvmInstance(wvmConnect):
self.instance.attachDeviceFlags(xml_disk, affect)
def detach_disk(self, target_dev):
tree = etree.fromstring(self._XMLDesc(0))
disk_el = tree.xpath("./devices/disk/target[@dev=$dev]", dev=target_dev)[
0
].getparent()
xml_disk = etree.tostring(disk_el).decode()
# Paused and other active states need the live detach too.
if self.instance.isActive():
self.instance.detachDeviceFlags(xml_disk, VIR_DOMAIN_AFFECT_LIVE)
self.instance.detachDeviceFlags(xml_disk, VIR_DOMAIN_AFFECT_CONFIG)
"""Detach target_dev from each definition that has it, as that definition describes it."""
found = False
for xml_flags, affect in self._definitions():
tree = etree.fromstring(self._XMLDesc(xml_flags))
disks = tree.xpath("./devices/disk[target/@dev=$dev]", dev=target_dev)
if disks:
found = True
self.instance.detachDeviceFlags(etree.tostring(disks[0]).decode(), affect)
if not found:
raise util.OperationError(f"Disk {target_dev} is not in the VM's definition")
def has_disk(self, target_dev):
"""True while the live or the persistent definition still has target_dev."""
@ -1502,13 +1491,13 @@ class wvmInstance(wvmConnect):
xml = self._XMLDesc(PERSISTENT_XML)
tree = etree.fromstring(xml)
video_models = tree.xpath("/domain/devices/video/model")
video_xml = "<model type='{}'/>".format(model)
for model in video_models:
if model.get("primary") == "yes" or len(video_models) == 1:
parent = model.getparent()
parent.remove(model)
parent.append(etree.fromstring(video_xml))
for video_model in video_models:
if video_model.get("primary") == "yes" or len(video_models) == 1:
parent = video_model.getparent()
parent.remove(video_model)
etree.SubElement(parent, "model", type=model)
self._defineXML(etree.tostring(tree).decode())
return
def resize_cpu(self, cur_vcpu, vcpu):
"""
@ -1968,7 +1957,7 @@ class wvmInstance(wvmConnect):
etree.SubElement(iface, "source", dev=source, mode="bridge")
else:
etree.SubElement(iface, "source", bridge=source)
if model:
if model and model != "default": # default: no model, libvirt chooses
etree.SubElement(iface, "model", type=model)
if nwfilter:
etree.SubElement(iface, "filterref", filter=nwfilter)
@ -2027,7 +2016,7 @@ class wvmInstance(wvmConnect):
iface.find("mac").set("address", mac)
for old_model in iface.findall("model"):
iface.remove(old_model)
if model:
if model and model != "default": # default: no model, libvirt chooses
etree.SubElement(iface, "model", type=model)
for old_filter in iface.findall("filterref"):
iface.remove(old_filter)
@ -2204,7 +2193,7 @@ class wvmInstance(wvmConnect):
else:
band.append(etree.fromstring(xml))
new_xml = etree.tostring(tree).decode()
self.wvm.defineXML(new_xml)
self._defineXML(new_xml)
def unset_qos(self, mac, direction):
tree = etree.fromstring(self._XMLDesc(PERSISTENT_XML))
@ -2219,7 +2208,7 @@ class wvmInstance(wvmConnect):
if parent_mac[0] == mac:
band_el.remove(direct)
self.wvm.defineXML(etree.tostring(tree).decode())
self._defineXML(etree.tostring(tree).decode())
def add_guest_agent(self):
channel_xml = """

110
vrtManager/test_device_edits.py Normal file
View file

@ -0,0 +1,110 @@
"""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."""
import unittest
from unittest.mock import MagicMock
from django.conf import settings
if not settings.configured:
settings.configure(MAC_OUI="52:54:10")
from lxml import etree
from vrtManager import util
from vrtManager.instance import PERSISTENT_XML, wvmInstance
from libvirt import VIR_DOMAIN_AFFECT_CONFIG, VIR_DOMAIN_AFFECT_LIVE, VIR_DOMAIN_XML_SECURE
PERSISTENT = """<domain><os><type>hvm</type></os><devices>
<disk type='file' device='disk'><source file='/pool/a.qcow2'/><target dev='vda' bus='virtio'/></disk>
<disk type='file' device='disk'><source file='/pool/b.qcow2'/><target dev='vdb' bus='virtio'/></disk>
<interface type='bridge'><mac address='52:54:10:00:00:02'/><source bridge='br0'/></interface>
<interface type='network'><mac address='52:54:10:00:00:01'/><source network='default'/>
<bandwidth><inbound average='1' peak='2' burst='3'/></bandwidth></interface>
<video><model type='vga'/></video>
</devices></domain>"""
LIVE = """<domain><devices>
<disk type='file' device='disk'><source file='/pool/a.qcow2'/><target dev='vda' bus='virtio'/><alias name='virtio-disk0'/></disk>
</devices></domain>"""
def proxy(active=True):
p = wvmInstance.__new__(wvmInstance) # no connection: only these calls
p.instance = MagicMock(**{"isActive.return_value": active})
p._XMLDesc = lambda flags: PERSISTENT if flags == PERSISTENT_XML else LIVE
p._defineXML = MagicMock()
p.wvm = MagicMock()
return p
def defined(p):
(xml,), _ = p._defineXML.call_args
return etree.fromstring(xml)
class DetachDiskTestCase(unittest.TestCase):
def test_each_definition_is_detached_as_it_describes_the_disk(self):
p = proxy()
p.detach_disk("vda")
(live, live_flags), (config, config_flags) = (c.args for c in p.instance.detachDeviceFlags.call_args_list)
self.assertEqual((live_flags, config_flags), (VIR_DOMAIN_AFFECT_LIVE, VIR_DOMAIN_AFFECT_CONFIG))
self.assertIn("virtio-disk0", live)
self.assertNotIn("alias", config)
def test_a_disk_only_in_the_persistent_definition_is_detached_there(self):
p = proxy()
p.detach_disk("vdb")
self.assertEqual([c.args[1] for c in p.instance.detachDeviceFlags.call_args_list], [VIR_DOMAIN_AFFECT_CONFIG])
def test_an_unknown_disk_is_refused(self):
p = proxy()
with self.assertRaises(util.OperationError):
p.detach_disk("vdz")
p.instance.detachDeviceFlags.assert_not_called()
class DeviceEditsTestCase(unittest.TestCase):
def test_a_block_volume_is_attached_by_its_device_path(self):
p = proxy(active=False)
p._definitions = lambda: [(PERSISTENT_XML, VIR_DOMAIN_AFFECT_CONFIG)]
p.attach_disk("vdc", "/dev/vg/c", disk_type="block", target_bus="virtio")
xml, _ = p.instance.attachDeviceFlags.call_args.args
self.assertEqual(etree.fromstring(xml).find("source").attrib, {"dev": "/dev/vg/c"})
def test_a_bridge_nic_can_be_a_boot_device(self):
p = proxy()
p.set_bootorder({0: {"type": "network", "dev": "52:54:10:00:00:02"}, 1: {"type": "disk", "dev": "vdb"}})
tree = defined(p)
self.assertEqual(tree.xpath("devices/interface[@type='bridge']/boot/@order"), ["1"])
self.assertEqual(tree.xpath("devices/disk[target/@dev='vdb']/boot/@order"), ["2"])
def test_the_video_model_is_a_value(self):
p = proxy()
p.set_video_model("vga' heads='9")
model = defined(p).find("devices/video/model")
self.assertEqual(model.attrib, {"type": "vga' heads='9"})
p._defineXML.assert_called_once()
def test_qos_changes_go_through_define_xml(self):
for change in (
lambda p: p.set_qos("52:54:10:00:00:01", "outbound", 1, 2, 3),
lambda p: p.unset_qos("52:54:10:00:00:01", "inbound"),
):
p = proxy()
change(p)
p._defineXML.assert_called_once()
p.wvm.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", "")
self.assertIsNone(defined(p).find("devices/interface[@type='network']/model"))
p = proxy(active=False)
p.add_network("52:54:10:00:00:09", "default", model="default")
xml, _ = p.instance.attachDeviceFlags.call_args.args
self.assertIsNone(etree.fromstring(xml).find("model"))
if __name__ == "__main__":
unittest.main()

8
vrtManager/test_util.py
View file

@ -17,6 +17,7 @@ from vrtManager.util import (
validate_macaddr,
validate_uuid,
vol_dev_type,
vol_driver_type,
xml_escape,
)
@ -128,6 +129,13 @@ class TestVrtManagerUtil(unittest.TestCase):
self.assertEqual(vol_dev_type("sata"), "sd")
self.assertIsNone(vol_dev_type("unknown_bus"))
def test_vol_driver_type(self):
for vol_format in ("qcow2", "qcow", "raw"):
self.assertEqual(vol_driver_type(vol_format), vol_format)
for vol_format in (None, "", "none", "unknown", "iso"):
self.assertEqual(vol_driver_type(vol_format), "raw")
self.assertEqual(vol_driver_type("linux-lvm", "disk"), "raw") # a partition
class OperationErrorTestCase(unittest.TestCase):

3
vrtManager/test_xml_safety.py
View file

@ -6,6 +6,7 @@ from django.conf import settings
if not settings.configured:
settings.configure(MAC_OUI="52:54:10")
from vrtManager import util
from vrtManager.instance import wvmInstance
from vrtManager.network import wvmNetworks, wvmNetwork
@ -291,7 +292,7 @@ class TestDiskXmlEscaping(unittest.TestCase):
"</disk></devices></domain>"
)
)
with self.assertRaises(IndexError):
with self.assertRaises(util.OperationError):
inst.detach_disk("x' or '1'='1")
inst.instance.detachDeviceFlags.assert_not_called()

8
vrtManager/util.py
View file

@ -216,3 +216,11 @@ def vol_dev_type(vol_bus):
elif vol_bus in ["scsi", "sata", "usb"]:
return "sd"
def vol_driver_type(vol_format, pool_type=None):
"""The disk driver type for a volume of vol_format: raw for a volume
whose pool knows no image format of it (none, unknown, an ISO, a disk
pool's partition), as libvirt uses a disk without a driver type."""
if pool_type == "disk" or vol_format in (None, "", "none", "unknown", "iso"):
return "raw"
return vol_format