File backport-virt-patches-from-3001-256.patch of Package venv-salt-minion - openSUSE Build Service

Overview Repositories Revisions Requests Users Attributes Meta

File backport-virt-patches-from-3001-256.patch of Package venv-salt-minion (Revision 97d218ab21c404afc6d0ba3ebe8e6f0e)

Currently displaying revision 97d218ab21c404afc6d0ba3ebe8e6f0e , Show latest

From 32559016ba2bd306a3a027a2191857f24258fc46 Mon Sep 17 00:00:00 2001
From: Cedric Bosdonnat <cbosdonnat@suse.com>
Date: Mon, 7 Sep 2020 15:00:40 +0200
Subject: [PATCH] Backport virt patches from 3001+ (#256)

* Fix various spelling mistakes in master branch (#55954)

* Fix typo of additional

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of against

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of amount

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of argument

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of attempt

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of bandwidth

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of caught

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of compatibility

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of consistency

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of conversions

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of corresponding

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of dependent

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of dictionary

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of disabled

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of adapters

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of disassociates

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of changes

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of command

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of communicate

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of community

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of configuration

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of default

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of absence

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of attribute

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of container

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of described

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of existence

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of explicit

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of formatted

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of guarantees

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of hexadecimal

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of hierarchy

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of initialize

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of label

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of management

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of mismatch

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of don't

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of manually

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of getting

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of information

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of meant

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of nonexistent

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of occur

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of omitted

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of normally

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of overridden

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of repository

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of separate

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of separator

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of specific

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of successful

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of succeeded

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of support

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of version

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of that's

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of "will be removed"

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of release

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of synchronize

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of python

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of usually

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of override

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of running

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of whether

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of package

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of persist

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of preferred

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of present

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix typo of run

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix spelling mistake of "allows someone to..."

"Allows to" is not correct English. It must either be "allows someone
to" or "allows doing".

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix spelling mistake of "number of times"

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix spelling mistake of msgpack

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix spelling mistake of daemonized

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix spelling mistake of daemons

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix spelling mistake of extemporaneous

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix spelling mistake of instead

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix spelling mistake of returning

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>

* Fix literal comparissons

* virt: Convert cpu_baseline ElementTree to string

In commit 0f5184c (Remove minidom use in virt module) the value
of `cpu` become `xml.etree.ElementTree.Element` and no longer
has a method `toxml()`. This results in the following error:

$ salt '*' virt.cpu_baseline
host2:
    The minion function caused an exception: Traceback (most recent call last):
      File "/usr/lib/python3.7/site-packages/salt/minion.py", line 1675, in _thread_return
        return_data = minion_instance.executors[fname](opts, data, func, args, kwargs)
      File "/usr/lib/python3.7/site-packages/salt/executors/direct_call.py", line 12, in execute
        return func(*args, **kwargs)
      File "/usr/lib/python3.7/site-packages/salt/modules/virt.py", line 4410, in cpu_baseline
        return cpu.toxml()
    AttributeError: 'xml.etree.ElementTree.Element' object has no attribute 'toxml'

Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>

* PR#57374 backport

virt: pool secret should be undefined in pool_undefine not pool_delete

virt: handle build differently depending on the pool type

virt: don't fail if the pool secret has been removed

* PR #57396 backport

add firmware auto select feature

* virt: Update dependencies

Closes: #57641

Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>

* use null in sls file to map None object

add sls file example

reword doc

* Update virt module and states and their tests to python3

* PR #57545 backport

Move virt.init boot_dev parameter away from the kwargs

virt: handle boot device in virt.update()

virt: add boot_dev parameter to virt.running state

* PR #57431 backport

virt: Handle no available hypervisors

virt: Remove unused imports

* Blacken salt

* Add method to remove circular references in data objects and add test (#54930)

* Add method to remove circular references in data objects and add test

* remove trailing whitespace

* Blacken changed files

Co-authored-by: xeacott <jeacott@saltstack.com>
Co-authored-by: Frode Gundersen <fgundersen@saltstack.com>
Co-authored-by: Daniel A. Wozniak <dwozniak@saltstack.com>

* PR #58332 backport

virt: add debug log with VM XML definition

Add xmlutil.get_xml_node() helper function

Add salt.utils.data.get_value function

Add change_xml() function to xmlutil

virt.update: refactor the XML diffing code

virt.test_update: move some code to make test more readable

Co-authored-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>
Co-authored-by: Pedro Algarvio <pedro@algarvio.me>
Co-authored-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Co-authored-by: Firefly <leevn2011@hotmail.com>
Co-authored-by: Blacken Salt <jenkins@saltstack.com>
Co-authored-by: Joe Eacott <31625359+xeacott@users.noreply.github.com>
Co-authored-by: xeacott <jeacott@saltstack.com>
Co-authored-by: Frode Gundersen <fgundersen@saltstack.com>
Co-authored-by: Daniel A. Wozniak <dwozniak@saltstack.com>
---
 changelog/56454.fixed                    |   1 +
 changelog/57544.added                    |   1 +
 changelog/58331.fixed                    |   1 +
 salt/modules/virt.py                     | 270 +++++++++++++----------
 salt/states/virt.py                      |  88 ++++++--
 salt/templates/virt/libvirt_domain.jinja |  29 +--
 salt/utils/xmlutil.py                    |   4 +-
 tests/unit/modules/test_virt.py          | 159 +++++++++----
 tests/unit/states/test_virt.py           |  93 +++++++-
 tests/unit/utils/test_data.py            |  32 ---
 10 files changed, 441 insertions(+), 237 deletions(-)
 create mode 100644 changelog/56454.fixed
 create mode 100644 changelog/57544.added
 create mode 100644 changelog/58331.fixed

diff --git a/changelog/56454.fixed b/changelog/56454.fixed
new file mode 100644
index 0000000000..978b4b6e03
--- /dev/null
+++ b/changelog/56454.fixed
@@ -0,0 +1 @@
+Better handle virt.pool_rebuild in virt.pool_running and virt.pool_defined states
diff --git a/changelog/57544.added b/changelog/57544.added
new file mode 100644
index 0000000000..52071cf2c7
--- /dev/null
+++ b/changelog/57544.added
@@ -0,0 +1 @@
+Allow setting VM boot devices order in virt.running and virt.defined states
diff --git a/changelog/58331.fixed b/changelog/58331.fixed
new file mode 100644
index 0000000000..4b8f78dd53
--- /dev/null
+++ b/changelog/58331.fixed
@@ -0,0 +1 @@
+Leave boot parameters untouched if boot parameter is set to None in virt.update
diff --git a/salt/modules/virt.py b/salt/modules/virt.py
index fb27397baa..ec40f08359 100644
--- a/salt/modules/virt.py
+++ b/salt/modules/virt.py
@@ -94,17 +94,13 @@ from xml.sax import saxutils
 import jinja2.exceptions
 import salt.utils.files
 import salt.utils.json
-import salt.utils.network
 import salt.utils.path
 import salt.utils.stringutils
 import salt.utils.templates
-import salt.utils.validate.net
-import salt.utils.versions
 import salt.utils.xmlutil as xmlutil
 import salt.utils.yaml
 from salt._compat import ipaddress
 from salt.exceptions import CommandExecutionError, SaltInvocationError
-from salt.ext import six
 from salt.ext.six.moves import range  # pylint: disable=import-error,redefined-builtin
 from salt.ext.six.moves.urllib.parse import urlparse, urlunparse
 from salt.utils.virt import check_remote, download_remote
@@ -647,6 +643,7 @@ def _gen_xml(
     arch,
     graphics=None,
     boot=None,
+    boot_dev=None,
     **kwargs
 ):
     """
@@ -680,15 +677,17 @@ def _gen_xml(
             graphics = None
     context["graphics"] = graphics
 
-    if "boot_dev" in kwargs:
-        context["boot_dev"] = []
-        for dev in kwargs["boot_dev"].split():
-            context["boot_dev"].append(dev)
-    else:
-        context["boot_dev"] = ["hd"]
+    context["boot_dev"] = boot_dev.split() if boot_dev is not None else ["hd"]
 
     context["boot"] = boot if boot else {}
 
+    # if efi parameter is specified, prepare os_attrib
+    efi_value = context["boot"].get("efi", None) if boot else None
+    if efi_value is True:
+        context["boot"]["os_attrib"] = "firmware='efi'"
+    elif efi_value is not None and type(efi_value) != bool:
+        raise SaltInvocationError("Invalid efi value")
+
     if os_type == "xen":
         # Compute the Xen PV boot method
         if __grains__["os_family"] == "Suse":
@@ -1519,17 +1518,24 @@ def _handle_remote_boot_params(orig_boot):
     new_boot = orig_boot.copy()
     keys = orig_boot.keys()
     cases = [
+        {"efi"},
+        {"kernel", "initrd", "efi"},
+        {"kernel", "initrd", "cmdline", "efi"},
         {"loader", "nvram"},
         {"kernel", "initrd"},
         {"kernel", "initrd", "cmdline"},
-        {"loader", "nvram", "kernel", "initrd"},
-        {"loader", "nvram", "kernel", "initrd", "cmdline"},
+        {"kernel", "initrd", "loader", "nvram"},
+        {"kernel", "initrd", "cmdline", "loader", "nvram"},
     ]
 
     try:
         if keys in cases:
             for key in keys:
-                if orig_boot.get(key) is not None and check_remote(orig_boot.get(key)):
+                if key == "efi" and type(orig_boot.get(key)) == bool:
+                    new_boot[key] = orig_boot.get(key)
+                elif orig_boot.get(key) is not None and check_remote(
+                    orig_boot.get(key)
+                ):
                     if saltinst_dir is None:
                         os.makedirs(CACHE_DIR)
                         saltinst_dir = CACHE_DIR
@@ -1537,12 +1543,41 @@ def _handle_remote_boot_params(orig_boot):
             return new_boot
         else:
             raise SaltInvocationError(
-                "Invalid boot parameters, (kernel, initrd) or/and (loader, nvram) must be both present"
+                "Invalid boot parameters,It has to follow this combination: [(kernel, initrd) or/and cmdline] or/and [(loader, nvram) or efi]"
             )
     except Exception as err:  # pylint: disable=broad-except
         raise err
 
 
+def _handle_efi_param(boot, desc):
+    """
+    Checks if boot parameter contains efi boolean value, if so, handles the firmware attribute.
+    :param boot: The boot parameters passed to the init or update functions.
+    :param desc: The XML description of that domain.
+    :return: A boolean value.
+    """
+    efi_value = boot.get("efi", None) if boot else None
+    parent_tag = desc.find("os")
+    os_attrib = parent_tag.attrib
+
+    # newly defined vm without running, loader tag might not be filled yet
+    if efi_value is False and os_attrib != {}:
+        parent_tag.attrib.pop("firmware", None)
+        return True
+
+    # check the case that loader tag might be present. This happens after the vm ran
+    elif type(efi_value) == bool and os_attrib == {}:
+        if efi_value is True and parent_tag.find("loader") is None:
+            parent_tag.set("firmware", "efi")
+        if efi_value is False and parent_tag.find("loader") is not None:
+            parent_tag.remove(parent_tag.find("loader"))
+            parent_tag.remove(parent_tag.find("nvram"))
+        return True
+    elif type(efi_value) != bool:
+        raise SaltInvocationError("Invalid efi value")
+    return False
+
+
 def init(
     name,
     cpu,
@@ -1563,6 +1598,7 @@ def init(
     os_type=None,
     arch=None,
     boot=None,
+    boot_dev=None,
     **kwargs
 ):
     """
@@ -1632,7 +1668,8 @@ def init(
         This is an optional parameter, all of the keys are optional within the dictionary. The structure of
         the dictionary is documented in :ref:`init-boot-def`. If a remote path is provided to kernel or initrd,
         salt will handle the downloading of the specified remote file and modify the XML accordingly.
-        To boot VM with UEFI, specify loader and nvram path.
+        To boot VM with UEFI, specify loader and nvram path or specify 'efi': ``True`` if your libvirtd version
+        is >= 5.2.0 and QEMU >= 3.0.0.
 
         .. versionadded:: 3000
 
@@ -1646,6 +1683,12 @@ def init(
                 'nvram': '/usr/share/OVMF/OVMF_VARS.ms.fd'
             }
 
+    :param boot_dev:
+        Space separated list of devices to boot from sorted by decreasing priority.
+        Values can be ``hd``, ``fd``, ``cdrom`` or ``network``.
+
+        By default, the value will ``"hd"``.
+
     .. _init-boot-def:
 
     .. rubric:: Boot parameters definition
@@ -1671,6 +1714,11 @@ def init(
 
         .. versionadded:: sodium
 
+    efi
+       A boolean value.
+
+       .. versionadded:: sodium
+
     .. _init-nic-def:
 
     .. rubric:: Network Interfaces Definitions
@@ -1855,6 +1903,8 @@ def init(
                     for x in y
                 }
             )
+            if len(hypervisors) == 0:
+                raise SaltInvocationError("No supported hypervisors were found")
             virt_hypervisor = "kvm" if "kvm" in hypervisors else hypervisors[0]
 
         # esxi used to be a possible value for the hypervisor: map it to vmware since it's the same
@@ -1962,8 +2012,10 @@ def init(
             arch,
             graphics,
             boot,
+            boot_dev,
             **kwargs
         )
+        log.debug("New virtual machine definition: %s", vm_xml)
         conn.defineXML(vm_xml)
     except libvirt.libvirtError as err:
         conn.close()
@@ -2189,6 +2241,7 @@ def update(
     live=True,
     boot=None,
     test=False,
+    boot_dev=None,
     **kwargs
 ):
     """
@@ -2248,11 +2301,28 @@ def update(
 
         Refer to :ref:`init-boot-def` for the complete boot parameter description.
 
-        To update any boot parameters, specify the new path for each. To remove any boot parameters,
-        pass a None object, for instance: 'kernel': ``None``.
+        To update any boot parameters, specify the new path for each. To remove any boot parameters, pass ``None`` object,
+        for instance: 'kernel': ``None``. To switch back to BIOS boot, specify ('loader': ``None`` and 'nvram': ``None``)
+        or 'efi': ``False``. Please note that ``None`` is mapped to ``null`` in sls file, pass ``null`` in sls file instead.
+
+        SLS file Example:
+
+        .. code-block:: yaml
+
+            - boot:
+                loader: null
+                nvram: null
 
         .. versionadded:: 3000
 
+    :param boot_dev:
+        Space separated list of devices to boot from sorted by decreasing priority.
+        Values can be ``hd``, ``fd``, ``cdrom`` or ``network``.
+
+        By default, the value will ``"hd"``.
+
+        .. versionadded:: Magnesium
+
     :param test: run in dry-run mode if set to True
 
         .. versionadded:: sodium
@@ -2327,67 +2397,54 @@ def update(
         cpu_node.set("current", str(cpu))
         need_update = True
 
-    # Update the kernel boot parameters
-    boot_tags = ["kernel", "initrd", "cmdline", "loader", "nvram"]
-    parent_tag = desc.find("os")
-
-    # We need to search for each possible subelement, and update it.
-    for tag in boot_tags:
-        # The Existing Tag...
-        found_tag = parent_tag.find(tag)
-
-        # The new value
-        boot_tag_value = boot.get(tag, None) if boot else None
-
-        # Existing tag is found and values don't match
-        if found_tag is not None and found_tag.text != boot_tag_value:
-
-            # If the existing tag is found, but the new value is None
-            # remove it. If the existing tag is found, and the new value
-            # doesn't match update it. In either case, mark for update.
-            if boot_tag_value is None and boot is not None and parent_tag is not None:
-                parent_tag.remove(found_tag)
-            else:
-                found_tag.text = boot_tag_value
-
-            # If the existing tag is loader or nvram, we need to update the corresponding attribute
-            if found_tag.tag == "loader" and boot_tag_value is not None:
-                found_tag.set("readonly", "yes")
-                found_tag.set("type", "pflash")
-
-            if found_tag.tag == "nvram" and boot_tag_value is not None:
-                found_tag.set("template", found_tag.text)
-                found_tag.text = None
+    def _set_loader(node, value):
+        salt.utils.xmlutil.set_node_text(node, value)
+        if value is not None:
+            node.set("readonly", "yes")
+            node.set("type", "pflash")
 
-            need_update = True
+    def _set_nvram(node, value):
+        node.set("template", value)
 
-            # Need to check for parent tag, and add it if it does not exist.
-            # Add a subelement and set the value to the new value, and then
-            # mark for update.
-            if parent_tag is not None:
-                child_tag = ElementTree.SubElement(parent_tag, tag)
-            else:
-                new_parent_tag = ElementTree.Element("os")
-                child_tag = ElementTree.SubElement(new_parent_tag, tag)
-
-            child_tag.text = boot_tag_value
-
-            # If the newly created tag is loader or nvram, we need to update the corresponding attribute
-            if child_tag.tag == "loader":
-                child_tag.set("readonly", "yes")
-                child_tag.set("type", "pflash")
+    def _set_with_mib_unit(node, value):
+        node.text = str(value)
+        node.set("unit", "MiB")
 
-            if child_tag.tag == "nvram":
-                child_tag.set("template", child_tag.text)
-                child_tag.text = None
+    # Update the kernel boot parameters
+    params_mapping = [
+        {"path": "boot:kernel", "xpath": "os/kernel"},
+        {"path": "boot:initrd", "xpath": "os/initrd"},
+        {"path": "boot:cmdline", "xpath": "os/cmdline"},
+        {"path": "boot:loader", "xpath": "os/loader", "set": _set_loader},
+        {"path": "boot:nvram", "xpath": "os/nvram", "set": _set_nvram},
+        # Update the memory, note that libvirt outputs all memory sizes in KiB
+        {
+            "path": "mem",
+            "xpath": "memory",
+            "get": lambda n: int(n.text) / 1024,
+            "set": _set_with_mib_unit,
+        },
+        {
+            "path": "mem",
+            "xpath": "currentMemory",
+            "get": lambda n: int(n.text) / 1024,
+            "set": _set_with_mib_unit,
+        },
+        {
+            "path": "boot_dev:{dev}",
+            "xpath": "os/boot[$dev]",
+            "get": lambda n: n.get("dev"),
+            "set": lambda n, v: n.set("dev", v),
+            "del": salt.utils.xmlutil.del_attribute("dev"),
+        },
+    ]
 
-    # Update the memory, note that libvirt outputs all memory sizes in KiB
-    for mem_node_name in ["memory", "currentMemory"]:
-        mem_node = desc.find(mem_node_name)
-        if mem and int(mem_node.text) != mem * 1024:
-            mem_node.text = str(mem)
-            mem_node.set("unit", "MiB")
-            need_update = True
+    data = {k: v for k, v in locals().items() if bool(v)}
+    if boot_dev:
+        data["boot_dev"] = {i + 1: dev for i, dev in enumerate(boot_dev.split())}
+    need_update = need_update or salt.utils.xmlutil.change_xml(
+        desc, data, params_mapping
+    )
 
     # Update the XML definition with the new disks and diff changes
     devices_node = desc.find("devices")
@@ -2434,9 +2491,9 @@ def update(
                         _disk_volume_create(conn, all_disks[idx])
 
             if not test:
-                conn.defineXML(
-                    salt.utils.stringutils.to_str(ElementTree.tostring(desc))
-                )
+                xml_desc = ElementTree.tostring(desc)
+                log.debug("Update virtual machine definition: %s", xml_desc)
+                conn.defineXML(salt.utils.stringutils.to_str(xml_desc))
             status["definition"] = True
         except libvirt.libvirtError as err:
             conn.close()
@@ -3218,24 +3275,19 @@ def get_profiles(hypervisor=None, **kwargs):
             for x in y
         }
     )
-    default_hypervisor = "kvm" if "kvm" in hypervisors else hypervisors[0]
+    if len(hypervisors) == 0:
+        raise SaltInvocationError("No supported hypervisors were found")
 
     if not hypervisor:
-        hypervisor = default_hypervisor
+        hypervisor = "kvm" if "kvm" in hypervisors else hypervisors[0]
     virtconf = __salt__["config.get"]("virt", {})
     for typ in ["disk", "nic"]:
         _func = getattr(sys.modules[__name__], "_{}_profile".format(typ))
-        ret[typ] = {
-            "default": _func(
-                "default", hypervisor if hypervisor else default_hypervisor
-            )
-        }
+        ret[typ] = {"default": _func("default", hypervisor)}
         if typ in virtconf:
             ret.setdefault(typ, {})
             for prf in virtconf[typ]:
-                ret[typ][prf] = _func(
-                    prf, hypervisor if hypervisor else default_hypervisor
-                )
+                ret[typ][prf] = _func(prf, hypervisor)
     return ret
 
 
@@ -4043,7 +4095,7 @@ def purge(vm_, dirs=False, removables=False, **kwargs):
             directories.add(os.path.dirname(disks[disk]["file"]))
         else:
             # We may have a volume to delete here
-            matcher = re.match("^(?P<pool>[^/]+)/(?P<volume>.*)$", disks[disk]["file"],)
+            matcher = re.match("^(?P<pool>[^/]+)/(?P<volume>.*)$", disks[disk]["file"])
             if matcher:
                 pool_name = matcher.group("pool")
                 pool = None
@@ -5431,7 +5483,7 @@ def list_networks(**kwargs):
 
 def network_info(name=None, **kwargs):
     """
-    Return informations on a virtual network provided its name.
+    Return information on a virtual network provided its name.
 
     :param name: virtual network name
     :param connection: libvirt connection URI, overriding defaults
@@ -6028,15 +6080,19 @@ def _pool_set_secret(
         if secret_type:
             # Get the previously defined secret if any
             secret = None
-            if usage:
-                usage_type = (
-                    libvirt.VIR_SECRET_USAGE_TYPE_CEPH
-                    if secret_type == "ceph"
-                    else libvirt.VIR_SECRET_USAGE_TYPE_ISCSI
-                )
-                secret = conn.secretLookupByUsage(usage_type, usage)
-            elif uuid:
-                secret = conn.secretLookupByUUIDString(uuid)
+            try:
+                if usage:
+                    usage_type = (
+                        libvirt.VIR_SECRET_USAGE_TYPE_CEPH
+                        if secret_type == "ceph"
+                        else libvirt.VIR_SECRET_USAGE_TYPE_ISCSI
+                    )
+                    secret = conn.secretLookupByUsage(usage_type, usage)
+                elif uuid:
+                    secret = conn.secretLookupByUUIDString(uuid)
+            except libvirt.libvirtError as err:
+                # For some reason the secret has been removed. Don't fail since we'll recreate it
+                log.info("Secret not found: %s", err.get_error_message())
 
             # Create secret if needed
             if not secret:
@@ -6288,7 +6344,7 @@ def list_pools(**kwargs):
 
 def pool_info(name=None, **kwargs):
     """
-    Return informations on a storage pool provided its name.
+    Return information on a storage pool provided its name.
 
     :param name: libvirt storage pool name
     :param connection: libvirt connection URI, overriding defaults
@@ -6505,22 +6561,6 @@ def pool_delete(name, **kwargs):
     conn = __get_conn(**kwargs)
     try:
         pool = conn.storagePoolLookupByName(name)
-        desc = ElementTree.fromstring(pool.XMLDesc())
-
-        # Is there a secret that we generated and would need to be removed?
-        # Don't remove the other secrets
-        auth_node = desc.find("source/auth")
-        if auth_node is not None:
-            auth_types = {
-                "ceph": libvirt.VIR_SECRET_USAGE_TYPE_CEPH,
-                "iscsi": libvirt.VIR_SECRET_USAGE_TYPE_ISCSI,
-            }
-            secret_type = auth_types[auth_node.get("type")]
-            secret_usage = auth_node.find("secret").get("usage")
-            if secret_type and "pool_{}".format(name) == secret_usage:
-                secret = conn.secretLookupByUsage(secret_type, secret_usage)
-                secret.undefine()
-
         return not bool(pool.delete(libvirt.VIR_STORAGE_POOL_DELETE_NORMAL))
     finally:
         conn.close()
diff --git a/salt/states/virt.py b/salt/states/virt.py
index cb15d57d8f..b45cf72ed3 100644
--- a/salt/states/virt.py
+++ b/salt/states/virt.py
@@ -33,6 +33,8 @@ except ImportError:
 
 __virtualname__ = "virt"
 
+log = logging.getLogger(__name__)
+
 
 def __virtual__():
     """
@@ -285,6 +287,7 @@ def defined(
     arch=None,
     boot=None,
     update=True,
+    boot_dev=None,
 ):
     """
     Starts an existing guest, or defines and starts a new VM with specified arguments.
@@ -345,6 +348,14 @@ def defined(
 
         .. deprecated:: sodium
 
+    :param boot_dev:
+        Space separated list of devices to boot from sorted by decreasing priority.
+        Values can be ``hd``, ``fd``, ``cdrom`` or ``network``.
+
+        By default, the value will ``"hd"``.
+
+        .. versionadded:: Magnesium
+
     .. rubric:: Example States
 
     Make sure a virtual machine called ``domain_name`` is defined:
@@ -355,6 +366,7 @@ def defined(
           virt.defined:
             - cpu: 2
             - mem: 2048
+            - boot_dev: network hd
             - disk_profile: prod
             - disks:
               - name: system
@@ -407,6 +419,7 @@ def defined(
                     password=password,
                     boot=boot,
                     test=__opts__["test"],
+                    boot_dev=boot_dev,
                 )
             ret["changes"][name] = status
             if not status.get("definition"):
@@ -441,6 +454,7 @@ def defined(
                     password=password,
                     boot=boot,
                     start=False,
+                    boot_dev=boot_dev,
                 )
             ret["changes"][name] = {"definition": True}
             ret["comment"] = "Domain {} defined".format(name)
@@ -473,6 +487,7 @@ def running(
     os_type=None,
     arch=None,
     boot=None,
+    boot_dev=None,
 ):
     """
     Starts an existing guest, or defines and starts a new VM with specified arguments.
@@ -584,6 +599,14 @@ def running(
 
         .. versionadded:: 3000
 
+    :param boot_dev:
+        Space separated list of devices to boot from sorted by decreasing priority.
+        Values can be ``hd``, ``fd``, ``cdrom`` or ``network``.
+
+        By default, the value will ``"hd"``.
+
+        .. versionadded:: Magnesium
+
     .. rubric:: Example States
 
     Make sure an already-defined virtual machine called ``domain_name`` is running:
@@ -651,6 +674,7 @@ def running(
         arch=arch,
         boot=boot,
         update=update,
+        boot_dev=boot_dev,
         connection=connection,
         username=username,
         password=password,
@@ -1218,14 +1242,24 @@ def pool_defined(
 
                 action = ""
                 if info[name]["state"] != "running":
-                    if not __opts__["test"]:
-                        __salt__["virt.pool_build"](
-                            name,
-                            connection=connection,
-                            username=username,
-                            password=password,
-                        )
-                    action = ", built"
+                    if ptype in BUILDABLE_POOL_TYPES:
+                        if not __opts__["test"]:
+                            # Storage pools build like disk or logical will fail if the disk or LV group
+                            # was already existing. Since we can't easily figure that out, just log the
+                            # possible libvirt error.
+                            try:
+                                __salt__["virt.pool_build"](
+                                    name,
+                                    connection=connection,
+                                    username=username,
+                                    password=password,
+                                )
+                            except libvirt.libvirtError as err:
+                                log.warning(
+                                    "Failed to build libvirt storage pool: %s",
+                                    err.get_error_message(),
+                                )
+                        action = ", built"
 
                 action = (
                     "{}, autostart flag changed".format(action)
@@ -1261,9 +1295,22 @@ def pool_defined(
                     password=password,
                 )
 
-                __salt__["virt.pool_build"](
-                    name, connection=connection, username=username, password=password
-                )
+                if ptype in BUILDABLE_POOL_TYPES:
+                    # Storage pools build like disk or logical will fail if the disk or LV group
+                    # was already existing. Since we can't easily figure that out, just log the
+                    # possible libvirt error.
+                    try:
+                        __salt__["virt.pool_build"](
+                            name,
+                            connection=connection,
+                            username=username,
+                            password=password,
+                        )
+                    except libvirt.libvirtError as err:
+                        log.warning(
+                            "Failed to build libvirt storage pool: %s",
+                            err.get_error_message(),
+                        )
             if needs_autostart:
                 ret["changes"][name] = "Pool defined, marked for autostart"
                 ret["comment"] = "Pool {} defined, marked for autostart".format(name)
@@ -1370,7 +1417,7 @@ def pool_running(
             is_running = info.get(name, {}).get("state", "stopped") == "running"
             if is_running:
                 if updated:
-                    action = "built, restarted"
+                    action = "restarted"
                     if not __opts__["test"]:
                         __salt__["virt.pool_stop"](
                             name,
@@ -1378,13 +1425,16 @@ def pool_running(
                             username=username,
                             password=password,
                         )
-                    if not __opts__["test"]:
-                        __salt__["virt.pool_build"](
-                            name,
-                            connection=connection,
-                            username=username,
-                            password=password,
-                        )
+                    # if the disk or LV group is already existing build will fail (issue #56454)
+                    if ptype in BUILDABLE_POOL_TYPES - {"disk", "logical"}:
+                        if not __opts__["test"]:
+                            __salt__["virt.pool_build"](
+                                name,
+                                connection=connection,
+                                username=username,
+                                password=password,
+                            )
+                        action = "built, {}".format(action)
                 else:
                     action = "already running"
                     result = True
diff --git a/salt/templates/virt/libvirt_domain.jinja b/salt/templates/virt/libvirt_domain.jinja
index 439ed83f7f..2a2f5e4141 100644
--- a/salt/templates/virt/libvirt_domain.jinja
+++ b/salt/templates/virt/libvirt_domain.jinja
@@ -2,32 +2,9 @@
 <domain type='{{ hypervisor }}'>
         <name>{{ name }}</name>
         <vcpu>{{ cpu }}</vcpu>
-        {%- if mem.max %}
-        <maxMemory {{ mem.slots }} unit='KiB'> {{ mem.max }}</maxMemory>
-        {%- endif %}
-        {%- if mem.boot %}
-        <memory unit='KiB'>{{ mem.boot }}</memory>
-        {%- endif %}
-        {%- if mem.current %}
-        <currentMemory unit='KiB'>{{ mem.current }}</currentMemory>
-        {%- endif %}
-        {%- if mem %}
-        <memtune>
-            {%- if 'hard_limit' in mem and mem.hard_limit %}
-                <hard_limit unit="KiB">{{ mem.hard_limit }}</hard_limit>
-            {%- endif %}
-            {%- if 'soft_limit' in mem and mem.soft_limit %}
-                <soft_limit unit="KiB">{{ mem.soft_limit }}</soft_limit>
-            {%- endif %}
-            {%- if 'swap_hard_limit' in mem and mem.swap_hard_limit %}
-                <swap_hard_limit unit="KiB">{{ mem.swap_hard_limit }}</swap_hard_limit>
-            {%- endif %}
-            {%- if 'min_guarantee' in mem and mem.min_guarantee %}
-                <min_guarantee unit="KiB">{{ mem.min_guarantee }}</min_guarantee>
-            {%- endif %}
-        </memtune>
-        {%- endif %}
-        <os {{ boot.os_attrib }}>
+        <memory unit='KiB'>{{ mem }}</memory>
+        <currentMemory unit='KiB'>{{ mem }}</currentMemory>
+        <os {{boot.os_attrib}}>
                 <type arch='{{ arch }}'>{{ os_type }}</type>
                 {% if boot %}
                   {% if 'kernel' in boot %}
diff --git a/salt/utils/xmlutil.py b/salt/utils/xmlutil.py
index e5c8ad4eec..b9f047820b 100644
--- a/salt/utils/xmlutil.py
+++ b/salt/utils/xmlutil.py
@@ -25,7 +25,7 @@ def _to_dict(xmltree):
     """
     Converts an XML ElementTree to a dictionary that only contains items.
     This is the default behavior in version 2017.7. This will default to prevent
-    unexpected parsing issues on modules dependent on this.
+    unexpected parsing issues on modules dependant on this.
     """
     # If this object has no children, the for..loop below will return nothing
     # for it, so just return a single dict representing it.
@@ -298,7 +298,7 @@ def change_xml(doc, data, mapping):
                 if convert_fn:
                     new_value = convert_fn(new_value)
 
-                if str(current_value) != str(new_value):
+                if current_value != new_value:
                     set_fn(node, new_value)
                     need_update = True
             else:
diff --git a/tests/unit/modules/test_virt.py b/tests/unit/modules/test_virt.py
index f53b4a85c1..4775fec31f 100644
--- a/tests/unit/modules/test_virt.py
+++ b/tests/unit/modules/test_virt.py
@@ -1843,17 +1843,36 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
             "cmdline": "console=ttyS0 ks=http://example.com/f8-i386/os/",
         }
 
-        boot_uefi = {
-            "loader": "/usr/share/OVMF/OVMF_CODE.fd",
-            "nvram": "/usr/share/OVMF/OVMF_VARS.ms.fd",
-        }
+        # Update boot devices case
+        define_mock.reset_mock()
+        self.assertEqual(
+            {
+                "definition": True,
+                "disk": {"attached": [], "detached": [], "updated": []},
+                "interface": {"attached": [], "detached": []},
+            },
+            virt.update("my_vm", boot_dev="cdrom network hd"),
+        )
+        setxml = ET.fromstring(define_mock.call_args[0][0])
+        self.assertEqual(
+            ["cdrom", "network", "hd"],
+            [node.get("dev") for node in setxml.findall("os/boot")],
+        )
 
-        invalid_boot = {
-            "loader": "/usr/share/OVMF/OVMF_CODE.fd",
-            "initrd": "/root/f8-i386-initrd",
-        }
+        # Update unchanged boot devices case
+        define_mock.reset_mock()
+        self.assertEqual(
+            {
+                "definition": False,
+                "disk": {"attached": [], "detached": [], "updated": []},
+                "interface": {"attached": [], "detached": []},
+            },
+            virt.update("my_vm", boot_dev="hd"),
+        )
+        define_mock.assert_not_called()
 
         # Update with boot parameter case
+        define_mock.reset_mock()
         self.assertEqual(
             {
                 "definition": True,
@@ -1877,6 +1896,11 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
             "console=ttyS0 ks=http://example.com/f8-i386/os/",
         )
 
+        boot_uefi = {
+            "loader": "/usr/share/OVMF/OVMF_CODE.fd",
+            "nvram": "/usr/share/OVMF/OVMF_VARS.ms.fd",
+        }
+
         self.assertEqual(
             {
                 "definition": True,
@@ -1896,9 +1920,28 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
             "/usr/share/OVMF/OVMF_VARS.ms.fd",
         )
 
+        self.assertEqual(
+            {
+                "definition": True,
+                "disk": {"attached": [], "detached": [], "updated": []},
+                "interface": {"attached": [], "detached": []},
+            },
+            virt.update("my_vm", boot={"efi": True}),
+        )
+        setxml = ET.fromstring(define_mock.call_args[0][0])
+        self.assertEqual(setxml.find("os").attrib.get("firmware"), "efi")
+
+        invalid_boot = {
+            "loader": "/usr/share/OVMF/OVMF_CODE.fd",
+            "initrd": "/root/f8-i386-initrd",
+        }
+
         with self.assertRaises(SaltInvocationError):
             virt.update("my_vm", boot=invalid_boot)
 
+        with self.assertRaises(SaltInvocationError):
+            virt.update("my_vm", boot={"efi": "Not a boolean value"})
+
         # Update memory case
         setmem_mock = MagicMock(return_value=0)
         domain_mock.setMemoryFlags = setmem_mock
@@ -2390,6 +2433,43 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
             ],
         )
 
+    def test_update_xen_boot_params(self):
+        """
+        Test virt.update() a Xen definition no boot parameter.
+        """
+        root_dir = os.path.join(salt.syspaths.ROOT_DIR, "srv", "salt-images")
+        xml_boot = """
+            <domain type='xen' id='8'>
+              <name>vm</name>
+              <memory unit='KiB'>1048576</memory>
+              <currentMemory unit='KiB'>1048576</currentMemory>
+              <vcpu placement='auto'>1</vcpu>
+              <os>
+                <type arch='x86_64' machine='xenfv'>hvm</type>
+                <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>
+              </os>
+            </domain>
+        """
+        domain_mock_boot = self.set_mock_vm("vm", xml_boot)
+        domain_mock_boot.OSType = MagicMock(return_value="hvm")
+        define_mock_boot = MagicMock(return_value=True)
+        define_mock_boot.setVcpusFlags = MagicMock(return_value=0)
+        self.mock_conn.defineXML = define_mock_boot
+        self.assertEqual(
+            {
+                "cpu": False,
+                "definition": True,
+                "disk": {"attached": [], "detached": [], "updated": []},
+                "interface": {"attached": [], "detached": []},
+            },
+            virt.update("vm", cpu=2),
+        )
+        setxml = ET.fromstring(define_mock_boot.call_args[0][0])
+        self.assertEqual(setxml.find("os").find("loader").attrib.get("type"), "rom")
+        self.assertEqual(
+            setxml.find("os").find("loader").text, "/usr/lib/xen/boot/hvmloader"
+        )
+
     def test_update_existing_boot_params(self):
         """
         Test virt.update() with existing boot parameters.
@@ -2530,6 +2610,18 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         self.assertEqual(setxml.find("os").find("initrd"), None)
         self.assertEqual(setxml.find("os").find("cmdline"), None)
 
+        self.assertEqual(
+            {
+                "definition": True,
+                "disk": {"attached": [], "detached": [], "updated": []},
+                "interface": {"attached": [], "detached": []},
+            },
+            virt.update("vm_with_boot_param", boot={"efi": False}),
+        )
+        setxml = ET.fromstring(define_mock_boot.call_args[0][0])
+        self.assertEqual(setxml.find("os").find("nvram"), None)
+        self.assertEqual(setxml.find("os").find("loader"), None)
+
         self.assertEqual(
             {
                 "definition": True,
@@ -4248,7 +4340,6 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         """
         mock_pool = MagicMock()
         mock_pool.delete = MagicMock(return_value=0)
-        mock_pool.XMLDesc.return_value = "<pool type='dir'/>"
         self.mock_conn.storagePoolLookupByName = MagicMock(return_value=mock_pool)
 
         res = virt.pool_delete("test-pool")
@@ -4262,12 +4353,12 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
             self.mock_libvirt.VIR_STORAGE_POOL_DELETE_NORMAL
         )
 
-    def test_pool_delete_secret(self):
+    def test_pool_undefine_secret(self):
         """
-        Test virt.pool_delete function where the pool has a secret
+        Test virt.pool_undefine function where the pool has a secret
         """
         mock_pool = MagicMock()
-        mock_pool.delete = MagicMock(return_value=0)
+        mock_pool.undefine = MagicMock(return_value=0)
         mock_pool.XMLDesc.return_value = """
             <pool type='rbd'>
               <name>test-ses</name>
@@ -4284,16 +4375,11 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         mock_undefine = MagicMock(return_value=0)
         self.mock_conn.secretLookupByUsage.return_value.undefine = mock_undefine
 
-        res = virt.pool_delete("test-ses")
+        res = virt.pool_undefine("test-ses")
         self.assertTrue(res)
 
         self.mock_conn.storagePoolLookupByName.assert_called_once_with("test-ses")
-
-        # Shouldn't be called with another parameter so far since those are not implemented
-        # and thus throwing exceptions.
-        mock_pool.delete.assert_called_once_with(
-            self.mock_libvirt.VIR_STORAGE_POOL_DELETE_NORMAL
-        )
+        mock_pool.undefine.assert_called_once_with()
 
         self.mock_conn.secretLookupByUsage.assert_called_once_with(
             self.mock_libvirt.VIR_SECRET_USAGE_TYPE_CEPH, "pool_test-ses"
@@ -4562,24 +4648,6 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
           </source>
         </pool>"""
 
-        expected_xml = (
-            '<pool type="rbd">'
-            "<name>default</name>"
-            "<uuid>20fbe05c-ab40-418a-9afa-136d512f0ede</uuid>"
-            '<capacity unit="bytes">1999421108224</capacity>'
-            '<allocation unit="bytes">713207042048</allocation>'
-            '<available unit="bytes">1286214066176</available>'
-            "<source>"
-            '<host name="ses4.tf.local" />'
-            '<host name="ses5.tf.local" />'
-            '<auth type="ceph" username="libvirt">'
-            '<secret uuid="14e9a0f1-8fbf-4097-b816-5b094c182212" />'
-            "</auth>"
-            "<name>iscsi-images</name>"
-            "</source>"
-            "</pool>"
-        )
-
         mock_secret = MagicMock()
         self.mock_conn.secretLookupByUUIDString = MagicMock(return_value=mock_secret)
 
@@ -4600,6 +4668,23 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         self.mock_conn.storagePoolDefineXML.assert_not_called()
         mock_secret.setValue.assert_called_once_with(b"secret")
 
+        # Case where the secret can't be found
+        self.mock_conn.secretLookupByUUIDString = MagicMock(
+            side_effect=self.mock_libvirt.libvirtError("secret not found")
+        )
+        self.assertFalse(
+            virt.pool_update(
+                "default",
+                "rbd",
+                source_name="iscsi-images",
+                source_hosts=["ses4.tf.local", "ses5.tf.local"],
+                source_auth={"username": "libvirt", "password": "c2VjcmV0"},
+            )
+        )
+        self.mock_conn.storagePoolDefineXML.assert_not_called()
+        self.mock_conn.secretDefineXML.assert_called_once()
+        mock_secret.setValue.assert_called_once_with(b"secret")
+
     def test_pool_update_password_create(self):
         """
         Test the pool_update function, where the password only is changed
diff --git a/tests/unit/states/test_virt.py b/tests/unit/states/test_virt.py
index 6d38829870..8fe892f607 100644
--- a/tests/unit/states/test_virt.py
+++ b/tests/unit/states/test_virt.py
@@ -333,6 +333,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                         "myvm",
                         cpu=2,
                         mem=2048,
+                        boot_dev="cdrom hd",
                         os_type="linux",
                         arch="i686",
                         vm_type="qemu",
@@ -355,6 +356,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     "myvm",
                     cpu=2,
                     mem=2048,
+                    boot_dev="cdrom hd",
                     os_type="linux",
                     arch="i686",
                     disk="prod",
@@ -463,10 +465,13 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                         "comment": "Domain myvm updated with live update(s) failures",
                     }
                 )
-                self.assertDictEqual(virt.defined("myvm", cpu=2), ret)
+                self.assertDictEqual(
+                    virt.defined("myvm", cpu=2, boot_dev="cdrom hd"), ret
+                )
                 update_mock.assert_called_with(
                     "myvm",
                     cpu=2,
+                    boot_dev="cdrom hd",
                     mem=None,
                     disk_profile=None,
                     disks=None,
@@ -590,6 +595,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     password=None,
                     boot=None,
                     test=True,
+                    boot_dev=None,
                 )
 
             # No changes case
@@ -624,6 +630,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     password=None,
                     boot=None,
                     test=True,
+                    boot_dev=None,
                 )
 
     def test_running(self):
@@ -700,6 +707,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     install=True,
                     pub_key=None,
                     priv_key=None,
+                    boot_dev=None,
                     connection=None,
                     username=None,
                     password=None,
@@ -761,6 +769,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                         install=False,
                         pub_key="/path/to/key.pub",
                         priv_key="/path/to/key",
+                        boot_dev="network hd",
                         connection="someconnection",
                         username="libvirtuser",
                         password="supersecret",
@@ -785,6 +794,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     start=False,
                     pub_key="/path/to/key.pub",
                     priv_key="/path/to/key",
+                    boot_dev="network hd",
                     connection="someconnection",
                     username="libvirtuser",
                     password="supersecret",
@@ -929,6 +939,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     password=None,
                     boot=None,
                     test=False,
+                    boot_dev=None,
                 )
 
             # Failed definition update case
@@ -1047,6 +1058,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     password=None,
                     boot=None,
                     test=True,
+                    boot_dev=None,
                 )
                 start_mock.assert_not_called()
 
@@ -1083,6 +1095,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     password=None,
                     boot=None,
                     test=True,
+                    boot_dev=None,
                 )
 
     def test_stopped(self):
@@ -1970,6 +1983,72 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     password="secret",
                 )
 
+            # Define a pool that doesn't handle build
+            for mock in mocks:
+                mocks[mock].reset_mock()
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.pool_info": MagicMock(
+                        side_effect=[
+                            {},
+                            {"mypool": {"state": "stopped", "autostart": True}},
+                        ]
+                    ),
+                    "virt.pool_define": mocks["define"],
+                    "virt.pool_build": mocks["build"],
+                    "virt.pool_set_autostart": mocks["autostart"],
+                },
+            ):
+                ret.update(
+                    {
+                        "changes": {"mypool": "Pool defined, marked for autostart"},
+                        "comment": "Pool mypool defined, marked for autostart",
+                    }
+                )
+                self.assertDictEqual(
+                    virt.pool_defined(
+                        "mypool",
+                        ptype="rbd",
+                        source={
+                            "name": "libvirt-pool",
+                            "hosts": ["ses2.tf.local", "ses3.tf.local"],
+                            "auth": {
+                                "username": "libvirt",
+                                "password": "AQAz+PRdtquBBRAASMv7nlMZYfxIyLw3St65Xw==",
+                            },
+                        },
+                        autostart=True,
+                    ),
+                    ret,
+                )
+                mocks["define"].assert_called_with(
+                    "mypool",
+                    ptype="rbd",
+                    target=None,
+                    permissions=None,
+                    source_devices=None,
+                    source_dir=None,
+                    source_adapter=None,
+                    source_hosts=["ses2.tf.local", "ses3.tf.local"],
+                    source_auth={
+                        "username": "libvirt",
+                        "password": "AQAz+PRdtquBBRAASMv7nlMZYfxIyLw3St65Xw==",
+                    },
+                    source_name="libvirt-pool",
+                    source_format=None,
+                    source_initiator=None,
+                    start=False,
+                    transient=False,
+                    connection=None,
+                    username=None,
+                    password=None,
+                )
+                mocks["autostart"].assert_called_with(
+                    "mypool", state="on", connection=None, username=None, password=None,
+                )
+                mocks["build"].assert_not_called()
+
             mocks["update"] = MagicMock(return_value=False)
             for mock in mocks:
                 mocks[mock].reset_mock()
@@ -2019,6 +2098,9 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
             for mock in mocks:
                 mocks[mock].reset_mock()
             mocks["update"] = MagicMock(return_value=True)
+            mocks["build"] = MagicMock(
+                side_effect=self.mock_libvirt.libvirtError("Existing VG")
+            )
             with patch.dict(
                 virt.__salt__,
                 {  # pylint: disable=no-member
@@ -2122,6 +2204,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     ),
                     ret,
                 )
+                mocks["build"].assert_not_called()
                 mocks["update"].assert_called_with(
                     "mypool",
                     ptype="logical",
@@ -2469,8 +2552,8 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
             ):
                 ret.update(
                     {
-                        "changes": {"mypool": "Pool updated, built, restarted"},
-                        "comment": "Pool mypool updated, built, restarted",
+                        "changes": {"mypool": "Pool updated, restarted"},
+                        "comment": "Pool mypool updated, restarted",
                         "result": True,
                     }
                 )
@@ -2496,9 +2579,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                 mocks["start"].assert_called_with(
                     "mypool", connection=None, username=None, password=None
                 )
-                mocks["build"].assert_called_with(
-                    "mypool", connection=None, username=None, password=None
-                )
+                mocks["build"].assert_not_called()
                 mocks["update"].assert_called_with(
                     "mypool",
                     ptype="logical",
diff --git a/tests/unit/utils/test_data.py b/tests/unit/utils/test_data.py
index 9206979284..aff7384232 100644
--- a/tests/unit/utils/test_data.py
+++ b/tests/unit/utils/test_data.py
@@ -220,38 +220,6 @@ class DataTestCase(TestCase):
             ),
         )
 
-        # Traverse and match integer key in a nested dict
-        # https://github.com/saltstack/salt/issues/56444
-        self.assertEqual(
-            "it worked",
-            salt.utils.data.traverse_dict_and_list(
-                {"foo": {1234: "it worked"}}, "foo:1234", "it didn't work",
-            ),
-        )
-        # Make sure that we properly return the default value when the initial
-        # attempt fails and YAML-loading the target key doesn't change its
-        # value.
-        self.assertEqual(
-            "default",
-            salt.utils.data.traverse_dict_and_list(
-                {"foo": {"baz": "didn't work"}}, "foo:bar", "default",
-            ),
-        )
-
-    def test_issue_39709(self):
-        test_two_level_dict_and_list = {
-            "foo": ["bar", "baz", {"lorem": {"ipsum": [{"dolor": "sit"}]}}]
-        }
-
-        self.assertEqual(
-            "sit",
-            salt.utils.data.traverse_dict_and_list(
-                test_two_level_dict_and_list,
-                ["foo", "lorem", "ipsum", "dolor"],
-                {"not_found": "not_found"},
-            ),
-        )
-
     def test_compare_dicts(self):
         ret = salt.utils.data.compare_dicts(old={"foo": "bar"}, new={"foo": "bar"})
         self.assertEqual(ret, {})
-- 
2.29.2

openSUSE Build Service is sponsored by