salt/opensuse-3000-virt-defined-states-222.patch

From 8deed909147041f8befad8fee9d27bb81595ed23 Mon Sep 17 00:00:00 2001
From: Cedric Bosdonnat <cbosdonnat@suse.com>
Date: Fri, 13 Mar 2020 16:38:08 +0100
Subject: [PATCH] openSUSE-3000 virt-defined-states (#222)

* Create virt.pool_defined state out of virt.pool_running

Users may want to use states to ensure a virtual storage pool is defined
and not enforce it to be running. Extract the code that performs the
pool definition / update from virt.pool_running state into a
virt.pool_defined.

Obviously the virt.pool_running state calls the virt.pool_defined one.
In such a case no additionnal test is needed for virt.pool_defined since
this is already tested with virt.pool_running.

* Add virt.update test parameter

In order to allow running dry-runs of virt.update module add a test
parameter. This will later be used by the virt states.

* Extract virt.defined state from virt.running

In order to ensure a virtual guest is defined independently of its
status, extract the corresponding code from the virt.running state.

This commit also handles the __opts__['test'] for the running state.
Since the update call only performs changes if needed, deprecate the
update parameter.

* Extract virt.network_defined from virt.network_running

Just like domains and storage pools, users may want to ensure a network
is defined without influencing it's status. Extract the code from
network_running state defining the network into a network_defined state.

While at it, support __opt__['test'] == True in these states. Updating
the network definition in the pool_defined state will come in a future
PR.

* Fix virt.update to handle None mem and cpu

virt.running state now may call virt.update with None mem and cpu
parameters. This was not handled in _gen_xml(). Also add some more tests
cases matching this for virt.update.
---
 salt/modules/virt.py            |  44 +-
 salt/states/virt.py             | 268 +++++++---
 tests/unit/modules/test_virt.py | 845 +-----------------------------
 tests/unit/states/test_virt.py  | 893 +++++++++++++++++++++++++++-----
 4 files changed, 971 insertions(+), 1079 deletions(-)

diff --git a/salt/modules/virt.py b/salt/modules/virt.py
index 362c2a68b5..7314bf1d6e 100644
--- a/salt/modules/virt.py
+++ b/salt/modules/virt.py
@@ -2579,7 +2579,6 @@ def update(
     live=True,
     boot=None,
     test=False,
-    boot_dev=None,
     **kwargs
 ):
     """
@@ -2653,17 +2652,9 @@ def update(
 
         .. 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:: 3002
-
     :param test: run in dry-run mode if set to True
 
-        .. versionadded:: 3001
+        .. versionadded:: sodium
 
     :return:
 
@@ -2713,7 +2704,6 @@ def update(
 
     new_desc = ElementTree.fromstring(
         _gen_xml(
-            conn,
             name,
             cpu or 0,
             mem or 0,
@@ -2879,26 +2869,22 @@ def update(
     # Set the new definition
     if need_update:
         # Create missing disks if needed
-        try:
-            if changes["disk"]:
-                for idx, item in enumerate(changes["disk"]["sorted"]):
-                    source_file = all_disks[idx].get("source_file")
-                    # We don't want to create image disks for cdrom devices
-                    if all_disks[idx].get("device", "disk") == "cdrom":
-                        continue
-                    if (
-                        item in changes["disk"]["new"]
-                        and source_file
-                        and not os.path.isfile(source_file)
-                    ):
-                        _qemu_image_create(all_disks[idx])
-                    elif item in changes["disk"]["new"] and not source_file:
-                        _disk_volume_create(conn, all_disks[idx])
+        if changes["disk"]:
+            for idx, item in enumerate(changes["disk"]["sorted"]):
+                source_file = all_disks[idx]["source_file"]
+                if (
+                    item in changes["disk"]["new"]
+                    and source_file
+                    and not os.path.isfile(source_file)
+                    and not test
+                ):
+                    _qemu_image_create(all_disks[idx])
 
+        try:
             if not test:
-                xml_desc = ElementTree.tostring(desc)
-                log.debug("Update virtual machine definition: %s", xml_desc)
-                conn.defineXML(salt.utils.stringutils.to_str(xml_desc))
+                conn.defineXML(
+                    salt.utils.stringutils.to_str(ElementTree.tostring(desc))
+                )
             status["definition"] = True
         except libvirt.libvirtError as err:
             conn.close()
diff --git a/salt/states/virt.py b/salt/states/virt.py
index 200c79d35c..2394d0745e 100644
--- a/salt/states/virt.py
+++ b/salt/states/virt.py
@@ -12,6 +12,7 @@ for the generation and signing of certificates for systems running libvirt:
 """
 
 
+import copy
 import fnmatch
 import logging
 import os
@@ -285,37 +286,15 @@ 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.
 
-    .. versionadded:: 3001
+    .. versionadded:: sodium
 
     :param name: name of the virtual machine to run
     :param cpu: number of CPUs for the virtual machine to create
-    :param mem: Amount of memory to allocate to the virtual machine in MiB. Since 3002, a dictionary can be used to
-        contain detailed configuration which support memory allocation or tuning. Supported parameters are ``boot``,
-        ``current``, ``max``, ``slots``, ``hard_limit``, ``soft_limit``, ``swap_hard_limit`` and ``min_guarantee``. The
-        structure of the dictionary is documented in  :ref:`init-mem-def`. Both decimal and binary base are supported.
-        Detail unit specification is documented  in :ref:`virt-units`. Please note that the value for ``slots`` must be
-        an integer.
-
-        .. code-block:: python
-
-            {
-                'boot': 1g,
-                'current': 1g,
-                'max': 1g,
-                'slots': 10,
-                'hard_limit': '1024'
-                'soft_limit': '512m'
-                'swap_hard_limit': '1g'
-                'min_guarantee': '512mib'
-            }
-
-        .. versionchanged:: 3002
-
+    :param mem: amount of memory in MiB for the new virtual machine
     :param vm_type: force virtual machine type for the new VM. The default value is taken from
         the host capabilities. This could be useful for example to use ``'qemu'`` type instead
         of the ``'kvm'`` one.
@@ -353,27 +332,23 @@ def defined(
         but ``x86_64`` is prefed over ``i686``. Only used when creating a new virtual machine.
 
     :param boot:
-        Specifies kernel, initial ramdisk and kernel command line parameters for the virtual machine.
-        This is an optional parameter, all of the keys are optional within the dictionary.
-
-        Refer to :ref:`init-boot-def` for the complete boot parameters description.
+        Specifies kernel for the virtual machine, as well as boot parameters
+        for the virtual machine. This is an optionl parameter, and all of the
+        keys are optional within the dictionary. If a remote path is provided
+        to kernel or initrd, salt will handle the downloading of the specified
+        remote fild, and will modify the XML accordingly.
 
-        To update any boot parameters, specify the new path for each. To remove any boot parameters,
-        pass a None object, for instance: 'kernel': ``None``.
+        .. code-block:: python
 
-        .. versionadded:: 3000
+            {
+                'kernel': '/root/f8-i386-vmlinuz',
+                'initrd': '/root/f8-i386-initrd',
+                'cmdline': 'console=ttyS0 ks=http://example.com/f8-i386/os/'
+            }
 
     :param update: set to ``False`` to prevent updating a defined domain. (Default: ``True``)
 
-        .. deprecated:: 3001
-
-    :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:: 3002
+        .. deprecated:: sodium
 
     .. rubric:: Example States
 
@@ -385,7 +360,6 @@ def defined(
           virt.defined:
             - cpu: 2
             - mem: 2048
-            - boot_dev: network hd
             - disk_profile: prod
             - disks:
               - name: system
@@ -438,7 +412,6 @@ def defined(
                     password=password,
                     boot=boot,
                     test=__opts__["test"],
-                    boot_dev=boot_dev,
                 )
             ret["changes"][name] = status
             if not status.get("definition"):
@@ -473,7 +446,6 @@ def defined(
                     password=password,
                     boot=boot,
                     start=False,
-                    boot_dev=boot_dev,
                 )
             ret["changes"][name] = {"definition": True}
             ret["comment"] = "Domain {} defined".format(name)
@@ -489,6 +461,7 @@ def running(
     name,
     cpu=None,
     mem=None,
+    image=None,
     vm_type=None,
     disk_profile=None,
     disks=None,
@@ -506,7 +479,6 @@ 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,7 +556,7 @@ def running(
     :param update: set to ``True`` to update a defined domain. (Default: ``False``)
 
         .. versionadded:: 2019.2.0
-        .. deprecated:: 3001
+        .. deprecated:: sodium
     :param connection: libvirt connection URI, overriding defaults
 
         .. versionadded:: 2019.2.0
@@ -676,10 +648,32 @@ def running(
 
     """
     merged_disks = disks
+    if image:
+        default_disks = [{"system": {}}]
+        disknames = ["system"]
+        if disk_profile:
+            disklist = copy.deepcopy(
+                __salt__["config.get"]("virt:disk", {}).get(disk_profile, default_disks)
+            )
+            disknames = disklist.keys()
+        disk = {"name": disknames[0], "image": image}
+        if merged_disks:
+            first_disk = [d for d in merged_disks if d.get("name") == disknames[0]]
+            if first_disk and "image" not in first_disk[0]:
+                first_disk[0]["image"] = image
+            else:
+                merged_disks.append(disk)
+        else:
+            merged_disks = [disk]
+        salt.utils.versions.warn_until(
+            "Sodium",
+            "'image' parameter has been deprecated. Rather use the 'disks' parameter "
+            "to override or define the image. 'image' will be removed in {version}.",
+        )
 
     if not update:
         salt.utils.versions.warn_until(
-            "Aluminium",
+            "Magnesium",
             "'update' parameter has been deprecated. Future behavior will be the one of update=True"
             "It will be removed in {version}.",
         )
@@ -701,7 +695,6 @@ def running(
         arch=arch,
         boot=boot,
         update=update,
-        boot_dev=boot_dev,
         connection=connection,
         username=username,
         password=password,
@@ -953,7 +946,7 @@ def network_defined(
     :param username: username to connect with, overriding defaults
     :param password: password to connect with, overriding defaults
 
-    .. versionadded:: 3001
+    .. versionadded:: sodium
 
     .. code-block:: yaml
 
@@ -1170,7 +1163,7 @@ def pool_defined(
     """
     Defines a new pool with specified arguments.
 
-    .. versionadded:: 3001
+    .. versionadded:: sodium
 
     :param ptype: libvirt pool type
     :param target: full path to the target device or folder. (Default: ``None``)
@@ -1269,24 +1262,14 @@ def pool_defined(
 
                 action = ""
                 if info[name]["state"] != "running":
-                    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"
+                    if not __opts__["test"]:
+                        __salt__["virt.pool_build"](
+                            name,
+                            connection=connection,
+                            username=username,
+                            password=password,
+                        )
+                    action = ", built"
 
                 action = (
                     "{}, autostart flag changed".format(action)
@@ -1322,22 +1305,9 @@ def pool_defined(
                     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(),
-                        )
+                __salt__["virt.pool_build"](
+                    name, connection=connection, username=username, password=password
+                )
             if needs_autostart:
                 ret["changes"][name] = "Pool defined, marked for autostart"
                 ret["comment"] = "Pool {} defined, marked for autostart".format(name)
@@ -1494,6 +1464,138 @@ def pool_running(
     return ret
 
 
+def pool_running(
+    name,
+    ptype=None,
+    target=None,
+    permissions=None,
+    source=None,
+    transient=False,
+    autostart=True,
+    connection=None,
+    username=None,
+    password=None,
+):
+    """
+    Defines and starts a new pool with specified arguments.
+
+    .. versionadded:: 2019.2.0
+
+    :param ptype: libvirt pool type
+    :param target: full path to the target device or folder. (Default: ``None``)
+    :param permissions:
+        target permissions. See :ref:`pool-define-permissions` for more details on this structure.
+    :param source:
+        dictionary containing keys matching the ``source_*`` parameters in function
+        :func:`salt.modules.virt.pool_define`.
+    :param transient:
+        when set to ``True``, the pool will be automatically undefined after being stopped. (Default: ``False``)
+    :param autostart:
+        Whether to start the pool when booting the host. (Default: ``True``)
+    :param start:
+        When ``True``, define and start the pool, otherwise the pool will be left stopped.
+    :param connection: libvirt connection URI, overriding defaults
+    :param username: username to connect with, overriding defaults
+    :param password: password to connect with, overriding defaults
+
+    .. code-block:: yaml
+
+        pool_name:
+          virt.pool_running
+
+    .. code-block:: yaml
+
+        pool_name:
+          virt.pool_running:
+            - ptype: netfs
+            - target: /mnt/cifs
+            - permissions:
+                - mode: 0770
+                - owner: 1000
+                - group: 100
+            - source:
+                dir: samba_share
+                hosts:
+                  - one.example.com
+                  - two.example.com
+                format: cifs
+            - autostart: True
+
+    """
+    ret = pool_defined(
+        name,
+        ptype=ptype,
+        target=target,
+        permissions=permissions,
+        source=source,
+        transient=transient,
+        autostart=autostart,
+        connection=connection,
+        username=username,
+        password=password,
+    )
+    defined = name in ret["changes"] and ret["changes"][name].startswith("Pool defined")
+    updated = name in ret["changes"] and ret["changes"][name].startswith("Pool updated")
+
+    result = True if not __opts__["test"] else None
+    if ret["result"] is None or ret["result"]:
+        try:
+            info = __salt__["virt.pool_info"](
+                name, connection=connection, username=username, password=password
+            )
+            action = "started"
+            # In the corner case where test=True and the pool wasn't defined
+            # we may get not get our pool in the info dict and that is normal.
+            is_running = info.get(name, {}).get("state", "stopped") == "running"
+            if is_running:
+                if updated:
+                    action = "built, restarted"
+                    if not __opts__["test"]:
+                        __salt__["virt.pool_stop"](
+                            name,
+                            connection=connection,
+                            username=username,
+                            password=password,
+                        )
+                    if not __opts__["test"]:
+                        __salt__["virt.pool_build"](
+                            name,
+                            connection=connection,
+                            username=username,
+                            password=password,
+                        )
+                else:
+                    action = "already running"
+                    result = True
+
+            if not is_running or updated or defined:
+                if not __opts__["test"]:
+                    __salt__["virt.pool_start"](
+                        name,
+                        connection=connection,
+                        username=username,
+                        password=password,
+                    )
+
+            comment = "Pool {}".format(name)
+            change = "Pool"
+            if name in ret["changes"]:
+                comment = "{},".format(ret["comment"])
+                change = "{},".format(ret["changes"][name])
+
+            if action != "already running":
+                ret["changes"][name] = "{} {}".format(change, action)
+
+            ret["comment"] = "{} {}".format(comment, action)
+            ret["result"] = result
+
+        except libvirt.libvirtError as err:
+            ret["comment"] = err.get_error_message()
+            ret["result"] = False
+
+    return ret
+
+
 def pool_deleted(name, purge=False, connection=None, username=None, password=None):
     """
     Deletes a virtual storage pool.
diff --git a/tests/unit/modules/test_virt.py b/tests/unit/modules/test_virt.py
index e9e73d7b5d..db6ba007b7 100644
--- a/tests/unit/modules/test_virt.py
+++ b/tests/unit/modules/test_virt.py
@@ -1849,40 +1849,21 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         self.assertEqual(
             {
                 "definition": False,
-                "disk": {"attached": [], "detached": [], "updated": []},
+                "disk": {"attached": [], "detached": []},
                 "interface": {"attached": [], "detached": []},
             },
-            virt.update("my_vm"),
+            virt.update("my vm"),
         )
 
-        # mem + cpu case
-        define_mock.reset_mock()
-        domain_mock.setMemoryFlags.return_value = 0
-        domain_mock.setVcpusFlags.return_value = 0
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-                "mem": True,
-                "cpu": True,
-            },
-            virt.update("my_vm", mem=2048, cpu=2),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual("2", setxml.find("vcpu").text)
-        self.assertEqual("2147483648", setxml.find("memory").text)
-        self.assertEqual(2048 * 1024, domain_mock.setMemoryFlags.call_args[0][0])
-
         # Same parameters passed than in default virt.defined state case
         self.assertEqual(
             {
                 "definition": False,
-                "disk": {"attached": [], "detached": [], "updated": []},
+                "disk": {"attached": [], "detached": []},
                 "interface": {"attached": [], "detached": []},
             },
             virt.update(
-                "my_vm",
+                "my vm",
                 cpu=None,
                 mem=None,
                 disk_profile=None,
@@ -1905,829 +1886,15 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
             {
                 "definition": True,
                 "cpu": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
+                "disk": {"attached": [], "detached": []},
                 "interface": {"attached": [], "detached": []},
             },
-            virt.update("my_vm", cpu=2),
+            virt.update("my vm", cpu=2),
         )
         setxml = ET.fromstring(define_mock.call_args[0][0])
         self.assertEqual(setxml.find("vcpu").text, "2")
         self.assertEqual(setvcpus_mock.call_args[0][0], 2)
 
-        boot = {
-            "kernel": "/root/f8-i386-vmlinuz",
-            "initrd": "/root/f8-i386-initrd",
-            "cmdline": "console=ttyS0 ks=http://example.com/f8-i386/os/",
-        }
-
-        # 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")],
-        )
-
-        # 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,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", boot=boot),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(setxml.find("os").find("kernel").text, "/root/f8-i386-vmlinuz")
-        self.assertEqual(setxml.find("os").find("initrd").text, "/root/f8-i386-initrd")
-        self.assertEqual(
-            setxml.find("os").find("cmdline").text,
-            "console=ttyS0 ks=http://example.com/f8-i386/os/",
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(setxml.find("os").find("kernel").text, "/root/f8-i386-vmlinuz")
-        self.assertEqual(setxml.find("os").find("initrd").text, "/root/f8-i386-initrd")
-        self.assertEqual(
-            setxml.find("os").find("cmdline").text,
-            "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,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", boot=boot_uefi),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(
-            setxml.find("os").find("loader").text, "/usr/share/OVMF/OVMF_CODE.fd"
-        )
-        self.assertEqual(setxml.find("os").find("loader").attrib.get("readonly"), "yes")
-        self.assertEqual(setxml.find("os").find("loader").attrib["type"], "pflash")
-        self.assertEqual(
-            setxml.find("os").find("nvram").attrib["template"],
-            "/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 memtune parameter case
-        memtune = {
-            "soft_limit": "0.5g",
-            "hard_limit": "1024",
-            "swap_hard_limit": "2048m",
-            "min_guarantee": "1 g",
-        }
-
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", mem=memtune),
-        )
-
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(
-            setxml.find("memtune").find("soft_limit").text, str(int(0.5 * 1024 ** 3))
-        )
-        self.assertEqual(setxml.find("memtune").find("soft_limit").get("unit"), "bytes")
-        self.assertEqual(
-            setxml.find("memtune").find("hard_limit").text, str(1024 * 1024 ** 2)
-        )
-        self.assertEqual(
-            setxml.find("memtune").find("swap_hard_limit").text, str(2048 * 1024 ** 2)
-        )
-        self.assertEqual(
-            setxml.find("memtune").find("min_guarantee").text, str(1 * 1024 ** 3)
-        )
-
-        invalid_unit = {"soft_limit": "2HB"}
-
-        with self.assertRaises(SaltInvocationError):
-            virt.update("my_vm", mem=invalid_unit)
-
-        invalid_number = {
-            "soft_limit": "3.4.MB",
-        }
-
-        with self.assertRaises(SaltInvocationError):
-            virt.update("my_vm", mem=invalid_number)
-
-        # Update memory case
-        setmem_mock = MagicMock(return_value=0)
-        domain_mock.setMemoryFlags = setmem_mock
-
-        self.assertEqual(
-            {
-                "definition": True,
-                "mem": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", mem=2048),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(setxml.find("memory").text, str(2048 * 1024 ** 2))
-        self.assertEqual(setxml.find("memory").get("unit"), "bytes")
-        self.assertEqual(setmem_mock.call_args[0][0], 2048 * 1024)
-
-        mem_dict = {"boot": "0.5g", "current": "2g", "max": "1g", "slots": 12}
-        self.assertEqual(
-            {
-                "definition": True,
-                "mem": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", mem=mem_dict),
-        )
-
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(setxml.find("memory").get("unit"), "bytes")
-        self.assertEqual(setxml.find("memory").text, str(int(0.5 * 1024 ** 3)))
-        self.assertEqual(setxml.find("maxMemory").text, str(1 * 1024 ** 3))
-        self.assertEqual(setxml.find("currentMemory").text, str(2 * 1024 ** 3))
-
-        max_slot_reverse = {
-            "slots": "10",
-            "max": "3096m",
-        }
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", mem=max_slot_reverse),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(setxml.find("maxMemory").text, str(3096 * 1024 ** 2))
-        self.assertEqual(setxml.find("maxMemory").attrib.get("slots"), "10")
-
-        # Update disks case
-        devattach_mock = MagicMock(return_value=0)
-        devdetach_mock = MagicMock(return_value=0)
-        domain_mock.attachDevice = devattach_mock
-        domain_mock.detachDevice = devdetach_mock
-        mock_chmod = MagicMock()
-        mock_run = MagicMock()
-        with patch.dict(
-            os.__dict__, {"chmod": mock_chmod, "makedirs": MagicMock()}
-        ):  # pylint: disable=no-member
-            with patch.dict(
-                virt.__salt__, {"cmd.run": mock_run}
-            ):  # pylint: disable=no-member
-                ret = virt.update(
-                    "my_vm",
-                    disk_profile="default",
-                    disks=[
-                        {
-                            "name": "cddrive",
-                            "device": "cdrom",
-                            "source_file": None,
-                            "model": "ide",
-                        },
-                        {"name": "added", "size": 2048},
-                    ],
-                )
-                added_disk_path = os.path.join(
-                    virt.__salt__["config.get"]("virt:images"), "my_vm_added.qcow2"
-                )  # pylint: disable=no-member
-                self.assertEqual(
-                    mock_run.call_args[0][0],
-                    'qemu-img create -f qcow2 "{}" 2048M'.format(added_disk_path),
-                )
-                self.assertEqual(mock_chmod.call_args[0][0], added_disk_path)
-                self.assertListEqual(
-                    [None, os.path.join(root_dir, "my_vm_added.qcow2")],
-                    [
-                        ET.fromstring(disk).find("source").get("file")
-                        if str(disk).find("<source") > -1
-                        else None
-                        for disk in ret["disk"]["attached"]
-                    ],
-                )
-
-                self.assertListEqual(
-                    ["my_vm_data", "libvirt-pool/my_vm_data2"],
-                    [
-                        ET.fromstring(disk).find("source").get("volume")
-                        or ET.fromstring(disk).find("source").get("name")
-                        for disk in ret["disk"]["detached"]
-                    ],
-                )
-                self.assertEqual(devattach_mock.call_count, 2)
-                self.assertEqual(devdetach_mock.call_count, 2)
-
-        # Update nics case
-        yaml_config = """
-          virt:
-             nic:
-                myprofile:
-                   - network: default
-                     name: eth0
-        """
-        mock_config = salt.utils.yaml.safe_load(yaml_config)
-        devattach_mock.reset_mock()
-        devdetach_mock.reset_mock()
-        with patch.dict(
-            salt.modules.config.__opts__, mock_config  # pylint: disable=no-member
-        ):
-            ret = virt.update(
-                "my_vm",
-                nic_profile="myprofile",
-                interfaces=[
-                    {
-                        "name": "eth0",
-                        "type": "network",
-                        "source": "default",
-                        "mac": "52:54:00:39:02:b1",
-                    },
-                    {"name": "eth1", "type": "network", "source": "newnet"},
-                ],
-            )
-            self.assertEqual(
-                ["newnet"],
-                [
-                    ET.fromstring(nic).find("source").get("network")
-                    for nic in ret["interface"]["attached"]
-                ],
-            )
-            self.assertEqual(
-                ["oldnet"],
-                [
-                    ET.fromstring(nic).find("source").get("network")
-                    for nic in ret["interface"]["detached"]
-                ],
-            )
-            devattach_mock.assert_called_once()
-            devdetach_mock.assert_called_once()
-
-        # Remove nics case
-        devattach_mock.reset_mock()
-        devdetach_mock.reset_mock()
-        ret = virt.update("my_vm", nic_profile=None, interfaces=[])
-        self.assertEqual([], ret["interface"]["attached"])
-        self.assertEqual(2, len(ret["interface"]["detached"]))
-        devattach_mock.assert_not_called()
-        devdetach_mock.assert_called()
-
-        # Remove disks case (yeah, it surely is silly)
-        devattach_mock.reset_mock()
-        devdetach_mock.reset_mock()
-        ret = virt.update("my_vm", disk_profile=None, disks=[])
-        self.assertEqual([], ret["disk"]["attached"])
-        self.assertEqual(3, len(ret["disk"]["detached"]))
-        devattach_mock.assert_not_called()
-        devdetach_mock.assert_called()
-
-        # Graphics change test case
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", graphics={"type": "vnc"}),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual("vnc", setxml.find("devices/graphics").get("type"))
-
-        # Update with no diff case
-        pool_mock = MagicMock()
-        default_pool_desc = "<pool type='dir'></pool>"
-        rbd_pool_desc = """
-            <pool type='rbd'>
-              <name>test-rbd</name>
-              <source>
-                <host name='ses2.tf.local'/>
-                <host name='ses3.tf.local' port='1234'/>
-                <name>libvirt-pool</name>
-                <auth type='ceph' username='libvirt'>
-                  <secret usage='pool_test-rbd'/>
-                </auth>
-              </source>
-            </pool>
-            """
-        pool_mock.XMLDesc.side_effect = [
-            default_pool_desc,
-            rbd_pool_desc,
-            default_pool_desc,
-            rbd_pool_desc,
-        ]
-        self.mock_conn.storagePoolLookupByName.return_value = pool_mock
-        self.mock_conn.listStoragePools.return_value = ["test-rbd", "default"]
-        self.assertEqual(
-            {
-                "definition": False,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update(
-                "my_vm",
-                cpu=1,
-                mem=1024,
-                disk_profile="default",
-                disks=[
-                    {"name": "data", "size": 2048, "pool": "default"},
-                    {
-                        "name": "data2",
-                        "size": 4096,
-                        "pool": "test-rbd",
-                        "format": "raw",
-                    },
-                ],
-                nic_profile="myprofile",
-                interfaces=[
-                    {
-                        "name": "eth0",
-                        "type": "network",
-                        "source": "default",
-                        "mac": "52:54:00:39:02:b1",
-                    },
-                    {"name": "eth1", "type": "network", "source": "oldnet"},
-                ],
-                graphics={
-                    "type": "spice",
-                    "listen": {"type": "address", "address": "127.0.0.1"},
-                },
-            ),
-        )
-
-        # Failed XML description update case
-        self.mock_conn.defineXML.side_effect = self.mock_libvirt.libvirtError(
-            "Test error"
-        )
-        setmem_mock.reset_mock()
-        with self.assertRaises(self.mock_libvirt.libvirtError):
-            virt.update("my_vm", mem=2048)
-
-        # Failed single update failure case
-        self.mock_conn.defineXML = MagicMock(return_value=True)
-        setmem_mock.side_effect = self.mock_libvirt.libvirtError(
-            "Failed to live change memory"
-        )
-        self.assertEqual(
-            {
-                "definition": True,
-                "errors": ["Failed to live change memory"],
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", mem=2048),
-        )
-
-        # Failed multiple updates failure case
-        self.assertEqual(
-            {
-                "definition": True,
-                "errors": ["Failed to live change memory"],
-                "cpu": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", cpu=4, mem=2048),
-        )
-
-    def test_update_backing_store(self):
-        """
-        Test updating a disk with a backing store
-        """
-        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='volume' device='disk'>
-                  <driver name='qemu' type='qcow2' cache='none' io='native'/>
-                  <source pool='default' volume='my_vm_system' index='1'/>
-                  <backingStore type='file' index='2'>
-                    <format type='qcow2'/>
-                    <source file='/path/to/base.qcow2'/>
-                    <backingStore/>
-                  </backingStore>
-                  <target dev='vda' bus='virtio'/>
-                  <alias name='virtio-disk0'/>
-                  <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
-                </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
-        self.mock_conn.listStoragePools.return_value = ["default"]
-        self.mock_conn.storagePoolLookupByName.return_value.XMLDesc.return_value = (
-            "<pool type='dir'/>"
-        )
-
-        ret = virt.update(
-            "my_vm",
-            disks=[
-                {
-                    "name": "system",
-                    "pool": "default",
-                    "backing_store_path": "/path/to/base.qcow2",
-                    "backing_store_format": "qcow2",
-                },
-            ],
-        )
-        self.assertFalse(ret["definition"])
-        self.assertFalse(ret["disk"]["attached"])
-        self.assertFalse(ret["disk"]["detached"])
-
-    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='network' device='cdrom'>
-                  <driver name='qemu' type='raw' cache='none' io='native'/>
-                  <source protocol='https' name='/dvd-image-1.iso'>
-                    <host name='test-srv.local' port='80'/>
-                  </source>
-                  <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>
-                <disk type='network' device='cdrom'>
-                  <driver name='qemu' type='raw' cache='none' io='native'/>
-                  <source protocol='https' name='/dvd-image-6.iso'>
-                    <host name='test-srv.local' port='80'/>
-                  </source>
-                  <backingStore/>
-                  <target dev='hde' bus='ide'/>
-                  <readonly/>
-                </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",
-                },
-                {
-                    "name": "dvd5",
-                    "device": "cdrom",
-                    "source_file": "/srv/dvd-image-6.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"},
-                },
-                {
-                    "type": "file",
-                    "device": "cdrom",
-                    "driver": {
-                        "name": "qemu",
-                        "type": "raw",
-                        "cache": "none",
-                        "io": "native",
-                    },
-                    "backingStore": None,
-                    "target": {"dev": "hde", "bus": "ide"},
-                    "readonly": None,
-                    "source": {"file": "/srv/dvd-image-6.iso"},
-                },
-            ],
-            [
-                salt.utils.xmlutil.to_dict(ET.fromstring(disk), True)
-                for disk in ret["disk"]["updated"]
-            ],
-        )
-
-    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.
-        """
-        xml_boot = """
-            <domain type='kvm' id='8'>
-              <name>vm_with_boot_param</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>
-                <kernel>/boot/oldkernel</kernel>
-                <initrd>/boot/initrdold.img</initrd>
-                <cmdline>console=ttyS0 ks=http://example.com/old/os/</cmdline>
-                <loader>/usr/share/old/OVMF_CODE.fd</loader>
-                <nvram>/usr/share/old/OVMF_VARS.ms.fd</nvram>
-              </os>
-              <devices>
-                <disk type='file' device='disk'>
-                  <driver name='qemu' type='qcow2'/>
-                  <source file='{0}{1}vm_with_boot_param_system.qcow2'/>
-                  <backingStore/>
-                  <target dev='vda' bus='virtio'/>
-                  <alias name='virtio-disk0'/>
-                  <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/>
-                </disk>
-                <disk type='file' device='disk'>
-                  <driver name='qemu' type='qcow2'/>
-                  <source file='{0}{1}vm_with_boot_param_data.qcow2'/>
-                  <backingStore/>
-                  <target dev='vdb' bus='virtio'/>
-                  <alias name='virtio-disk1'/>
-                  <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x1'/>
-                </disk>
-                <interface type='network'>
-                  <mac address='52:54:00:39:02:b1'/>
-                  <source network='default' bridge='virbr0'/>
-                  <target dev='vnet0'/>
-                  <model type='virtio'/>
-                  <alias name='net0'/>
-                  <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
-                </interface>
-                <interface type='network'>
-                  <mac address='52:54:00:39:02:b2'/>
-                  <source network='oldnet' bridge='virbr1'/>
-                  <target dev='vnet1'/>
-                  <model type='virtio'/>
-                  <alias name='net1'/>
-                  <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x1'/>
-                </interface>
-                <graphics type='spice' port='5900' autoport='yes' listen='127.0.0.1'>
-                  <listen type='address' address='127.0.0.1'/>
-                </graphics>
-                <video>
-                  <model type='qxl' ram='65536' vram='65536' vgamem='16384' heads='1' primary='yes'/>
-                  <alias name='video0'/>
-                  <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
-                </video>
-              </devices>
-            </domain>
-        """
-        domain_mock_boot = self.set_mock_vm("vm_with_boot_param", xml_boot)
-        domain_mock_boot.OSType = MagicMock(return_value="hvm")
-        define_mock_boot = MagicMock(return_value=True)
-        self.mock_conn.defineXML = define_mock_boot
-        boot_new = {
-            "kernel": "/root/new-vmlinuz",
-            "initrd": "/root/new-initrd",
-            "cmdline": "console=ttyS0 ks=http://example.com/new/os/",
-        }
-
-        uefi_boot_new = {
-            "loader": "/usr/share/new/OVMF_CODE.fd",
-            "nvram": "/usr/share/new/OVMF_VARS.ms.fd",
-        }
-
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("vm_with_boot_param", boot=boot_new),
-        )
-        setxml_boot = ET.fromstring(define_mock_boot.call_args[0][0])
-        self.assertEqual(
-            setxml_boot.find("os").find("kernel").text, "/root/new-vmlinuz"
-        )
-        self.assertEqual(setxml_boot.find("os").find("initrd").text, "/root/new-initrd")
-        self.assertEqual(
-            setxml_boot.find("os").find("cmdline").text,
-            "console=ttyS0 ks=http://example.com/new/os/",
-        )
-
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("vm_with_boot_param", boot=uefi_boot_new),
-        )
-
         setxml = ET.fromstring(define_mock_boot.call_args[0][0])
         self.assertEqual(
             setxml.find("os").find("loader").text, "/usr/share/new/OVMF_CODE.fd"
diff --git a/tests/unit/states/test_virt.py b/tests/unit/states/test_virt.py
index f03159334b..0a47562074 100644
--- a/tests/unit/states/test_virt.py
+++ b/tests/unit/states/test_virt.py
@@ -1,21 +1,15 @@
 """
     :codeauthor: Jayesh Kariya <jayeshk@saltstack.com>
 """
-# Import Python libs
 
 import shutil
 import tempfile
 
-# Import Salt Libs
 import salt.states.virt as virt
 import salt.utils.files
 from salt.exceptions import CommandExecutionError, SaltInvocationError
-
-# Import 3rd-party libs
 from tests.support.mixins import LoaderModuleMockMixin
 from tests.support.mock import MagicMock, mock_open, patch
-
-# Import Salt Testing Libs
 from tests.support.runtests import RUNTIME_VARS
 from tests.support.unit import TestCase
 
@@ -338,7 +332,375 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                         "myvm",
                         cpu=2,
                         mem=2048,
-                        boot_dev="cdrom hd",
+                        boot_dev="cdrom hd",
+                        os_type="linux",
+                        arch="i686",
+                        vm_type="qemu",
+                        disk_profile="prod",
+                        disks=disks,
+                        nic_profile="prod",
+                        interfaces=ifaces,
+                        graphics=graphics,
+                        seed=False,
+                        install=False,
+                        pub_key="/path/to/key.pub",
+                        priv_key="/path/to/key",
+                        connection="someconnection",
+                        username="libvirtuser",
+                        password="supersecret",
+                    ),
+                    ret,
+                )
+                init_mock.assert_called_with(
+                    "myvm",
+                    cpu=2,
+                    mem=2048,
+                    boot_dev="cdrom hd",
+                    os_type="linux",
+                    arch="i686",
+                    disk="prod",
+                    disks=disks,
+                    nic="prod",
+                    interfaces=ifaces,
+                    graphics=graphics,
+                    hypervisor="qemu",
+                    seed=False,
+                    boot=None,
+                    install=False,
+                    start=False,
+                    pub_key="/path/to/key.pub",
+                    priv_key="/path/to/key",
+                    connection="someconnection",
+                    username="libvirtuser",
+                    password="supersecret",
+                )
+
+            # Working update case when running
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.list_domains": MagicMock(return_value=["myvm"]),
+                    "virt.update": MagicMock(
+                        return_value={"definition": True, "cpu": True}
+                    ),
+                },
+            ):
+                ret.update(
+                    {
+                        "changes": {"myvm": {"definition": True, "cpu": True}},
+                        "result": True,
+                        "comment": "Domain myvm updated",
+                    }
+                )
+                self.assertDictEqual(virt.defined("myvm", cpu=2), ret)
+
+            # Working update case when running with boot params
+            boot = {
+                "kernel": "/root/f8-i386-vmlinuz",
+                "initrd": "/root/f8-i386-initrd",
+                "cmdline": "console=ttyS0 ks=http://example.com/f8-i386/os/",
+            }
+
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.list_domains": MagicMock(return_value=["myvm"]),
+                    "virt.update": MagicMock(
+                        return_value={"definition": True, "cpu": True}
+                    ),
+                },
+            ):
+                ret.update(
+                    {
+                        "changes": {"myvm": {"definition": True, "cpu": True}},
+                        "result": True,
+                        "comment": "Domain myvm updated",
+                    }
+                )
+                self.assertDictEqual(virt.defined("myvm", boot=boot), ret)
+
+            # Working update case when stopped
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.list_domains": MagicMock(return_value=["myvm"]),
+                    "virt.update": MagicMock(return_value={"definition": True}),
+                },
+            ):
+                ret.update(
+                    {
+                        "changes": {"myvm": {"definition": True}},
+                        "result": True,
+                        "comment": "Domain myvm updated",
+                    }
+                )
+                self.assertDictEqual(virt.defined("myvm", cpu=2), ret)
+
+            # Failed live update case
+            update_mock = MagicMock(
+                return_value={
+                    "definition": True,
+                    "cpu": False,
+                    "errors": ["some error"],
+                }
+            )
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.list_domains": MagicMock(return_value=["myvm"]),
+                    "virt.update": update_mock,
+                },
+            ):
+                ret.update(
+                    {
+                        "changes": {
+                            "myvm": {
+                                "definition": True,
+                                "cpu": False,
+                                "errors": ["some error"],
+                            }
+                        },
+                        "result": True,
+                        "comment": "Domain myvm updated with live update(s) failures",
+                    }
+                )
+                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,
+                    nic_profile=None,
+                    interfaces=None,
+                    graphics=None,
+                    live=True,
+                    connection=None,
+                    username=None,
+                    password=None,
+                    boot=None,
+                    test=False,
+                )
+
+            # Failed definition update case
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.list_domains": MagicMock(return_value=["myvm"]),
+                    "virt.update": MagicMock(
+                        side_effect=[self.mock_libvirt.libvirtError("error message")]
+                    ),
+                },
+            ):
+                ret.update({"changes": {}, "result": False, "comment": "error message"})
+                self.assertDictEqual(virt.defined("myvm", cpu=2), ret)
+
+        # Test dry-run mode
+        with patch.dict(virt.__opts__, {"test": True}):
+            # Guest defined case
+            init_mock = MagicMock(return_value=True)
+            update_mock = MagicMock(side_effect=CommandExecutionError("not found"))
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.list_domains": MagicMock(return_value=[]),
+                    "virt.init": init_mock,
+                    "virt.update": update_mock,
+                },
+            ):
+                ret.update(
+                    {
+                        "changes": {"myvm": {"definition": True}},
+                        "result": None,
+                        "comment": "Domain myvm defined",
+                    }
+                )
+                disks = [
+                    {
+                        "name": "system",
+                        "size": 8192,
+                        "overlay_image": True,
+                        "pool": "default",
+                        "image": "/path/to/image.qcow2",
+                    },
+                    {"name": "data", "size": 16834},
+                ]
+                ifaces = [
+                    {"name": "eth0", "mac": "01:23:45:67:89:AB"},
+                    {"name": "eth1", "type": "network", "source": "admin"},
+                ]
+                graphics = {
+                    "type": "spice",
+                    "listen": {"type": "address", "address": "192.168.0.1"},
+                }
+                self.assertDictEqual(
+                    virt.defined(
+                        "myvm",
+                        cpu=2,
+                        mem=2048,
+                        os_type="linux",
+                        arch="i686",
+                        vm_type="qemu",
+                        disk_profile="prod",
+                        disks=disks,
+                        nic_profile="prod",
+                        interfaces=ifaces,
+                        graphics=graphics,
+                        seed=False,
+                        install=False,
+                        pub_key="/path/to/key.pub",
+                        priv_key="/path/to/key",
+                        connection="someconnection",
+                        username="libvirtuser",
+                        password="supersecret",
+                    ),
+                    ret,
+                )
+                init_mock.assert_not_called()
+                update_mock.assert_not_called()
+
+            # Guest update case
+            update_mock = MagicMock(return_value={"definition": True})
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.list_domains": MagicMock(return_value=["myvm"]),
+                    "virt.update": update_mock,
+                },
+            ):
+                ret.update(
+                    {
+                        "changes": {"myvm": {"definition": True}},
+                        "result": None,
+                        "comment": "Domain myvm updated",
+                    }
+                )
+                self.assertDictEqual(virt.defined("myvm", cpu=2), ret)
+                update_mock.assert_called_with(
+                    "myvm",
+                    cpu=2,
+                    mem=None,
+                    disk_profile=None,
+                    disks=None,
+                    nic_profile=None,
+                    interfaces=None,
+                    graphics=None,
+                    live=True,
+                    connection=None,
+                    username=None,
+                    password=None,
+                    boot=None,
+                    test=True,
+                    boot_dev=None,
+                )
+
+            # No changes case
+            update_mock = MagicMock(return_value={"definition": False})
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.list_domains": MagicMock(return_value=["myvm"]),
+                    "virt.update": update_mock,
+                },
+            ):
+                ret.update(
+                    {
+                        "changes": {"myvm": {"definition": False}},
+                        "result": True,
+                        "comment": "Domain myvm unchanged",
+                    }
+                )
+                self.assertDictEqual(virt.defined("myvm"), ret)
+                update_mock.assert_called_with(
+                    "myvm",
+                    cpu=None,
+                    mem=None,
+                    disk_profile=None,
+                    disks=None,
+                    nic_profile=None,
+                    interfaces=None,
+                    graphics=None,
+                    live=True,
+                    connection=None,
+                    username=None,
+                    password=None,
+                    boot=None,
+                    test=True,
+                    boot_dev=None,
+                )
+
+    def test_defined(self):
+        """
+        defined state test cases.
+        """
+        ret = {
+            "name": "myvm",
+            "changes": {},
+            "result": True,
+            "comment": "myvm is running",
+        }
+        with patch.dict(virt.__opts__, {"test": False}):
+            # no change test
+            init_mock = MagicMock(return_value=True)
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.list_domains": MagicMock(return_value=["myvm"]),
+                    "virt.update": MagicMock(return_value={"definition": False}),
+                },
+            ):
+                ret.update(
+                    {
+                        "changes": {"myvm": {"definition": False}},
+                        "comment": "Domain myvm unchanged",
+                    }
+                )
+                self.assertDictEqual(virt.defined("myvm"), ret)
+
+            # Test defining a guest with connection details
+            init_mock.reset_mock()
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.list_domains": MagicMock(return_value=[]),
+                    "virt.init": init_mock,
+                    "virt.update": MagicMock(
+                        side_effect=CommandExecutionError("not found")
+                    ),
+                },
+            ):
+                ret.update(
+                    {
+                        "changes": {"myvm": {"definition": True}},
+                        "comment": "Domain myvm defined",
+                    }
+                )
+                disks = [
+                    {
+                        "name": "system",
+                        "size": 8192,
+                        "overlay_image": True,
+                        "pool": "default",
+                        "image": "/path/to/image.qcow2",
+                    },
+                    {"name": "data", "size": 16834},
+                ]
+                ifaces = [
+                    {"name": "eth0", "mac": "01:23:45:67:89:AB"},
+                    {"name": "eth1", "type": "network", "source": "admin"},
+                ]
+                graphics = {
+                    "type": "spice",
+                    "listen": {"type": "address", "address": "192.168.0.1"},
+                }
+                self.assertDictEqual(
+                    virt.defined(
+                        "myvm",
+                        cpu=2,
+                        mem=2048,
                         os_type="linux",
                         arch="i686",
                         vm_type="qemu",
@@ -361,7 +723,6 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     "myvm",
                     cpu=2,
                     mem=2048,
-                    boot_dev="cdrom hd",
                     os_type="linux",
                     arch="i686",
                     disk="prod",
@@ -470,13 +831,10 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                         "comment": "Domain myvm updated with live update(s) failures",
                     }
                 )
-                self.assertDictEqual(
-                    virt.defined("myvm", cpu=2, boot_dev="cdrom hd"), ret
-                )
+                self.assertDictEqual(virt.defined("myvm", cpu=2), ret)
                 update_mock.assert_called_with(
                     "myvm",
                     cpu=2,
-                    boot_dev="cdrom hd",
                     mem=None,
                     disk_profile=None,
                     disks=None,
@@ -600,7 +958,6 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     password=None,
                     boot=None,
                     test=True,
-                    boot_dev=None,
                 )
 
             # No changes case
@@ -635,7 +992,6 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     password=None,
                     boot=None,
                     test=True,
-                    boot_dev=None,
                 )
 
     def test_running(self):
@@ -685,12 +1041,67 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                         "comment": "Domain myvm defined and started",
                     }
                 )
+                self.assertDictEqual(
+                    virt.running("myvm", cpu=2, mem=2048, image="/path/to/img.qcow2"),
+                    ret,
+                )
+                init_mock.assert_called_with(
+                    "myvm",
+                    cpu=2,
+                    mem=2048,
+                    os_type=None,
+                    arch=None,
+                    boot=None,
+                    disk=None,
+                    disks=[{"name": "system", "image": "/path/to/img.qcow2"}],
+                    nic=None,
+                    interfaces=None,
+                    graphics=None,
+                    hypervisor=None,
+                    start=False,
+                    seed=True,
+                    install=True,
+                    pub_key=None,
+                    priv_key=None,
+                    connection=None,
+                    username=None,
+                    password=None,
+                )
+                start_mock.assert_called_with(
+                    "myvm", connection=None, username=None, password=None
+                )
+
+            # Test image parameter with disks with defined image
+            init_mock = MagicMock(return_value=True)
+            start_mock = MagicMock(return_value=0)
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.vm_state": MagicMock(return_value={"myvm": "stopped"}),
+                    "virt.init": init_mock,
+                    "virt.start": start_mock,
+                    "virt.list_domains": MagicMock(return_value=[]),
+                },
+            ):
+                ret.update(
+                    {
+                        "changes": {"myvm": {"definition": True, "started": True}},
+                        "comment": "Domain myvm defined and started",
+                    }
+                )
+                disks = [
+                    {
+                        "name": "system",
+                        "size": 8192,
+                        "overlay_image": True,
+                        "pool": "default",
+                        "image": "/path/to/image.qcow2",
+                    },
+                    {"name": "data", "size": 16834},
+                ]
                 self.assertDictEqual(
                     virt.running(
-                        "myvm",
-                        cpu=2,
-                        mem=2048,
-                        disks=[{"name": "system", "image": "/path/to/img.qcow2"}],
+                        "myvm", cpu=2, mem=2048, disks=disks, image="/path/to/img.qcow2"
                     ),
                     ret,
                 )
@@ -702,7 +1113,75 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     arch=None,
                     boot=None,
                     disk=None,
-                    disks=[{"name": "system", "image": "/path/to/img.qcow2"}],
+                    disks=disks,
+                    nic=None,
+                    interfaces=None,
+                    graphics=None,
+                    hypervisor=None,
+                    start=False,
+                    seed=True,
+                    install=True,
+                    pub_key=None,
+                    priv_key=None,
+                    connection=None,
+                    username=None,
+                    password=None,
+                )
+                start_mock.assert_called_with(
+                    "myvm", connection=None, username=None, password=None
+                )
+
+            # Test image parameter with disks without defined image
+            init_mock = MagicMock(return_value=True)
+            start_mock = MagicMock(return_value=0)
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.vm_state": MagicMock(return_value={"myvm": "stopped"}),
+                    "virt.init": init_mock,
+                    "virt.start": start_mock,
+                    "virt.list_domains": MagicMock(return_value=[]),
+                },
+            ):
+                ret.update(
+                    {
+                        "changes": {"myvm": {"definition": True, "started": True}},
+                        "comment": "Domain myvm defined and started",
+                    }
+                )
+                disks = [
+                    {
+                        "name": "system",
+                        "size": 8192,
+                        "overlay_image": True,
+                        "pool": "default",
+                    },
+                    {"name": "data", "size": 16834},
+                ]
+                self.assertDictEqual(
+                    virt.running(
+                        "myvm", cpu=2, mem=2048, disks=disks, image="/path/to/img.qcow2"
+                    ),
+                    ret,
+                )
+                init_mock.assert_called_with(
+                    "myvm",
+                    cpu=2,
+                    mem=2048,
+                    os_type=None,
+                    arch=None,
+                    boot=None,
+                    disk=None,
+                    disks=[
+                        {
+                            "name": "system",
+                            "size": 8192,
+                            "overlay_image": True,
+                            "pool": "default",
+                            "image": "/path/to/img.qcow2",
+                        },
+                        {"name": "data", "size": 16834},
+                    ],
                     nic=None,
                     interfaces=None,
                     graphics=None,
@@ -712,7 +1191,6 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     install=True,
                     pub_key=None,
                     priv_key=None,
-                    boot_dev=None,
                     connection=None,
                     username=None,
                     password=None,
@@ -774,7 +1252,6 @@ 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",
@@ -799,7 +1276,6 @@ 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",
@@ -944,7 +1420,6 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     password=None,
                     boot=None,
                     test=False,
-                    boot_dev=None,
                 )
 
             # Failed definition update case
@@ -1063,7 +1538,6 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     password=None,
                     boot=None,
                     test=True,
-                    boot_dev=None,
                 )
                 start_mock.assert_not_called()
 
@@ -1100,7 +1574,6 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     password=None,
                     boot=None,
                     test=True,
-                    boot_dev=None,
                 )
 
     def test_stopped(self):
@@ -1401,38 +1874,231 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     connection="myconnection",
                     username="user",
                     password="secret",
-                ),
-                ret,
-            )
-            reboot_mock.assert_called_with(
-                "myvm", connection="myconnection", username="user", password="secret"
-            )
+                ),
+                ret,
+            )
+            reboot_mock.assert_called_with(
+                "myvm", connection="myconnection", username="user", password="secret"
+            )
+
+        with patch.dict(
+            virt.__salt__,
+            {  # pylint: disable=no-member
+                "virt.list_domains": MagicMock(return_value=["myvm", "vm1"]),
+                "virt.reboot": MagicMock(
+                    side_effect=self.mock_libvirt.libvirtError("Some error")
+                ),
+            },
+        ):
+            ret.update(
+                {
+                    "changes": {"ignored": [{"domain": "myvm", "issue": "Some error"}]},
+                    "result": False,
+                    "comment": "No changes had happened",
+                }
+            )
+            self.assertDictEqual(virt.rebooted("myvm"), ret)
+
+        with patch.dict(
+            virt.__salt__, {"virt.list_domains": MagicMock(return_value=[])}
+        ):  # pylint: disable=no-member
+            ret.update(
+                {"changes": {}, "result": False, "comment": "No changes had happened"}
+            )
+            self.assertDictEqual(virt.rebooted("myvm"), ret)
+
+    def test_network_defined(self):
+        """
+        network_defined state test cases.
+        """
+        ret = {"name": "mynet", "changes": {}, "result": True, "comment": ""}
+        with patch.dict(virt.__opts__, {"test": False}):
+            define_mock = MagicMock(return_value=True)
+            # Non-existing network case
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.network_info": MagicMock(
+                        side_effect=[{}, {"mynet": {"active": False}}]
+                    ),
+                    "virt.network_define": define_mock,
+                },
+            ):
+                ret.update(
+                    {
+                        "changes": {"mynet": "Network defined"},
+                        "comment": "Network mynet defined",
+                    }
+                )
+                self.assertDictEqual(
+                    virt.network_defined(
+                        "mynet",
+                        "br2",
+                        "bridge",
+                        vport="openvswitch",
+                        tag=180,
+                        ipv4_config={
+                            "cidr": "192.168.2.0/24",
+                            "dhcp_ranges": [
+                                {"start": "192.168.2.10", "end": "192.168.2.25"},
+                                {"start": "192.168.2.110", "end": "192.168.2.125"},
+                            ],
+                        },
+                        ipv6_config={
+                            "cidr": "2001:db8:ca2:2::1/64",
+                            "dhcp_ranges": [
+                                {
+                                    "start": "2001:db8:ca2:1::10",
+                                    "end": "2001:db8:ca2::1f",
+                                },
+                            ],
+                        },
+                        autostart=False,
+                        connection="myconnection",
+                        username="user",
+                        password="secret",
+                    ),
+                    ret,
+                )
+                define_mock.assert_called_with(
+                    "mynet",
+                    "br2",
+                    "bridge",
+                    vport="openvswitch",
+                    tag=180,
+                    autostart=False,
+                    start=False,
+                    ipv4_config={
+                        "cidr": "192.168.2.0/24",
+                        "dhcp_ranges": [
+                            {"start": "192.168.2.10", "end": "192.168.2.25"},
+                            {"start": "192.168.2.110", "end": "192.168.2.125"},
+                        ],
+                    },
+                    ipv6_config={
+                        "cidr": "2001:db8:ca2:2::1/64",
+                        "dhcp_ranges": [
+                            {"start": "2001:db8:ca2:1::10", "end": "2001:db8:ca2::1f"},
+                        ],
+                    },
+                    connection="myconnection",
+                    username="user",
+                    password="secret",
+                )
 
-        with patch.dict(
-            virt.__salt__,
-            {  # pylint: disable=no-member
-                "virt.list_domains": MagicMock(return_value=["myvm", "vm1"]),
-                "virt.reboot": MagicMock(
-                    side_effect=self.mock_libvirt.libvirtError("Some error")
-                ),
-            },
-        ):
-            ret.update(
-                {
-                    "changes": {"ignored": [{"domain": "myvm", "issue": "Some error"}]},
-                    "result": False,
-                    "comment": "No changes had happened",
-                }
-            )
-            self.assertDictEqual(virt.rebooted("myvm"), ret)
+            # Case where there is nothing to be done
+            define_mock.reset_mock()
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.network_info": MagicMock(
+                        return_value={"mynet": {"active": True}}
+                    ),
+                    "virt.network_define": define_mock,
+                },
+            ):
+                ret.update({"changes": {}, "comment": "Network mynet exists"})
+                self.assertDictEqual(
+                    virt.network_defined("mynet", "br2", "bridge"), ret
+                )
 
-        with patch.dict(
-            virt.__salt__, {"virt.list_domains": MagicMock(return_value=[])}
-        ):  # pylint: disable=no-member
-            ret.update(
-                {"changes": {}, "result": False, "comment": "No changes had happened"}
-            )
-            self.assertDictEqual(virt.rebooted("myvm"), ret)
+            # Error case
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.network_info": MagicMock(return_value={}),
+                    "virt.network_define": MagicMock(
+                        side_effect=self.mock_libvirt.libvirtError("Some error")
+                    ),
+                },
+            ):
+                ret.update({"changes": {}, "comment": "Some error", "result": False})
+                self.assertDictEqual(
+                    virt.network_defined("mynet", "br2", "bridge"), ret
+                )
+
+        # Test cases with __opt__['test'] set to True
+        with patch.dict(virt.__opts__, {"test": True}):
+            ret.update({"result": None})
+
+            # Non-existing network case
+            define_mock.reset_mock()
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.network_info": MagicMock(return_value={}),
+                    "virt.network_define": define_mock,
+                },
+            ):
+                ret.update(
+                    {
+                        "changes": {"mynet": "Network defined"},
+                        "comment": "Network mynet defined",
+                    }
+                )
+                self.assertDictEqual(
+                    virt.network_defined(
+                        "mynet",
+                        "br2",
+                        "bridge",
+                        vport="openvswitch",
+                        tag=180,
+                        ipv4_config={
+                            "cidr": "192.168.2.0/24",
+                            "dhcp_ranges": [
+                                {"start": "192.168.2.10", "end": "192.168.2.25"},
+                                {"start": "192.168.2.110", "end": "192.168.2.125"},
+                            ],
+                        },
+                        ipv6_config={
+                            "cidr": "2001:db8:ca2:2::1/64",
+                            "dhcp_ranges": [
+                                {
+                                    "start": "2001:db8:ca2:1::10",
+                                    "end": "2001:db8:ca2::1f",
+                                },
+                            ],
+                        },
+                        autostart=False,
+                        connection="myconnection",
+                        username="user",
+                        password="secret",
+                    ),
+                    ret,
+                )
+                define_mock.assert_not_called()
+
+            # Case where there is nothing to be done
+            define_mock.reset_mock()
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.network_info": MagicMock(
+                        return_value={"mynet": {"active": True}}
+                    ),
+                    "virt.network_define": define_mock,
+                },
+            ):
+                ret.update(
+                    {"changes": {}, "comment": "Network mynet exists", "result": True}
+                )
+                self.assertDictEqual(
+                    virt.network_defined("mynet", "br2", "bridge"), ret
+                )
+
+            # Error case
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.network_info": MagicMock(
+                        side_effect=self.mock_libvirt.libvirtError("Some error")
+                    )
+                },
+            ):
+                ret.update({"changes": {}, "comment": "Some error", "result": False})
+                self.assertDictEqual(
+                    virt.network_defined("mynet", "br2", "bridge"), ret
+                )
 
     def test_network_defined(self):
         """
@@ -1988,72 +2654,6 @@ 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()
@@ -2103,9 +2703,6 @@ 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
@@ -2209,7 +2806,6 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     ),
                     ret,
                 )
-                mocks["build"].assert_not_called()
                 mocks["update"].assert_called_with(
                     "mypool",
                     ptype="logical",
@@ -2557,8 +3153,8 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
             ):
                 ret.update(
                     {
-                        "changes": {"mypool": "Pool updated, restarted"},
-                        "comment": "Pool mypool updated, restarted",
+                        "changes": {"mypool": "Pool updated, built, restarted"},
+                        "comment": "Pool mypool updated, built, restarted",
                         "result": True,
                     }
                 )
@@ -2584,7 +3180,9 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                 mocks["start"].assert_called_with(
                     "mypool", connection=None, username=None, password=None
                 )
-                mocks["build"].assert_not_called()
+                mocks["build"].assert_called_with(
+                    "mypool", connection=None, username=None, password=None
+                )
                 mocks["update"].assert_called_with(
                     "mypool",
                     ptype="logical",
@@ -2705,6 +3303,45 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                     ret,
                 )
 
+            # test case with test=True and pool to be defined
+            for mock in mocks:
+                mocks[mock].reset_mock()
+            with patch.dict(
+                virt.__salt__,
+                {  # pylint: disable=no-member
+                    "virt.pool_info": MagicMock(return_value={}),
+                },
+            ):
+                ret.update(
+                    {
+                        "changes": {
+                            "mypool": "Pool defined, marked for autostart, started"
+                        },
+                        "comment": "Pool mypool defined, marked for autostart, started",
+                        "result": None,
+                    }
+                )
+                self.assertDictEqual(
+                    virt.pool_running(
+                        "mypool",
+                        ptype="logical",
+                        target="/dev/base",
+                        permissions={
+                            "mode": "0770",
+                            "owner": 1000,
+                            "group": 100,
+                            "label": "seclabel",
+                        },
+                        source={"devices": [{"path": "/dev/sda"}]},
+                        transient=True,
+                        autostart=True,
+                        connection="myconnection",
+                        username="user",
+                        password="secret",
+                    ),
+                    ret,
+                )
+
     def test_pool_deleted(self):
         """
         Test the pool_deleted state
-- 
2.29.2