diff --git a/salt/modules/virt.py b/salt/modules/virt.py index d200d60c6f0..34d9c1d86f6 100644 --- a/salt/modules/virt.py +++ b/salt/modules/virt.py @@ -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: diff --git a/tests/unit/modules/test_virt.py b/tests/unit/modules/test_virt.py index 216061b57ba..15397ec6f82 100644 --- a/tests/unit/modules/test_virt.py +++ b/tests/unit/modules/test_virt.py @@ -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 = """ + + my_vm + 1048576 + 1048576 + 1 + + hvm + + + + + + + + + +
+ + + + + + +
+ + + + + + + + +
+ + + + + + + + +
+ + + + """ + 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),