File handle-volumes-on-stopped-pools-in-virt.vm_info... of Package salt

Overview Repositories Revisions Requests Users Attributes Meta

File handle-volumes-on-stopped-pools-in-virt.vm_info-373.patch of Package salt (Revision 524041a25e3a2fc4854cde4fb680210c)

Currently displaying revision 524041a25e3a2fc4854cde4fb680210c , Show latest

From b154f0a17c85c2fe0b85226dfeb3919bd833a85c Mon Sep 17 00:00:00 2001
From: Cedric Bosdonnat <cedric.bosdonnat@free.fr>
Date: Fri, 21 May 2021 13:04:46 +0200
Subject: [PATCH] Handle volumes on stopped pools in virt.vm_info
 (#373)

For VMs having at least a disk on a stopped volume, we don't want the
user to get an exception when running virt.vm_info. Instead just provide
less information.
---
 changelog/60132.fixed                         |  1 +
 salt/modules/virt.py                          | 73 +++++++++++--------
 .../pytests/unit/modules/virt/test_domain.py  |  9 ++-
 3 files changed, 50 insertions(+), 33 deletions(-)
 create mode 100644 changelog/60132.fixed

diff --git a/changelog/60132.fixed b/changelog/60132.fixed
new file mode 100644
index 0000000000..1e3bc96b98
--- /dev/null
+++ b/changelog/60132.fixed
@@ -0,0 +1 @@
+Gracefuly handle errors in virt.vm_info
diff --git a/salt/modules/virt.py b/salt/modules/virt.py
index 6409089109..d8a8c51ce5 100644
--- a/salt/modules/virt.py
+++ b/salt/modules/virt.py
@@ -515,41 +515,50 @@ def _get_disks(conn, dom):
             def _get_disk_volume_data(pool_name, volume_name):
                 qemu_target = "{}/{}".format(pool_name, volume_name)
                 pool = conn.storagePoolLookupByName(pool_name)
-                vol = pool.storageVolLookupByName(volume_name)
-                vol_info = vol.info()
-                extra_properties = {
-                    "virtual size": vol_info[1],
-                    "disk size": vol_info[2],
-                }
-
-                backing_files = [
-                    {
-                        "file": node.find("source").get("file"),
-                        "file format": node.find("format").get("type"),
+                extra_properties = {}
+                try:
+                    vol = pool.storageVolLookupByName(volume_name)
+                    vol_info = vol.info()
+                    extra_properties = {
+                        "virtual size": vol_info[1],
+                        "disk size": vol_info[2],
                     }
-                    for node in elem.findall(".//backingStore[source]")
-                ]
 
-                if backing_files:
-                    # We had the backing files in a flat list, nest them again.
-                    extra_properties["backing file"] = backing_files[0]
-                    parent = extra_properties["backing file"]
-                    for sub_backing_file in backing_files[1:]:
-                        parent["backing file"] = sub_backing_file
-                        parent = sub_backing_file
+                    backing_files = [
+                        {
+                            "file": node.find("source").get("file"),
+                            "file format": node.find("format").get("type"),
+                        }
+                        for node in elem.findall(".//backingStore[source]")
+                    ]
 
-                else:
-                    # In some cases the backing chain is not displayed by the domain definition
-                    # Try to see if we have some of it in the volume definition.
-                    vol_desc = ElementTree.fromstring(vol.XMLDesc())
-                    backing_path = vol_desc.find("./backingStore/path")
-                    backing_format = vol_desc.find("./backingStore/format")
-                    if backing_path is not None:
-                        extra_properties["backing file"] = {"file": backing_path.text}
-                        if backing_format is not None:
-                            extra_properties["backing file"][
-                                "file format"
-                            ] = backing_format.get("type")
+                    if backing_files:
+                        # We had the backing files in a flat list, nest them again.
+                        extra_properties["backing file"] = backing_files[0]
+                        parent = extra_properties["backing file"]
+                        for sub_backing_file in backing_files[1:]:
+                            parent["backing file"] = sub_backing_file
+                            parent = sub_backing_file
+
+                    else:
+                        # In some cases the backing chain is not displayed by the domain definition
+                        # Try to see if we have some of it in the volume definition.
+                        vol_desc = ElementTree.fromstring(vol.XMLDesc())
+                        backing_path = vol_desc.find("./backingStore/path")
+                        backing_format = vol_desc.find("./backingStore/format")
+                        if backing_path is not None:
+                            extra_properties["backing file"] = {
+                                "file": backing_path.text
+                            }
+                            if backing_format is not None:
+                                extra_properties["backing file"][
+                                    "file format"
+                                ] = backing_format.get("type")
+                except libvirt.libvirtError:
+                    # The volume won't be found if the pool is not started, just output less infos
+                    log.info(
+                        "Couldn't extract all volume informations: pool is likely not running or refreshed"
+                    )
                 return (qemu_target, extra_properties)
 
             if disk_type == "file":
diff --git a/tests/pytests/unit/modules/virt/test_domain.py b/tests/pytests/unit/modules/virt/test_domain.py
index 76433eaef4..a9453e4a66 100644
--- a/tests/pytests/unit/modules/virt/test_domain.py
+++ b/tests/pytests/unit/modules/virt/test_domain.py
@@ -192,6 +192,11 @@ def test_get_disks(make_mock_vm, make_mock_storage_pool):
           <alias name='virtio-disk0'/>
           <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
         </disk>
+        <disk type='volume' device='disk'>
+          <driver name='qemu' type='qcow2' cache='none' io='native'/>
+          <source pool='stopped' volume='vm05_data'/>
+          <target dev='vdd' bus='virtio'/>
+        </disk>
         <disk type='network' device='cdrom'>
           <driver name='qemu' type='raw' cache='none' io='native'/>
           <source protocol='http' name='/pub/iso/myimage.iso' query='foo=bar&amp;baz=flurb' index='1'>
@@ -205,11 +210,12 @@ def test_get_disks(make_mock_vm, make_mock_storage_pool):
       </devices>
     </domain>
     """
-    domain_mock = make_mock_vm(vm_def)
+    make_mock_vm(vm_def)
 
     pool_mock = make_mock_storage_pool(
         "default", "dir", ["srv01_system", "srv01_data", "vm05_system"]
     )
+    make_mock_storage_pool("stopped", "dir", [])
 
     # Append backing store to srv01_data volume XML description
     srv1data_mock = pool_mock.storageVolLookupByName("srv01_data")
@@ -256,6 +262,7 @@ def test_get_disks(make_mock_vm, make_mock_storage_pool):
                 },
             },
         },
+        "vdd": {"type": "disk", "file": "stopped/vm05_data", "file format": "qcow2"},
         "hda": {
             "type": "cdrom",
             "file format": "raw",
-- 
2.31.1

Places

File handle-volumes-on-stopped-pools-in-virt.vm_info-373.patch of Package salt (Revision 524041a25e3a2fc4854cde4fb680210c)

Places