mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-08 19:26:48 +00:00
fix(instances): ISO mount/unmount for every VM state and CD-ROM
The old code edited the live XML, called attachDevice and redefined the domain from it: a stopped VM lost its VNC password, a paused VM crashed with UnboundLocalError, an unknown CD-ROM was silently ignored (on a running VM the last disk of the loop could be updated instead), and a missing ISO raised AttributeError. Media changes now use updateDeviceFlags on the requested CD-ROM, built from the live and the persistent definition separately. A CD-ROM missing from one definition, an unknown ISO, an ISO name present in more than one pool, or a change that reached only the running VM is reported to the user. The mount and unmount log messages were swapped.
This commit is contained in:
catborise
parent
08bf39ef61
commit
9f86330552
4 changed files with 207 additions and 54 deletions
96
instances/test_live_iso.py
Normal file
96
instances/test_live_iso.py
Normal file
|
|
@ -0,0 +1,96 @@
|
|||
"""Live checks for ISO mount/unmount (R-12). Runs only with TEST_LIBVIRT_HOST set."""
|
||||
|
||||
import os
|
||||
import unittest
|
||||
|
||||
import libvirt
|
||||
from django.test import SimpleTestCase
|
||||
from lxml import etree
|
||||
from vrtManager.connection import connection_manager
|
||||
from vrtManager.instance import wvmInstance
|
||||
|
||||
from . import livetest
|
||||
|
||||
P = livetest.PREFIX
|
||||
CDROMS = (
|
||||
"<disk type='file' device='cdrom'><driver name='qemu' type='raw'/><target dev='sda' bus='sata'/><readonly/></disk>"
|
||||
"<disk type='file' device='cdrom'><driver name='qemu' type='raw'/><target dev='sdb' bus='sata'/><readonly/></disk>"
|
||||
"<graphics type='vnc' autoport='yes' passwd='keep1234'/>"
|
||||
)
|
||||
PERSISTENT = libvirt.VIR_DOMAIN_XML_INACTIVE | libvirt.VIR_DOMAIN_XML_SECURE
|
||||
|
||||
|
||||
class LiveIsoTestCase(SimpleTestCase):
|
||||
@classmethod
|
||||
def setUpClass(cls):
|
||||
if not livetest.enabled():
|
||||
raise unittest.SkipTest("Set TEST_LIBVIRT_HOST to run the live libvirt tests")
|
||||
cls.args = (
|
||||
os.environ["TEST_LIBVIRT_HOST"],
|
||||
os.environ.get("TEST_LIBVIRT_LOGIN", ""),
|
||||
os.environ.get("TEST_LIBVIRT_PASSWORD", ""),
|
||||
int(os.environ.get("TEST_LIBVIRT_TYPE", 4)),
|
||||
)
|
||||
cls.conn = connection_manager.get_connection(*cls.args)
|
||||
livetest.cleanup(cls.conn)
|
||||
pool = livetest.ensure_pool(cls.conn)
|
||||
cls.inventory = livetest.inventory(cls.conn)
|
||||
cls.iso = pool.createXML(
|
||||
f"<volume><name>{P}media.iso</name><capacity unit='MiB'>1</capacity><target><format type='raw'/></target></volume>", 0
|
||||
).path()
|
||||
super().setUpClass()
|
||||
|
||||
@classmethod
|
||||
def tearDownClass(cls):
|
||||
try:
|
||||
livetest.remove_pool(cls.conn)
|
||||
finally:
|
||||
super().tearDownClass()
|
||||
if livetest.inventory(cls.conn) != cls.inventory:
|
||||
raise AssertionError("Live tests changed objects outside the test namespace")
|
||||
|
||||
def tearDown(self):
|
||||
for dom in self.conn.listAllDomains():
|
||||
if dom.name().startswith(P):
|
||||
if dom.isActive():
|
||||
dom.destroy()
|
||||
dom.undefineFlags(livetest.UNDEFINE_FLAGS)
|
||||
|
||||
def vm(self, state):
|
||||
dom = livetest.define_vm(self.conn, P + "iso-" + state, [], extra_devices=CDROMS)
|
||||
if state in ("running", "paused"):
|
||||
dom.create()
|
||||
if state == "paused":
|
||||
dom.suspend()
|
||||
return dom, wvmInstance(*self.args, None, uuid=dom.UUIDString())
|
||||
|
||||
def media(self, dom, flags):
|
||||
tree = etree.fromstring(dom.XMLDesc(flags))
|
||||
return {d.find("target").get("dev"): d.xpath("string(source/@file)") for d in tree.xpath("devices/disk[@device='cdrom']")}
|
||||
|
||||
def test_mount_and_unmount_in_every_state(self):
|
||||
for state in ("stopped", "running", "paused"):
|
||||
with self.subTest(state):
|
||||
dom, vm = self.vm(state)
|
||||
try:
|
||||
vm.mount_iso("sdb", os.path.basename(self.iso))
|
||||
views = [PERSISTENT] + ([0] if dom.isActive() else [])
|
||||
for flags in views:
|
||||
self.assertEqual(self.media(dom, flags), {"sda": "", "sdb": self.iso}, f"flags={flags}")
|
||||
self.assertIn("keep1234", dom.XMLDesc(PERSISTENT))
|
||||
|
||||
vm.umount_iso("sdb", self.iso)
|
||||
for flags in views:
|
||||
self.assertEqual(self.media(dom, flags), {"sda": "", "sdb": ""}, f"flags={flags}")
|
||||
self.assertIn("keep1234", dom.XMLDesc(PERSISTENT))
|
||||
finally:
|
||||
self.tearDown()
|
||||
|
||||
def test_unknown_iso_or_device_changes_nothing(self):
|
||||
dom, vm = self.vm("running")
|
||||
before = (dom.XMLDesc(0), dom.XMLDesc(PERSISTENT))
|
||||
for dev, image in (("sdb", "no-such.iso"), ("sdz", os.path.basename(self.iso))):
|
||||
with self.subTest(dev=dev, image=image):
|
||||
with self.assertRaises(libvirt.libvirtError):
|
||||
vm.mount_iso(dev, image)
|
||||
self.assertEqual((dom.XMLDesc(0), dom.XMLDesc(PERSISTENT)), before)
|
||||
|
|
@ -1191,13 +1191,15 @@ def detach_cdrom(request, pk, dev):
|
|||
@serialize_instance_mutation
|
||||
def unmount_iso(request, pk):
|
||||
instance = get_instance(request.user, pk, perm_type="change")
|
||||
image = request.POST.get("path", "")
|
||||
dev = request.POST.get("umount_iso", "")
|
||||
instance.proxy.umount_iso(dev, image)
|
||||
msg = _("Mount media: %(dev)s") % {"dev": dev}
|
||||
addlogmsg(request.user.username, instance.compute.name, instance.name, msg)
|
||||
|
||||
return redirect(request.META.get("HTTP_REFERER") + "#disks")
|
||||
try:
|
||||
instance.proxy.umount_iso(dev)
|
||||
except libvirtError as err:
|
||||
messages.error(request, err)
|
||||
else:
|
||||
msg = _("Unmount media: %(dev)s") % {"dev": dev}
|
||||
addlogmsg(request.user.username, instance.compute.name, instance.name, msg)
|
||||
return redirect(_same_origin_referer(request, reverse("instances:instance", args=[pk])) + "#disks")
|
||||
|
||||
|
||||
@require_POST
|
||||
|
|
@ -1207,11 +1209,14 @@ def mount_iso(request, pk):
|
|||
instance = get_instance(request.user, pk, perm_type="change")
|
||||
image = request.POST.get("media", "")
|
||||
dev = request.POST.get("mount_iso", "")
|
||||
instance.proxy.mount_iso(dev, image)
|
||||
msg = _("Unmount media: %(dev)s") % {"dev": dev}
|
||||
addlogmsg(request.user.username, instance.compute.name, instance.name, msg)
|
||||
|
||||
return redirect(request.META.get("HTTP_REFERER") + "#disks")
|
||||
try:
|
||||
instance.proxy.mount_iso(dev, image)
|
||||
except libvirtError as err:
|
||||
messages.error(request, err)
|
||||
else:
|
||||
msg = _("Mount media: %(dev)s") % {"dev": dev}
|
||||
addlogmsg(request.user.username, instance.compute.name, instance.name, msg)
|
||||
return redirect(_same_origin_referer(request, reverse("instances:instance", args=[pk])) + "#disks")
|
||||
|
||||
|
||||
@require_POST
|
||||
|
|
|
|||
|
|
@ -707,52 +707,48 @@ class wvmInstance(wvmConnect):
|
|||
raise Exception("Invalid Device Type for boot order")
|
||||
self._defineXML(ElementTree.tostring(tree).decode())
|
||||
|
||||
def _set_cdrom_media(self, dev, path):
|
||||
"""Insert path into, or with None eject, CD-ROM dev.
|
||||
|
||||
The live and the persistent definition are updated separately from
|
||||
their own device XML, so neither is rewritten from the other (R-12).
|
||||
"""
|
||||
definitions = [("persistent", PERSISTENT_XML, VIR_DOMAIN_AFFECT_CONFIG)]
|
||||
if self.instance.isActive(): # running or paused
|
||||
definitions.insert(0, ("running", VIR_DOMAIN_XML_SECURE, VIR_DOMAIN_AFFECT_LIVE))
|
||||
devices = []
|
||||
for label, xml_flags, _ in definitions:
|
||||
tree = etree.fromstring(self._XMLDesc(xml_flags))
|
||||
found = tree.xpath("./devices/disk[@device='cdrom'][target/@dev=$dev]", dev=dev)
|
||||
if not found:
|
||||
raise libvirtError(f"CD-ROM {dev} is not in the {label} definition")
|
||||
devices.append(found[0])
|
||||
for (label, _, affect), disk in zip(definitions, devices):
|
||||
for source in disk.findall("source"):
|
||||
disk.remove(source)
|
||||
if path:
|
||||
disk.insert(1, etree.Element("source", file=path))
|
||||
try:
|
||||
self.instance.updateDeviceFlags(etree.tostring(disk).decode(), affect)
|
||||
except libvirtError as err:
|
||||
if label == "persistent" and len(definitions) == 2:
|
||||
raise libvirtError(f"Media of {dev} changed in the running VM only: {err}") from err
|
||||
raise
|
||||
|
||||
def mount_iso(self, dev, image):
|
||||
def attach_iso(dev, disk, vol):
|
||||
if disk.get("device") == "cdrom":
|
||||
for elm in disk:
|
||||
if elm.tag == "target" and elm.get("dev") == dev:
|
||||
src_media = ElementTree.Element("source")
|
||||
src_media.set("file", vol.path())
|
||||
disk.insert(2, src_media)
|
||||
return True
|
||||
|
||||
vol = None
|
||||
storages = self.get_storages(only_actives=True)
|
||||
for storage in storages:
|
||||
paths = []
|
||||
for storage in self.get_storages(only_actives=True):
|
||||
stg = self.get_storage(storage)
|
||||
if stg.info()[0] != 0:
|
||||
for img in stg.listVolumes():
|
||||
if image == img:
|
||||
vol = stg.storageVolLookupByName(image)
|
||||
tree = ElementTree.fromstring(self._XMLDesc(0))
|
||||
for disk in tree.findall("devices/disk"):
|
||||
if attach_iso(dev, disk, vol):
|
||||
break
|
||||
if self.get_status() == 1:
|
||||
xml = ElementTree.tostring(disk).decode()
|
||||
self.instance.attachDevice(xml)
|
||||
xmldom = self._XMLDesc(VIR_DOMAIN_XML_SECURE)
|
||||
if self.get_status() == 5:
|
||||
xmldom = ElementTree.tostring(tree).decode()
|
||||
self._defineXML(xmldom)
|
||||
if image in stg.listVolumes():
|
||||
paths.append(stg.storageVolLookupByName(image).path())
|
||||
if not paths:
|
||||
raise libvirtError(f"ISO image {image} was not found")
|
||||
if len(paths) > 1:
|
||||
raise libvirtError(f"ISO image {image} is in more than one storage pool")
|
||||
self._set_cdrom_media(dev, paths[0])
|
||||
|
||||
def umount_iso(self, dev, image):
|
||||
tree = ElementTree.fromstring(self._XMLDesc(0))
|
||||
for disk in tree.findall("devices/disk"):
|
||||
if disk.get("device") == "cdrom":
|
||||
for elm in disk:
|
||||
if elm.tag == "source" and elm.get("file") == image:
|
||||
src_media = elm
|
||||
if elm.tag == "target" and elm.get("dev") == dev:
|
||||
disk.remove(src_media)
|
||||
if self.get_status() == 1:
|
||||
xml_disk = ElementTree.tostring(disk).decode()
|
||||
self.instance.attachDevice(xml_disk)
|
||||
xmldom = self._XMLDesc(VIR_DOMAIN_XML_SECURE)
|
||||
if self.get_status() == 5:
|
||||
xmldom = ElementTree.tostring(tree).decode()
|
||||
self._defineXML(xmldom)
|
||||
def umount_iso(self, dev, image=None):
|
||||
self._set_cdrom_media(dev, None)
|
||||
|
||||
def attach_disk(
|
||||
self,
|
||||
|
|
|
|||
56
vrtManager/test_iso_media.py
Normal file
56
vrtManager/test_iso_media.py
Normal file
|
|
@ -0,0 +1,56 @@
|
|||
import unittest
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from django.conf import settings
|
||||
|
||||
if not settings.configured:
|
||||
settings.configure(MAC_OUI="52:54:10")
|
||||
|
||||
from libvirt import VIR_DOMAIN_AFFECT_CONFIG, VIR_DOMAIN_AFFECT_LIVE, libvirtError
|
||||
from lxml import etree
|
||||
from vrtManager.instance import wvmInstance
|
||||
|
||||
DOMAIN = """<domain><devices>
|
||||
<disk type='file' device='cdrom'><driver name='qemu' type='raw'/><target dev='sda' bus='sata'/></disk>
|
||||
<disk type='file' device='cdrom'><driver name='qemu' type='raw'/><target dev='sdb' bus='sata'/></disk>
|
||||
</devices></domain>"""
|
||||
|
||||
|
||||
def vm(active):
|
||||
inst = wvmInstance.__new__(wvmInstance)
|
||||
inst.instance = MagicMock()
|
||||
inst.instance.isActive.return_value = active
|
||||
inst._XMLDesc = MagicMock(return_value=DOMAIN)
|
||||
return inst
|
||||
|
||||
|
||||
class IsoMediaTestCase(unittest.TestCase):
|
||||
"""R-12: media change goes through updateDeviceFlags for the right CD-ROM."""
|
||||
|
||||
def updates(self, inst):
|
||||
return [
|
||||
(etree.fromstring(c.args[0]).find("target").get("dev"), etree.fromstring(c.args[0]).xpath("string(source/@file)"), c.args[1])
|
||||
for c in inst.instance.updateDeviceFlags.call_args_list
|
||||
]
|
||||
|
||||
def test_stopped_vm_changes_only_the_persistent_definition(self):
|
||||
inst = vm(active=False)
|
||||
inst._set_cdrom_media("sdb", "/iso/a.iso")
|
||||
self.assertEqual(self.updates(inst), [("sdb", "/iso/a.iso", VIR_DOMAIN_AFFECT_CONFIG)])
|
||||
|
||||
def test_running_vm_changes_live_then_persistent(self):
|
||||
inst = vm(active=True)
|
||||
inst._set_cdrom_media("sdb", None)
|
||||
self.assertEqual(
|
||||
self.updates(inst), [("sdb", "", VIR_DOMAIN_AFFECT_LIVE), ("sdb", "", VIR_DOMAIN_AFFECT_CONFIG)]
|
||||
)
|
||||
|
||||
def test_unknown_cdrom_changes_nothing(self):
|
||||
inst = vm(active=True)
|
||||
with self.assertRaises(libvirtError):
|
||||
inst._set_cdrom_media("sdz", "/iso/a.iso")
|
||||
inst.instance.updateDeviceFlags.assert_not_called()
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Loading…
Add table
Add a link
Reference in a new issue