mirror of
https://github.com/saltstack/salt.git
synced 2025-04-16 09:40:20 +00:00
virt.update properly handle removable devices
Live attaching / detaching removable devices results in failures. To change the source of those, we need to call updateDeviceFlags instead.
This commit is contained in:
Cédric Bosdonnat
Daniel Wozniak
parent
73438d43d8
commit
7d18001dba
2 changed files with 243 additions and 16 deletions
|
|
@ -2193,7 +2193,7 @@ def update(
|
|||
"""
|
||||
status = {
|
||||
"definition": False,
|
||||
"disk": {"attached": [], "detached": []},
|
||||
"disk": {"attached": [], "detached": [], "updated": []},
|
||||
"interface": {"attached": [], "detached": []},
|
||||
}
|
||||
conn = __get_conn(**kwargs)
|
||||
|
|
@ -2374,6 +2374,53 @@ def update(
|
|||
}
|
||||
)
|
||||
|
||||
# Look for removable device source changes
|
||||
removable_changes = []
|
||||
new_disks = []
|
||||
for new_disk in changes["disk"].get("new", []):
|
||||
device = new_disk.get("device", "disk")
|
||||
if new_disk.get("type") != "file" and device not in ["cdrom", "floppy"]:
|
||||
new_disks.append(new_disk)
|
||||
continue
|
||||
|
||||
target_dev = new_disk.find("target").get("dev")
|
||||
matching = [
|
||||
old_disk
|
||||
for old_disk in changes["disk"].get("deleted", [])
|
||||
if old_disk.get("type") == "file"
|
||||
and old_disk.get("device", "disk") == device
|
||||
and old_disk.find("target").get("dev") == target_dev
|
||||
]
|
||||
if not matching:
|
||||
new_disks.append(new_disk)
|
||||
else:
|
||||
# libvirt needs to keep the XML exactly as it was before
|
||||
updated_disk = matching[0]
|
||||
changes["disk"]["deleted"].remove(updated_disk)
|
||||
removable_changes.append(updated_disk)
|
||||
source_node = updated_disk.find("source")
|
||||
new_source_node = new_disk.find("source")
|
||||
source_file = (
|
||||
new_source_node.get("file")
|
||||
if new_source_node is not None
|
||||
else None
|
||||
)
|
||||
|
||||
if source_node is not None:
|
||||
if not source_file:
|
||||
# Detaching device
|
||||
updated_disk.remove(source_node)
|
||||
else:
|
||||
# Changing device
|
||||
source_node.set("file", source_file)
|
||||
else:
|
||||
# Attaching device
|
||||
ElementTree.SubElement(
|
||||
updated_disk, "source", attrib={"file": source_file}
|
||||
)
|
||||
|
||||
changes["disk"]["new"] = new_disks
|
||||
|
||||
for dev_type in ["disk", "interface"]:
|
||||
for added in changes[dev_type].get("new", []):
|
||||
commands.append(
|
||||
|
|
@ -2401,6 +2448,19 @@ def update(
|
|||
}
|
||||
)
|
||||
|
||||
for updated_disk in removable_changes:
|
||||
commands.append(
|
||||
{
|
||||
"device": "disk",
|
||||
"cmd": "updateDeviceFlags",
|
||||
"args": [
|
||||
salt.utils.stringutils.to_str(
|
||||
ElementTree.tostring(updated_disk)
|
||||
)
|
||||
],
|
||||
}
|
||||
)
|
||||
|
||||
for cmd in commands:
|
||||
try:
|
||||
ret = getattr(domain, cmd["cmd"])(*cmd["args"]) if not test else 0
|
||||
|
|
@ -2408,7 +2468,11 @@ def update(
|
|||
if device_type in ["cpu", "mem"]:
|
||||
status[device_type] = not bool(ret)
|
||||
else:
|
||||
actions = {"attachDevice": "attached", "detachDevice": "detached"}
|
||||
actions = {
|
||||
"attachDevice": "attached",
|
||||
"detachDevice": "detached",
|
||||
"updateDeviceFlags": "updated",
|
||||
}
|
||||
status[device_type][actions[cmd["cmd"]]].append(cmd["args"][0])
|
||||
|
||||
except libvirt.libvirtError as err:
|
||||
|
|
|
|||
|
|
@ -1758,7 +1758,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
self.assertEqual(
|
||||
{
|
||||
"definition": False,
|
||||
"disk": {"attached": [], "detached": []},
|
||||
"disk": {"attached": [], "detached": [], "updated": []},
|
||||
"interface": {"attached": [], "detached": []},
|
||||
},
|
||||
virt.update("my_vm"),
|
||||
|
|
@ -1768,7 +1768,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
self.assertEqual(
|
||||
{
|
||||
"definition": False,
|
||||
"disk": {"attached": [], "detached": []},
|
||||
"disk": {"attached": [], "detached": [], "updated": []},
|
||||
"interface": {"attached": [], "detached": []},
|
||||
},
|
||||
virt.update(
|
||||
|
|
@ -1795,7 +1795,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
{
|
||||
"definition": True,
|
||||
"cpu": True,
|
||||
"disk": {"attached": [], "detached": []},
|
||||
"disk": {"attached": [], "detached": [], "updated": []},
|
||||
"interface": {"attached": [], "detached": []},
|
||||
},
|
||||
virt.update("my_vm", cpu=2),
|
||||
|
|
@ -1824,7 +1824,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
self.assertEqual(
|
||||
{
|
||||
"definition": True,
|
||||
"disk": {"attached": [], "detached": []},
|
||||
"disk": {"attached": [], "detached": [], "updated": []},
|
||||
"interface": {"attached": [], "detached": []},
|
||||
},
|
||||
virt.update("my_vm", boot=boot),
|
||||
|
|
@ -1847,7 +1847,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
self.assertEqual(
|
||||
{
|
||||
"definition": True,
|
||||
"disk": {"attached": [], "detached": []},
|
||||
"disk": {"attached": [], "detached": [], "updated": []},
|
||||
"interface": {"attached": [], "detached": []},
|
||||
},
|
||||
virt.update("my_vm", boot=boot_uefi),
|
||||
|
|
@ -1874,7 +1874,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
{
|
||||
"definition": True,
|
||||
"mem": True,
|
||||
"disk": {"attached": [], "detached": []},
|
||||
"disk": {"attached": [], "detached": [], "updated": []},
|
||||
"interface": {"attached": [], "detached": []},
|
||||
},
|
||||
virt.update("my_vm", mem=2048),
|
||||
|
|
@ -2005,7 +2005,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
self.assertEqual(
|
||||
{
|
||||
"definition": True,
|
||||
"disk": {"attached": [], "detached": []},
|
||||
"disk": {"attached": [], "detached": [], "updated": []},
|
||||
"interface": {"attached": [], "detached": []},
|
||||
},
|
||||
virt.update("my_vm", graphics={"type": "vnc"}),
|
||||
|
|
@ -2040,7 +2040,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
self.assertEqual(
|
||||
{
|
||||
"definition": False,
|
||||
"disk": {"attached": [], "detached": []},
|
||||
"disk": {"attached": [], "detached": [], "updated": []},
|
||||
"interface": {"attached": [], "detached": []},
|
||||
},
|
||||
virt.update(
|
||||
|
|
@ -2091,7 +2091,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
{
|
||||
"definition": True,
|
||||
"errors": ["Failed to live change memory"],
|
||||
"disk": {"attached": [], "detached": []},
|
||||
"disk": {"attached": [], "detached": [], "updated": []},
|
||||
"interface": {"attached": [], "detached": []},
|
||||
},
|
||||
virt.update("my_vm", mem=2048),
|
||||
|
|
@ -2103,12 +2103,175 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
"definition": True,
|
||||
"errors": ["Failed to live change memory"],
|
||||
"cpu": True,
|
||||
"disk": {"attached": [], "detached": []},
|
||||
"disk": {"attached": [], "detached": [], "updated": []},
|
||||
"interface": {"attached": [], "detached": []},
|
||||
},
|
||||
virt.update("my_vm", cpu=4, mem=2048),
|
||||
)
|
||||
|
||||
def test_update_removables(self):
|
||||
"""
|
||||
Test attaching, detaching, changing removable devices
|
||||
"""
|
||||
xml = """
|
||||
<domain type='kvm' id='7'>
|
||||
<name>my_vm</name>
|
||||
<memory unit='KiB'>1048576</memory>
|
||||
<currentMemory unit='KiB'>1048576</currentMemory>
|
||||
<vcpu placement='auto'>1</vcpu>
|
||||
<os>
|
||||
<type arch='x86_64' machine='pc-i440fx-2.6'>hvm</type>
|
||||
</os>
|
||||
<devices>
|
||||
<disk type='file' device='cdrom'>
|
||||
<driver name='qemu' type='raw' cache='none' io='native'/>
|
||||
<source file='/srv/dvd-image-1.iso'/>
|
||||
<backingStore/>
|
||||
<target dev='hda' bus='ide'/>
|
||||
<readonly/>
|
||||
<alias name='ide0-0-0'/>
|
||||
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
|
||||
</disk>
|
||||
<disk type='file' device='cdrom'>
|
||||
<driver name='qemu' type='raw' cache='none' io='native'/>
|
||||
<target dev='hdb' bus='ide'/>
|
||||
<readonly/>
|
||||
<alias name='ide0-0-1'/>
|
||||
<address type='drive' controller='0' bus='0' target='0' unit='1'/>
|
||||
</disk>
|
||||
<disk type='file' device='cdrom'>
|
||||
<driver name='qemu' type='raw' cache='none' io='native'/>
|
||||
<source file='/srv/dvd-image-2.iso'/>
|
||||
<backingStore/>
|
||||
<target dev='hdc' bus='ide'/>
|
||||
<readonly/>
|
||||
<alias name='ide0-0-2'/>
|
||||
<address type='drive' controller='0' bus='0' target='0' unit='2'/>
|
||||
</disk>
|
||||
<disk type='file' device='cdrom'>
|
||||
<driver name='qemu' type='raw' cache='none' io='native'/>
|
||||
<source file='/srv/dvd-image-3.iso'/>
|
||||
<backingStore/>
|
||||
<target dev='hdd' bus='ide'/>
|
||||
<readonly/>
|
||||
<alias name='ide0-0-3'/>
|
||||
<address type='drive' controller='0' bus='0' target='0' unit='3'/>
|
||||
</disk>
|
||||
</devices>
|
||||
</domain>
|
||||
"""
|
||||
domain_mock = self.set_mock_vm("my_vm", xml)
|
||||
domain_mock.OSType.return_value = "hvm"
|
||||
self.mock_conn.defineXML.return_value = True
|
||||
updatedev_mock = MagicMock(return_value=0)
|
||||
domain_mock.updateDeviceFlags = updatedev_mock
|
||||
|
||||
ret = virt.update(
|
||||
"my_vm",
|
||||
disks=[
|
||||
{
|
||||
"name": "dvd1",
|
||||
"device": "cdrom",
|
||||
"source_file": None,
|
||||
"model": "ide",
|
||||
},
|
||||
{
|
||||
"name": "dvd2",
|
||||
"device": "cdrom",
|
||||
"source_file": "/srv/dvd-image-4.iso",
|
||||
"model": "ide",
|
||||
},
|
||||
{
|
||||
"name": "dvd3",
|
||||
"device": "cdrom",
|
||||
"source_file": "/srv/dvd-image-2.iso",
|
||||
"model": "ide",
|
||||
},
|
||||
{
|
||||
"name": "dvd4",
|
||||
"device": "cdrom",
|
||||
"source_file": "/srv/dvd-image-5.iso",
|
||||
"model": "ide",
|
||||
},
|
||||
],
|
||||
)
|
||||
|
||||
self.assertTrue(ret["definition"])
|
||||
self.assertFalse(ret["disk"]["attached"])
|
||||
self.assertFalse(ret["disk"]["detached"])
|
||||
self.assertEqual(
|
||||
[
|
||||
{
|
||||
"type": "file",
|
||||
"device": "cdrom",
|
||||
"driver": {
|
||||
"name": "qemu",
|
||||
"type": "raw",
|
||||
"cache": "none",
|
||||
"io": "native",
|
||||
},
|
||||
"backingStore": None,
|
||||
"target": {"dev": "hda", "bus": "ide"},
|
||||
"readonly": None,
|
||||
"alias": {"name": "ide0-0-0"},
|
||||
"address": {
|
||||
"type": "drive",
|
||||
"controller": "0",
|
||||
"bus": "0",
|
||||
"target": "0",
|
||||
"unit": "0",
|
||||
},
|
||||
},
|
||||
{
|
||||
"type": "file",
|
||||
"device": "cdrom",
|
||||
"driver": {
|
||||
"name": "qemu",
|
||||
"type": "raw",
|
||||
"cache": "none",
|
||||
"io": "native",
|
||||
},
|
||||
"target": {"dev": "hdb", "bus": "ide"},
|
||||
"readonly": None,
|
||||
"alias": {"name": "ide0-0-1"},
|
||||
"address": {
|
||||
"type": "drive",
|
||||
"controller": "0",
|
||||
"bus": "0",
|
||||
"target": "0",
|
||||
"unit": "1",
|
||||
},
|
||||
"source": {"file": "/srv/dvd-image-4.iso"},
|
||||
},
|
||||
{
|
||||
"type": "file",
|
||||
"device": "cdrom",
|
||||
"driver": {
|
||||
"name": "qemu",
|
||||
"type": "raw",
|
||||
"cache": "none",
|
||||
"io": "native",
|
||||
},
|
||||
"backingStore": None,
|
||||
"target": {"dev": "hdd", "bus": "ide"},
|
||||
"readonly": None,
|
||||
"alias": {"name": "ide0-0-3"},
|
||||
"address": {
|
||||
"type": "drive",
|
||||
"controller": "0",
|
||||
"bus": "0",
|
||||
"target": "0",
|
||||
"unit": "3",
|
||||
},
|
||||
"source": {"file": "/srv/dvd-image-5.iso"},
|
||||
},
|
||||
],
|
||||
[
|
||||
salt.utils.xmlutil.to_dict(ET.fromstring(disk), True)
|
||||
for disk in ret["disk"]["updated"]
|
||||
],
|
||||
)
|
||||
|
||||
def test_update_existing_boot_params(self):
|
||||
"""
|
||||
Test virt.update() with existing boot parameters.
|
||||
|
|
@ -2192,7 +2355,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
self.assertEqual(
|
||||
{
|
||||
"definition": True,
|
||||
"disk": {"attached": [], "detached": []},
|
||||
"disk": {"attached": [], "detached": [], "updated": []},
|
||||
"interface": {"attached": [], "detached": []},
|
||||
},
|
||||
virt.update("vm_with_boot_param", boot=boot_new),
|
||||
|
|
@ -2210,7 +2373,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
self.assertEqual(
|
||||
{
|
||||
"definition": True,
|
||||
"disk": {"attached": [], "detached": []},
|
||||
"disk": {"attached": [], "detached": [], "updated": []},
|
||||
"interface": {"attached": [], "detached": []},
|
||||
},
|
||||
virt.update("vm_with_boot_param", boot=uefi_boot_new),
|
||||
|
|
@ -2238,7 +2401,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
self.assertEqual(
|
||||
{
|
||||
"definition": True,
|
||||
"disk": {"attached": [], "detached": []},
|
||||
"disk": {"attached": [], "detached": [], "updated": []},
|
||||
"interface": {"attached": [], "detached": []},
|
||||
},
|
||||
virt.update("vm_with_boot_param", boot=kernel_none),
|
||||
|
|
@ -2252,7 +2415,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
self.assertEqual(
|
||||
{
|
||||
"definition": True,
|
||||
"disk": {"attached": [], "detached": []},
|
||||
"disk": {"attached": [], "detached": [], "updated": []},
|
||||
"interface": {"attached": [], "detached": []},
|
||||
},
|
||||
virt.update("vm_with_boot_param", boot=uefi_none),
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue