File virt-uefi-fix-backport-312.patch of Package salt.19538

Overview Repositories Revisions Requests Users Attributes Meta

File virt-uefi-fix-backport-312.patch of Package salt.19538

From 86e7e81f13134382d50e7a12f544d7b25cae4a4b Mon Sep 17 00:00:00 2001
From: Cedric Bosdonnat <cbosdonnat@suse.com>
Date: Mon, 8 Feb 2021 14:23:21 +0100
Subject: [PATCH] virt UEFI fix backport (#312)

* virt: serial and console fixes

While reworking the tests, I figured out the serial and console update
one wasn't quite right and it has shown errors in the code too.

* Convert the virt test_update() test to pytest and split it

Convert the huge test_update() function to pytest. While at it, also
split it into smaller more manageable test functions.

* virt: fix update of efi=True

Libvirt uses efi=True as a flag to look for the proper UEFI loader and
nvram template. This means that on a subsenquent state apply efi=True
will trigger a definition update even if not needed.

In case efi=True is provided, don't udpate if loader and nvram values
are already set.

* virt: remove useless try/except

* Reverse all asserts in code introduced by PR#59189

Asserts need to have the actual value on the left hand side and the
expected one on the right hand side. Reverting these in the newly
converted code.

In order to ease backporting to openSUSE package, this commit will remain
separate.
---
 changelog/59188.fixed                         |    1 +
 salt/modules/virt.py                          |   52 +-
 tests/pytests/unit/modules/virt/conftest.py   |   30 +-
 .../pytests/unit/modules/virt/test_domain.py  | 1010 ++++++++++++++-
 .../pytests/unit/modules/virt/test_helpers.py |    7 +
 tests/unit/modules/test_virt.py               | 1118 -----------------
 6 files changed, 1056 insertions(+), 1162 deletions(-)
 create mode 100644 changelog/59188.fixed

diff --git a/changelog/59188.fixed b/changelog/59188.fixed
new file mode 100644
index 0000000000..1382f9f07d
--- /dev/null
+++ b/changelog/59188.fixed
@@ -0,0 +1 @@
+virt.update doesn't update the definition if efi=True and a loader is already set
diff --git a/salt/modules/virt.py b/salt/modules/virt.py
index 9f61983e8d..7da35445a3 100644
--- a/salt/modules/virt.py
+++ b/salt/modules/virt.py
@@ -139,13 +139,13 @@ import salt.utils.json
 import salt.utils.path
 import salt.utils.stringutils
 import salt.utils.templates
+import salt.utils.virt
 import salt.utils.xmlutil as xmlutil
 import salt.utils.yaml
 from salt._compat import ElementTree, ipaddress, saxutils
 from salt.exceptions import CommandExecutionError, SaltInvocationError
 from salt.ext.six.moves import range  # pylint: disable=import-error,redefined-builtin
 from salt.ext.six.moves.urllib.parse import urlparse, urlunparse
-from salt.utils.virt import check_remote, download_remote
 
 try:
     import libvirt  # pylint: disable=import-error
@@ -1829,25 +1829,24 @@ def _handle_remote_boot_params(orig_boot):
         {"kernel", "initrd", "cmdline", "loader", "nvram"},
     ]
 
-    try:
-        if keys in cases:
-            for key in keys:
-                if key == "efi" and type(orig_boot.get(key)) == bool:
-                    new_boot[key] = orig_boot.get(key)
-                elif orig_boot.get(key) is not None and check_remote(
-                    orig_boot.get(key)
-                ):
-                    if saltinst_dir is None:
-                        os.makedirs(CACHE_DIR)
-                        saltinst_dir = CACHE_DIR
-                    new_boot[key] = download_remote(orig_boot.get(key), saltinst_dir)
-            return new_boot
-        else:
-            raise SaltInvocationError(
-                "Invalid boot parameters,It has to follow this combination: [(kernel, initrd) or/and cmdline] or/and [(loader, nvram) or efi]"
-            )
-    except Exception as err:  # pylint: disable=broad-except
-        raise err
+    if keys in cases:
+        for key in keys:
+            if key == "efi" and type(orig_boot.get(key)) == bool:
+                new_boot[key] = orig_boot.get(key)
+            elif orig_boot.get(key) is not None and salt.utils.virt.check_remote(
+                orig_boot.get(key)
+            ):
+                if saltinst_dir is None:
+                    os.makedirs(CACHE_DIR)
+                    saltinst_dir = CACHE_DIR
+                new_boot[key] = salt.utils.virt.download_remote(
+                    orig_boot.get(key), saltinst_dir
+                )
+        return new_boot
+    else:
+        raise SaltInvocationError(
+            "Invalid boot parameters,It has to follow this combination: [(kernel, initrd) or/and cmdline] or/and [(loader, nvram) or efi]"
+        )
 
 
 def _handle_efi_param(boot, desc):
@@ -1870,10 +1869,11 @@ def _handle_efi_param(boot, desc):
     elif type(efi_value) == bool and os_attrib == {}:
         if efi_value is True and parent_tag.find("loader") is None:
             parent_tag.set("firmware", "efi")
+            return True
         if efi_value is False and parent_tag.find("loader") is not None:
             parent_tag.remove(parent_tag.find("loader"))
             parent_tag.remove(parent_tag.find("nvram"))
-        return True
+            return True
     elif type(efi_value) != bool:
         raise SaltInvocationError("Invalid efi value")
     return False
@@ -3150,7 +3150,7 @@ def _serial_or_concole_equal(old, new):
     return _filter_serial_or_concole(old) == _filter_serial_or_concole(new)
 
 
-def _diff_serial_list(old, new):
+def _diff_serial_lists(old, new):
     """
     Compare serial definitions to extract the changes
 
@@ -3160,7 +3160,7 @@ def _diff_serial_list(old, new):
     return _diff_lists(old, new, _serial_or_concole_equal)
 
 
-def _diff_console_list(old, new):
+def _diff_console_lists(old, new):
     """
     Compare console definitions to extract the changes
 
@@ -3656,7 +3656,7 @@ def update(
             boot,
             boot_dev,
             numatune,
-            serial=serials,
+            serials=serials,
             consoles=consoles,
             stop_on_reboot=stop_on_reboot,
             host_devices=host_devices,
@@ -4050,8 +4050,8 @@ def update(
         "disk": _skip_update(["disks", "disk_profile"]),
         "interface": _skip_update(["interfaces", "nic_profile"]),
         "graphics": _skip_update(["graphics"]),
-        "serial": _skip_update(["serial"]),
-        "console": _skip_update(["console"]),
+        "serial": _skip_update(["serials"]),
+        "console": _skip_update(["consoles"]),
         "hostdev": _skip_update(["host_devices"]),
     }
     changes = _compute_device_changes(desc, new_desc, to_skip)
diff --git a/tests/pytests/unit/modules/virt/conftest.py b/tests/pytests/unit/modules/virt/conftest.py
index 3bacd734a7..d306997deb 100644
--- a/tests/pytests/unit/modules/virt/conftest.py
+++ b/tests/pytests/unit/modules/virt/conftest.py
@@ -65,10 +65,24 @@ def loader_modules_config():
 
 @pytest.fixture
 def make_mock_vm():
-    def _make_mock_vm(xml_def, running=False, inactive_def=None):
+    def _make_mock_vm(xml_def=None, running=False, inactive_def=None):
         mocked_conn = virt.libvirt.openAuth.return_value
 
-        doc = ET.fromstring(xml_def)
+        desc = xml_def
+        if not desc:
+            desc = """
+                <domain type='kvm' id='7'>
+                  <name>my_vm</name>
+                  <memory unit='KiB'>1048576</memory>
+                  <currentMemory unit='KiB'>1048576</currentMemory>
+                  <vcpu placement='auto'>1</vcpu>
+                  <on_reboot>restart</on_reboot>
+                  <os>
+                    <type arch='x86_64' machine='pc-i440fx-2.6'>hvm</type>
+                  </os>
+                </domain>
+            """
+        doc = ET.fromstring(desc)
         name = doc.find("name").text
         os_type = "hvm"
         os_type_node = doc.find("os/type")
@@ -84,9 +98,9 @@ def make_mock_vm():
         domain_mock = mocked_conn.lookupByName(name)
 
         domain_mock.XMLDesc = MappedResultMock()
-        domain_mock.XMLDesc.add(0, xml_def)
+        domain_mock.XMLDesc.add(0, desc)
         domain_mock.XMLDesc.add(
-            virt.libvirt.VIR_DOMAIN_XML_INACTIVE, inactive_def or xml_def
+            virt.libvirt.VIR_DOMAIN_XML_INACTIVE, inactive_def or desc
         )
         domain_mock.OSType.return_value = os_type
 
@@ -103,6 +117,8 @@ def make_mock_vm():
 
         domain_mock.attachDevice.return_value = 0
         domain_mock.detachDevice.return_value = 0
+        domain_mock.setMemoryFlags.return_value = 0
+        domain_mock.setVcpusFlags.return_value = 0
 
         domain_mock.connect.return_value = mocked_conn
 
@@ -113,7 +129,7 @@ def make_mock_vm():
 
 @pytest.fixture
 def make_mock_storage_pool():
-    def _make_mock_storage_pool(name, type, volumes):
+    def _make_mock_storage_pool(name, type, volumes, source=None):
         mocked_conn = virt.libvirt.openAuth.return_value
 
         # Append the pool name to the list of known mocked pools
@@ -130,8 +146,8 @@ def make_mock_storage_pool():
         # Configure the pool
         mocked_conn.storagePoolLookupByName.add(name)
         mocked_pool = mocked_conn.storagePoolLookupByName(name)
-        source = ""
-        if type == "disk":
+        source_def = source
+        if not source and type == "disk":
             source = "<device path='/dev/{}'/>".format(name)
         pool_path = "/path/to/{}".format(name)
         mocked_pool.XMLDesc.return_value = """
diff --git a/tests/pytests/unit/modules/virt/test_domain.py b/tests/pytests/unit/modules/virt/test_domain.py
index 0bde881403..33357c60ea 100644
--- a/tests/pytests/unit/modules/virt/test_domain.py
+++ b/tests/pytests/unit/modules/virt/test_domain.py
@@ -1,11 +1,15 @@
+import os.path
+
 import pytest
 import salt.modules.virt as virt
 import salt.utils.xmlutil as xmlutil
+import salt.syspaths
 from salt._compat import ElementTree as ET
+from salt.exceptions import SaltInvocationError
 from tests.support.mock import MagicMock, patch
 
 from .conftest import loader_modules_config
-from .test_helpers import append_to_XMLDesc, assert_called, strip_xml
+from .test_helpers import append_to_XMLDesc, assert_called, strip_xml, assertEqualUnit
 
 
 @pytest.fixture
@@ -54,8 +58,8 @@ def test_update_xen_disk_volumes(make_mock_vm, make_mock_storage_pool):
     )
 
     assert ret["definition"]
-    define_mock = virt.libvirt.openAuth().defineXML
-    setxml = ET.fromstring(define_mock.call_args[0][0])
+    virt.libvirt.openAuth().defineXML = virt.libvirt.openAuth().defineXML
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
     assert "block" == setxml.find(".//disk[3]").get("type")
     assert "/path/to/vdb/vdb1" == setxml.find(".//disk[3]/source").get("dev")
 
@@ -544,8 +548,8 @@ def test_update_stop_on_reboot_reset(make_mock_vm):
     ret = virt.update("my_vm")
 
     assert ret["definition"]
-    define_mock = virt.libvirt.openAuth().defineXML
-    setxml = ET.fromstring(define_mock.call_args[0][0])
+    virt.libvirt.openAuth().defineXML = virt.libvirt.openAuth().defineXML
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
     assert "restart" == setxml.find("./on_reboot").text
 
 
@@ -568,8 +572,8 @@ def test_update_stop_on_reboot(make_mock_vm):
     ret = virt.update("my_vm", stop_on_reboot=True)
 
     assert ret["definition"]
-    define_mock = virt.libvirt.openAuth().defineXML
-    setxml = ET.fromstring(define_mock.call_args[0][0])
+    virt.libvirt.openAuth().defineXML = virt.libvirt.openAuth().defineXML
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
     assert "destroy" == setxml.find("./on_reboot").text
 
 
@@ -581,8 +585,8 @@ def test_init_no_stop_on_reboot(make_capabilities):
     with patch.dict(virt.os.__dict__, {"chmod": MagicMock(), "makedirs": MagicMock()}):
         with patch.dict(virt.__salt__, {"cmd.run": MagicMock()}):
             virt.init("test_vm", 2, 2048, start=False)
-            define_mock = virt.libvirt.openAuth().defineXML
-            setxml = ET.fromstring(define_mock.call_args[0][0])
+            virt.libvirt.openAuth().defineXML = virt.libvirt.openAuth().defineXML
+            setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
             assert "restart" == setxml.find("./on_reboot").text
 
 
@@ -594,8 +598,8 @@ def test_init_stop_on_reboot(make_capabilities):
     with patch.dict(virt.os.__dict__, {"chmod": MagicMock(), "makedirs": MagicMock()}):
         with patch.dict(virt.__salt__, {"cmd.run": MagicMock()}):
             virt.init("test_vm", 2, 2048, stop_on_reboot=True, start=False)
-            define_mock = virt.libvirt.openAuth().defineXML
-            setxml = ET.fromstring(define_mock.call_args[0][0])
+            virt.libvirt.openAuth().defineXML = virt.libvirt.openAuth().defineXML
+            setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
             assert "destroy" == setxml.find("./on_reboot").text
 
 
@@ -1060,3 +1064,987 @@ def test_update_nic_hostdev_nochange(make_mock_network, make_mock_vm, test):
     define_mock.assert_not_called()
     domain_mock.attachDevice.assert_not_called()
     domain_mock.detachDevice.assert_not_called()
+
+
+def test_update_no_param(make_mock_vm):
+    """
+    Test virt.update(), no parameter passed
+    """
+    domain_mock = make_mock_vm()
+    ret = virt.update("my_vm")
+    assert not ret["definition"]
+    assert not ret.get("mem")
+    assert not ret.get("cpu")
+
+
+def test_update_cpu_and_mem(make_mock_vm):
+    """
+    Test virt.update(), update both cpu and memory
+    """
+    domain_mock = make_mock_vm()
+    ret = virt.update("my_vm", mem=2048, cpu=2)
+    assert ret["definition"]
+    assert ret["mem"]
+    assert ret["cpu"]
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+    assert setxml.find("vcpu").text == "2"
+    assert setxml.find("memory").text == "2147483648"
+    assert domain_mock.setMemoryFlags.call_args[0][0] == 2048 * 1024
+    assert domain_mock.setVcpusFlags.call_args[0][0] == 2
+
+
+def test_update_cpu_simple(make_mock_vm):
+    """
+    Test virt.update(), simple cpu count update
+    """
+    domain_mock = make_mock_vm()
+    ret = virt.update("my_vm", cpu=2)
+    assert ret["definition"]
+    assert ret["cpu"]
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+    assert setxml.find("vcpu").text == "2"
+    assert domain_mock.setVcpusFlags.call_args[0][0] == 2
+
+
+def test_update_add_cpu_topology(make_mock_vm):
+    """
+    Test virt.update(), add cpu topology settings
+    """
+    domain_mock = make_mock_vm()
+    ret = virt.update(
+        "my_vm",
+        cpu={
+            "placement": "static",
+            "cpuset": "0-11",
+            "current": 5,
+            "maximum": 12,
+            "vcpus": {
+                "0": {"enabled": True, "hotpluggable": False, "order": 1},
+                "1": {"enabled": False, "hotpluggable": True},
+            },
+            "mode": "custom",
+            "match": "exact",
+            "check": "full",
+            "model": {
+                "name": "coreduo",
+                "fallback": "allow",
+                "vendor_id": "Genuine20201",
+            },
+            "vendor": "Intel",
+            "topology": {"sockets": 1, "cores": 12, "threads": 1},
+            "cache": {"mode": "emulate", "level": 3},
+            "features": {"lahf": "optional", "pcid": "disable"},
+            "numa": {
+                "0": {
+                    "cpus": "0-3",
+                    "memory": "1g",
+                    "discard": True,
+                    "distances": {0: 10, 1: 21, 2: 31, 3: 41},
+                },
+                "1": {
+                    "cpus": "4-6",
+                    "memory": "0.5g",
+                    "discard": False,
+                    "memAccess": "shared",
+                    "distances": {0: 21, 1: 10, 2: 15, 3: 30},
+                },
+            },
+        },
+    )
+    assert ret["definition"]
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+
+    assert setxml.find("vcpu").text == "12"
+    assert setxml.find("vcpu").get("placement") == "static"
+    assert setxml.find("vcpu").get("cpuset") == "0,1,2,3,4,5,6,7,8,9,10,11"
+    assert setxml.find("vcpu").get("current") == "5"
+
+    assert setxml.find("./vcpus/vcpu/[@id='0']").get("id") == "0"
+    assert setxml.find("./vcpus/vcpu/[@id='0']").get("enabled") == "yes"
+    assert setxml.find("./vcpus/vcpu/[@id='0']").get("hotpluggable") == "no"
+    assert setxml.find("./vcpus/vcpu/[@id='0']").get("order") == "1"
+    assert setxml.find("./vcpus/vcpu/[@id='1']").get("id") == "1"
+    assert setxml.find("./vcpus/vcpu/[@id='1']").get("enabled") == "no"
+    assert setxml.find("./vcpus/vcpu/[@id='1']").get("hotpluggable") == "yes"
+    assert setxml.find("./vcpus/vcpu/[@id='1']").get("order") is None
+
+    assert setxml.find("cpu").get("mode") == "custom"
+    assert setxml.find("cpu").get("match") == "exact"
+    assert setxml.find("cpu").get("check") == "full"
+
+    assert setxml.find("cpu/model").get("vendor_id") == "Genuine20201"
+    assert setxml.find("cpu/model").get("fallback") == "allow"
+    assert setxml.find("cpu/model").text == "coreduo"
+
+    assert setxml.find("cpu/vendor").text == "Intel"
+
+    assert setxml.find("cpu/topology").get("sockets") == "1"
+    assert setxml.find("cpu/topology").get("cores") == "12"
+    assert setxml.find("cpu/topology").get("threads") == "1"
+
+    assert setxml.find("cpu/cache").get("level") == "3"
+    assert setxml.find("cpu/cache").get("mode") == "emulate"
+
+    assert setxml.find("./cpu/feature[@name='pcid']").get("policy") == "disable"
+    assert setxml.find("./cpu/feature[@name='lahf']").get("policy") == "optional"
+
+    assert setxml.find("./cpu/numa/cell/[@id='0']").get("cpus") == "0,1,2,3"
+    assert setxml.find("./cpu/numa/cell/[@id='0']").get("memory") == str(1024 ** 3)
+    assert setxml.find("./cpu/numa/cell/[@id='0']").get("unit") == "bytes"
+    assert setxml.find("./cpu/numa/cell/[@id='0']").get("discard") == "yes"
+    assert (
+        setxml.find("./cpu/numa/cell/[@id='0']/distances/sibling/[@id='0']").get(
+            "value"
+        )
+        == "10"
+    )
+    assert (
+        setxml.find("./cpu/numa/cell/[@id='0']/distances/sibling/[@id='1']").get(
+            "value"
+        )
+        == "21"
+    )
+    assert (
+        setxml.find("./cpu/numa/cell/[@id='0']/distances/sibling/[@id='2']").get(
+            "value"
+        )
+        == "31"
+    )
+    assert (
+        setxml.find("./cpu/numa/cell/[@id='0']/distances/sibling/[@id='3']").get(
+            "value"
+        )
+        == "41"
+    )
+    assert setxml.find("./cpu/numa/cell/[@id='1']").get("cpus") == "4,5,6"
+    assert setxml.find("./cpu/numa/cell/[@id='1']").get("memory") == str(
+        int(1024 ** 3 / 2)
+    )
+    assert setxml.find("./cpu/numa/cell/[@id='1']").get("unit") == "bytes"
+    assert setxml.find("./cpu/numa/cell/[@id='1']").get("discard") == "no"
+    assert setxml.find("./cpu/numa/cell/[@id='1']").get("memAccess") == "shared"
+    assert (
+        setxml.find("./cpu/numa/cell/[@id='1']/distances/sibling/[@id='0']").get(
+            "value"
+        )
+        == "21"
+    )
+    assert (
+        setxml.find("./cpu/numa/cell/[@id='1']/distances/sibling/[@id='1']").get(
+            "value"
+        )
+        == "10"
+    )
+    assert (
+        setxml.find("./cpu/numa/cell/[@id='1']/distances/sibling/[@id='2']").get(
+            "value"
+        )
+        == "15"
+    )
+    assert (
+        setxml.find("./cpu/numa/cell/[@id='1']/distances/sibling/[@id='3']").get(
+            "value"
+        )
+        == "30"
+    )
+
+
+@pytest.mark.parametrize("boot_dev", ["hd", "cdrom network hd"])
+def test_update_bootdev_unchanged(make_mock_vm, boot_dev):
+    """
+    Test virt.update(), unchanged boot devices case
+    """
+    domain_mock = make_mock_vm(
+        """
+            <domain type='kvm' id='7'>
+              <name>my_vm</name>
+              <memory unit='KiB'>1048576</memory>
+              <currentMemory unit='KiB'>1048576</currentMemory>
+              <vcpu placement='auto'>1</vcpu>
+              <on_reboot>restart</on_reboot>
+              <os>
+                <type arch='x86_64' machine='pc-i440fx-2.6'>hvm</type>
+                <boot dev="hd"/>
+              </os>
+            </domain>
+        """
+    )
+    ret = virt.update("my_vm", boot_dev=boot_dev)
+    assert (boot_dev != "hd") == ret["definition"]
+    if boot_dev == "hd":
+        virt.libvirt.openAuth().defineXML.assert_not_called()
+    else:
+        setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+        assert [node.get("dev") for node in setxml.findall("os/boot")] == [
+            "cdrom",
+            "network",
+            "hd",
+        ]
+
+
+def test_update_boot_kernel_paths(make_mock_vm):
+    """
+    Test virt.update(), change boot with kernel/initrd path and kernel params
+    """
+    domain_mock = make_mock_vm()
+    ret = virt.update(
+        "my_vm",
+        boot={
+            "kernel": "/root/f8-i386-vmlinuz",
+            "initrd": "/root/f8-i386-initrd",
+            "cmdline": "console=ttyS0 ks=http://example.com/f8-i386/os/",
+        },
+    )
+    assert ret["definition"]
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+    assert setxml.find("os/kernel").text == "/root/f8-i386-vmlinuz"
+    assert setxml.find("os/initrd").text == "/root/f8-i386-initrd"
+    assert (
+        setxml.find("os/cmdline").text
+        == "console=ttyS0 ks=http://example.com/f8-i386/os/"
+    )
+
+
+def test_update_boot_uefi_paths(make_mock_vm):
+    """
+    Test virt.update(), add boot with uefi loader and nvram paths
+    """
+    domain_mock = make_mock_vm()
+
+    ret = virt.update(
+        "my_vm",
+        boot={
+            "loader": "/usr/share/OVMF/OVMF_CODE.fd",
+            "nvram": "/usr/share/OVMF/OVMF_VARS.ms.fd",
+        },
+    )
+
+    assert ret["definition"]
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+    assert setxml.find("os/loader").text == "/usr/share/OVMF/OVMF_CODE.fd"
+    assert setxml.find("os/loader").get("readonly") == "yes"
+    assert setxml.find("os/loader").get("type") == "pflash"
+    assert setxml.find("os/nvram").get("template") == "/usr/share/OVMF/OVMF_VARS.ms.fd"
+
+
+def test_update_boot_uefi_auto(make_mock_vm):
+    """
+    Test virt.update(), change boot with efi value (automatic discovery of loader)
+    """
+    domain_mock = make_mock_vm()
+
+    ret = virt.update("my_vm", boot={"efi": True})
+    assert ret["definition"]
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+    assert setxml.find("os").get("firmware") == "efi"
+
+
+def test_update_boot_uefi_auto_nochange(make_mock_vm):
+    """
+    Test virt.update(), change boot with efi value and no change.
+    libvirt converts the efi=True value into a loader and nvram config with path.
+    """
+    domain_mock = make_mock_vm(
+        """
+        <domain type='kvm' id='1'>
+          <name>my_vm</name>
+          <uuid>27434df0-706d-4603-8ad7-5a88d19a3417</uuid>
+          <memory unit='KiB'>524288</memory>
+          <currentMemory unit='KiB'>524288</currentMemory>
+          <vcpu placement='static'>1</vcpu>
+          <resource>
+            <partition>/machine</partition>
+          </resource>
+          <os>
+            <type arch='x86_64' machine='pc-i440fx-4.2'>hvm</type>
+            <loader readonly='yes' type='pflash'>/usr/share/qemu/edk2-x86_64-code.fd</loader>
+            <nvram template='/usr/share/qemu/edk2-i386-vars.fd'>/var/lib/libvirt/qemu/nvram/vm01_VARS.fd</nvram>
+          </os>
+          <on_reboot>restart</on_reboot>
+        </domain>
+        """
+    )
+
+    ret = virt.update("my_vm", boot={"efi": True})
+    assert not ret["definition"]
+    virt.libvirt.openAuth().defineXML.assert_not_called()
+
+
+def test_update_boot_invalid(make_mock_vm):
+    """
+    Test virt.update(), change boot, invalid values
+    """
+    domain_mock = make_mock_vm()
+
+    with pytest.raises(SaltInvocationError):
+        virt.update(
+            "my_vm",
+            boot={
+                "loader": "/usr/share/OVMF/OVMF_CODE.fd",
+                "initrd": "/root/f8-i386-initrd",
+            },
+        )
+
+    with pytest.raises(SaltInvocationError):
+        virt.update("my_vm", boot={"efi": "Not a boolean value"})
+
+
+def test_update_add_memtune(make_mock_vm):
+    """
+    Test virt.update(), add memory tune config case
+    """
+    domain_mock = make_mock_vm()
+    ret = virt.update(
+        "my_vm",
+        mem={
+            "soft_limit": "0.5g",
+            "hard_limit": "1024",
+            "swap_hard_limit": "2048m",
+            "min_guarantee": "1 g",
+        },
+    )
+
+    assert ret["definition"]
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+    assertEqualUnit(setxml.find("memtune/soft_limit"), int(0.5 * 1024 ** 3), "bytes")
+    assertEqualUnit(setxml.find("memtune/hard_limit"), 1024 * 1024 ** 2, "bytes")
+    assertEqualUnit(setxml.find("memtune/swap_hard_limit"), 2048 * 1024 ** 2, "bytes")
+    assertEqualUnit(setxml.find("memtune/min_guarantee"), 1 * 1024 ** 3, "bytes")
+
+
+def test_update_add_memtune_invalid_unit(make_mock_vm):
+    """
+    Test virt.update(), add invalid unit to memory tuning config
+    """
+    domain_mock = make_mock_vm()
+
+    with pytest.raises(SaltInvocationError):
+        virt.update("my_vm", mem={"soft_limit": "2HB"})
+
+    with pytest.raises(SaltInvocationError):
+        virt.update("my_vm", mem={"soft_limit": "3.4.MB"})
+
+
+def test_update_add_numatune(make_mock_vm):
+    """
+    Test virt.update(), add numatune config case
+    """
+    domain_mock = make_mock_vm()
+    ret = virt.update(
+        "my_vm",
+        numatune={
+            "memory": {"mode": "strict", "nodeset": 1},
+            "memnodes": {
+                0: {"mode": "strict", "nodeset": 1},
+                1: {"mode": "preferred", "nodeset": 2},
+            },
+        },
+    )
+
+    assert ret["definition"]
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+    assert setxml.find("numatune/memory").get("mode") == "strict"
+    assert setxml.find("numatune/memory").get("nodeset") == "1"
+    assert setxml.find("./numatune/memnode/[@cellid='0']").get("mode") == "strict"
+    assert setxml.find("./numatune/memnode/[@cellid='0']").get("nodeset") == "1"
+    assert setxml.find("./numatune/memnode/[@cellid='1']").get("mode") == "preferred"
+    assert setxml.find("./numatune/memnode/[@cellid='1']").get("nodeset") == "2"
+
+
+def test_update_mem_simple(make_mock_vm):
+    """
+    Test virt.update(), simple memory amount change
+    """
+    domain_mock = make_mock_vm()
+    ret = virt.update("my_vm", mem=2048)
+    assert ret["definition"]
+    assert ret["mem"]
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+    assert setxml.find("memory").text == str(2048 * 1024 ** 2)
+    assert setxml.find("memory").get("unit") == "bytes"
+    assert domain_mock.setMemoryFlags.call_args[0][0] == 2048 * 1024
+
+
+def test_update_mem(make_mock_vm):
+    """
+    Test virt.update(), advanced memory amounts changes
+    """
+    domain_mock = make_mock_vm()
+
+    ret = virt.update(
+        "my_vm", mem={"boot": "0.5g", "current": "2g", "max": "1g", "slots": 12},
+    )
+    assert ret["definition"]
+    assert ret["mem"]
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+    assert setxml.find("memory").get("unit") == "bytes"
+    assert setxml.find("memory").text == str(int(0.5 * 1024 ** 3))
+    assert setxml.find("maxMemory").text == str(1 * 1024 ** 3)
+    assert setxml.find("currentMemory").text == str(2 * 1024 ** 3)
+
+
+def test_update_add_mem_backing(make_mock_vm):
+    """
+    Test virt.update(), add memory backing case
+    """
+    domain_mock = make_mock_vm()
+    ret = virt.update(
+        "my_vm",
+        mem={
+            "hugepages": [
+                {"nodeset": "1-5,^4", "size": "1g"},
+                {"nodeset": "4", "size": "2g"},
+            ],
+            "nosharepages": True,
+            "locked": True,
+            "source": "file",
+            "access": "shared",
+            "allocation": "immediate",
+            "discard": True,
+        },
+    )
+
+    assert ret["definition"]
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+    assert {
+        p.get("nodeset"): {"size": p.get("size"), "unit": p.get("unit")}
+        for p in setxml.findall("memoryBacking/hugepages/page")
+    } == {
+        "1,2,3,5": {"size": str(1024 ** 3), "unit": "bytes"},
+        "4": {"size": str(2 * 1024 ** 3), "unit": "bytes"},
+    }
+    assert setxml.find("./memoryBacking/nosharepages") is not None
+    assert setxml.find("./memoryBacking/nosharepages").text is None
+    assert setxml.find("./memoryBacking/nosharepages").keys() == []
+    assert setxml.find("./memoryBacking/locked") is not None
+    assert setxml.find("./memoryBacking/locked").text is None
+    assert setxml.find("./memoryBacking/locked").keys() == []
+    assert setxml.find("./memoryBacking/source").attrib["type"] == "file"
+    assert setxml.find("./memoryBacking/access").attrib["mode"] == "shared"
+    assert setxml.find("./memoryBacking/discard") is not None
+
+
+def test_update_add_iothreads(make_mock_vm):
+    """
+    Test virt.update(), add iothreads
+    """
+    domain_mock = make_mock_vm()
+    ret = virt.update("my_vm", cpu={"iothreads": 5})
+    assert ret["definition"]
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+    assert setxml.find("iothreads").text == "5"
+
+
+def test_update_add_cputune(make_mock_vm):
+    """
+    Test virt.update(), adding CPU tuning parameters
+    """
+    domain_mock = make_mock_vm()
+    cputune = {
+        "shares": 2048,
+        "period": 122000,
+        "quota": -1,
+        "global_period": 1000000,
+        "global_quota": -3,
+        "emulator_period": 1200000,
+        "emulator_quota": -10,
+        "iothread_period": 133000,
+        "iothread_quota": -1,
+        "vcpupin": {0: "1-4,^2", 1: "0,1", 2: "2,3", 3: "0,4"},
+        "emulatorpin": "1-3",
+        "iothreadpin": {1: "5-6", 2: "7-8"},
+        "vcpusched": [
+            {"scheduler": "fifo", "priority": 1, "vcpus": "0"},
+            {"scheduler": "fifo", "priotity": 2, "vcpus": "1"},
+            {"scheduler": "idle", "priotity": 3, "vcpus": "2"},
+        ],
+        "iothreadsched": [{"scheduler": "batch", "iothreads": "7"}],
+        "cachetune": {
+            "0-3": {
+                0: {"level": 3, "type": "both", "size": 3},
+                1: {"level": 3, "type": "both", "size": 3},
+                "monitor": {1: 3, "0-3": 3},
+            },
+            "4-5": {"monitor": {4: 3, 5: 2}},
+        },
+        "memorytune": {"0-2": {0: 60}, "3-4": {0: 50, 1: 70}},
+    }
+    assert virt.update("my_vm", cpu={"tuning": cputune}) == {
+        "definition": True,
+        "disk": {"attached": [], "detached": [], "updated": []},
+        "interface": {"attached": [], "detached": []},
+    }
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+    assert setxml.find("cputune/shares").text == "2048"
+    assert setxml.find("cputune/period").text == "122000"
+    assert setxml.find("cputune/quota").text == "-1"
+    assert setxml.find("cputune/global_period").text == "1000000"
+    assert setxml.find("cputune/global_quota").text == "-3"
+    assert setxml.find("cputune/emulator_period").text == "1200000"
+    assert setxml.find("cputune/emulator_quota").text == "-10"
+    assert setxml.find("cputune/iothread_period").text == "133000"
+    assert setxml.find("cputune/iothread_quota").text == "-1"
+    assert setxml.find("cputune/vcpupin[@vcpu='0']").get("cpuset") == "1,3,4"
+    assert setxml.find("cputune/vcpupin[@vcpu='1']").get("cpuset") == "0,1"
+    assert setxml.find("cputune/vcpupin[@vcpu='2']").get("cpuset") == "2,3"
+    assert setxml.find("cputune/vcpupin[@vcpu='3']").get("cpuset") == "0,4"
+    assert setxml.find("cputune/emulatorpin").get("cpuset") == "1,2,3"
+    assert setxml.find("cputune/iothreadpin[@iothread='1']").get("cpuset") == "5,6"
+    assert setxml.find("cputune/iothreadpin[@iothread='2']").get("cpuset") == "7,8"
+    assert setxml.find("cputune/vcpusched[@vcpus='0']").get("priority") == "1"
+    assert setxml.find("cputune/vcpusched[@vcpus='0']").get("scheduler") == "fifo"
+    assert setxml.find("cputune/iothreadsched").get("iothreads") == "7"
+    assert setxml.find("cputune/iothreadsched").get("scheduler") == "batch"
+    assert (
+        setxml.find("./cputune/cachetune[@vcpus='0,1,2,3']/cache[@id='0']").get("level")
+        == "3"
+    )
+    assert (
+        setxml.find("./cputune/cachetune[@vcpus='0,1,2,3']/cache[@id='0']").get("type")
+        == "both"
+    )
+    assert (
+        setxml.find("./cputune/cachetune[@vcpus='0,1,2,3']/monitor[@vcpus='1']").get(
+            "level"
+        )
+        == "3"
+    )
+    assert (
+        setxml.find("./cputune/cachetune[@vcpus='4,5']/monitor[@vcpus='4']").get(
+            "level"
+        )
+        == "3"
+    )
+    assert (
+        setxml.find("./cputune/cachetune[@vcpus='4,5']/monitor[@vcpus='5']").get(
+            "level"
+        )
+        == "2"
+    )
+    assert (
+        setxml.find("./cputune/memorytune[@vcpus='0,1,2']/node[@id='0']").get(
+            "bandwidth"
+        )
+        == "60"
+    )
+    assert (
+        setxml.find("./cputune/memorytune[@vcpus='3,4']/node[@id='0']").get("bandwidth")
+        == "50"
+    )
+    assert (
+        setxml.find("./cputune/memorytune[@vcpus='3,4']/node[@id='1']").get("bandwidth")
+        == "70"
+    )
+
+
+def test_update_graphics(make_mock_vm):
+    """
+    Test virt.update(), graphics update case
+    """
+    domain_mock = make_mock_vm(
+        """
+        <domain type='kvm' id='7'>
+          <name>my_vm</name>
+          <memory unit='KiB'>1048576</memory>
+          <currentMemory unit='KiB'>1048576</currentMemory>
+          <vcpu placement='auto'>1</vcpu>
+          <on_reboot>restart</on_reboot>
+          <os>
+            <type arch='x86_64' machine='pc-i440fx-2.6'>hvm</type>
+          </os>
+          <devices>
+            <graphics type='spice' listen='127.0.0.1' autoport='yes'>
+              <listen type='address' address='127.0.0.1'/>
+            </graphics>
+          </devices>
+        </domain>
+        """
+    )
+    assert virt.update("my_vm", graphics={"type": "vnc"}) == {
+        "definition": True,
+        "disk": {"attached": [], "detached": [], "updated": []},
+        "interface": {"attached": [], "detached": []},
+    }
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+    assert setxml.find("devices/graphics").get("type") == "vnc"
+
+
+def test_update_console(make_mock_vm):
+    """
+    Test virt.update(), console and serial devices update case
+    """
+    domain_mock = make_mock_vm(
+        """
+        <domain type='kvm' id='7'>
+          <name>my_vm</name>
+          <memory unit='KiB'>1048576</memory>
+          <currentMemory unit='KiB'>1048576</currentMemory>
+          <vcpu placement='auto'>1</vcpu>
+          <on_reboot>restart</on_reboot>
+          <os>
+            <type arch='x86_64' machine='pc-i440fx-2.6'>hvm</type>
+          </os>
+          <devices>
+            <serial type='pty'/>
+            <console type='pty'/>
+          </devices>
+        </domain>
+        """
+    )
+
+    assert virt.update(
+        "my_vm", serials=[{"type": "tcp"}], consoles=[{"type": "tcp"}]
+    ) == {
+        "definition": True,
+        "disk": {"attached": [], "detached": [], "updated": []},
+        "interface": {"attached": [], "detached": []},
+    }
+    setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+    assert setxml.find("devices/serial").attrib["type"] == "tcp"
+    assert setxml.find("devices/console").attrib["type"] == "tcp"
+
+
+def test_update_disks(make_mock_vm):
+    """
+    Test virt.udpate() with disk device changes
+    """
+    root_dir = os.path.join(salt.syspaths.ROOT_DIR, "srv", "salt-images")
+    xml_def = """
+        <domain type='kvm' id='7'>
+          <name>my_vm</name>
+          <memory unit='KiB'>1048576</memory>
+          <currentMemory unit='KiB'>1048576</currentMemory>
+          <vcpu placement='auto'>1</vcpu>
+          <on_reboot>restart</on_reboot>
+          <os>
+            <type arch='x86_64' machine='pc-i440fx-2.6'>hvm</type>
+          </os>
+          <devices>
+            <disk type='file' device='disk'>
+              <driver name='qemu' type='qcow2'/>
+              <source file='{}{}my_vm_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="network" device="disk">
+              <driver name='raw' type='qcow2'/>
+              <source protocol='rbd' name='libvirt-pool/my_vm_data2'>
+                <host name='ses2.tf.local'/>
+              </source>
+              <target dev='vdc' bus='virtio'/>
+              <alias name='virtio-disk2'/>
+              <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x2'/>
+            </disk>
+          </devices>
+        </domain>
+    """.format(
+        root_dir, os.sep
+    )
+    domain_mock = make_mock_vm(xml_def)
+
+    mock_chmod = MagicMock()
+    mock_run = MagicMock()
+    with patch.dict(os.__dict__, {"chmod": mock_chmod, "makedirs": MagicMock()}):
+        with patch.dict(virt.__salt__, {"cmd.run": mock_run}):
+            ret = virt.update(
+                "my_vm",
+                disk_profile="default",
+                disks=[
+                    {
+                        "name": "cddrive",
+                        "device": "cdrom",
+                        "source_file": None,
+                        "model": "ide",
+                    },
+                    {"name": "added", "size": 2048, "io": "threads"},
+                ],
+            )
+            added_disk_path = os.path.join(
+                virt.__salt__["config.get"]("virt:images"), "my_vm_added.qcow2"
+            )
+            assert mock_run.call_args[0][
+                0
+            ] == 'qemu-img create -f qcow2 "{}" 2048M'.format(added_disk_path)
+            assert mock_chmod.call_args[0][0] == added_disk_path
+            assert [
+                ET.fromstring(disk).find("source").get("file")
+                if str(disk).find("<source") > -1
+                else None
+                for disk in ret["disk"]["attached"]
+            ] == [None, os.path.join(root_dir, "my_vm_added.qcow2")]
+
+            assert [
+                ET.fromstring(disk).find("source").get("volume")
+                or ET.fromstring(disk).find("source").get("name")
+                for disk in ret["disk"]["detached"]
+            ] == ["libvirt-pool/my_vm_data2"]
+            assert domain_mock.attachDevice.call_count == 2
+            assert domain_mock.detachDevice.call_count == 1
+
+            setxml = ET.fromstring(virt.libvirt.openAuth().defineXML.call_args[0][0])
+            assert setxml.find("devices/disk[3]/driver").get("io") == "threads"
+
+
+def test_update_nics(make_mock_vm):
+    """
+    Test virt.update() with NIC device changes
+    """
+    domain_mock = make_mock_vm(
+        """
+        <domain type='kvm' id='7'>
+          <name>my_vm</name>
+          <memory unit='KiB'>1048576</memory>
+          <currentMemory unit='KiB'>1048576</currentMemory>
+          <vcpu placement='auto'>1</vcpu>
+          <on_reboot>restart</on_reboot>
+          <os>
+            <type arch='x86_64' machine='pc-i440fx-2.6'>hvm</type>
+          </os>
+          <devices>
+            <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>
+          </devices>
+        </domain>
+        """
+    )
+    mock_config = salt.utils.yaml.safe_load(
+        """
+          virt:
+             nic:
+                myprofile:
+                   - network: default
+                     name: eth0
+        """
+    )
+    with patch.dict(salt.modules.config.__opts__, mock_config):
+        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"},
+            ],
+        )
+        assert [
+            ET.fromstring(nic).find("source").get("network")
+            for nic in ret["interface"]["attached"]
+        ] == ["newnet"]
+        assert [
+            ET.fromstring(nic).find("source").get("network")
+            for nic in ret["interface"]["detached"]
+        ] == ["oldnet"]
+        domain_mock.attachDevice.assert_called_once()
+        domain_mock.detachDevice.assert_called_once()
+
+
+def test_update_remove_disks_nics(make_mock_vm):
+    """
+    Test virt.update() when removing nics and disks even if that may sound silly
+    """
+    root_dir = os.path.join(salt.syspaths.ROOT_DIR, "srv", "salt-images")
+    xml_def = """
+        <domain type='kvm' id='7'>
+          <name>my_vm</name>
+          <memory unit='KiB'>1048576</memory>
+          <currentMemory unit='KiB'>1048576</currentMemory>
+          <vcpu placement='auto'>1</vcpu>
+          <on_reboot>restart</on_reboot>
+          <os>
+            <type arch='x86_64' machine='pc-i440fx-2.6'>hvm</type>
+          </os>
+          <devices>
+            <disk type='file' device='disk'>
+              <driver name='qemu' type='qcow2'/>
+              <source file='{}{}my_vm_system.qcow2'/>
+              <backingStore/>
+              <target dev='vda' bus='virtio'/>
+              <alias name='virtio-disk0'/>
+              <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/>
+            </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>
+          </devices>
+        </domain>
+    """.format(
+        root_dir, os.sep
+    )
+    domain_mock = make_mock_vm(xml_def)
+
+    ret = virt.update(
+        "my_vm", nic_profile=None, interfaces=[], disk_profile=None, disks=[]
+    )
+    assert ret["interface"].get("attached", []) == []
+    assert len(ret["interface"]["detached"]) == 1
+    assert ret["disk"].get("attached", []) == []
+    assert len(ret["disk"]["detached"]) == 1
+
+    domain_mock.attachDevice.assert_not_called()
+    assert domain_mock.detachDevice.call_count == 2
+
+
+def test_update_no_change(make_mock_vm, make_mock_storage_pool):
+    """
+    Test virt.update() with no change
+    """
+    root_dir = os.path.join(salt.syspaths.ROOT_DIR, "srv", "salt-images")
+    xml_def = """
+        <domain type='kvm' id='7'>
+          <name>my_vm</name>
+          <memory unit='KiB'>1048576</memory>
+          <currentMemory unit='KiB'>1048576</currentMemory>
+          <vcpu placement='auto'>1</vcpu>
+          <on_reboot>restart</on_reboot>
+          <os>
+            <type arch='x86_64' machine='pc-i440fx-2.6'>hvm</type>
+            <boot dev="hd"/>
+          </os>
+          <devices>
+            <disk type='file' device='disk'>
+              <driver name='qemu' type='qcow2'/>
+              <source file='{}{}my_vm_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='volume' device='disk'>
+              <driver name='qemu' type='qcow2'/>
+              <source pool='default' volume='my_vm_data'/>
+              <backingStore/>
+              <target dev='vdb' bus='virtio'/>
+              <alias name='virtio-disk1'/>
+              <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x1'/>
+            </disk>
+            <disk type="network" device="disk">
+              <driver name='raw' type='qcow2'/>
+              <source protocol='rbd' name='libvirt-pool/my_vm_data2'>
+                <host name='ses2.tf.local'/>
+                <host name='ses3.tf.local' port='1234'/>
+                <auth username='libvirt'>
+                  <secret type='ceph' usage='pool_test-rbd'/>
+                </auth>
+              </source>
+              <target dev='vdc' bus='virtio'/>
+              <alias name='virtio-disk2'/>
+              <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x2'/>
+            </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>
+            <graphics type='spice' listen='127.0.0.1' autoport='yes'>
+              <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>
+            <serial type='pty'/>
+            <console type='pty'/>
+          </devices>
+        </domain>
+    """.format(
+        root_dir, os.sep
+    )
+    domain_mock = make_mock_vm(xml_def)
+
+    make_mock_storage_pool("default", "dir", ["my_vm_data"])
+    make_mock_storage_pool(
+        "test-rbd",
+        "rbd",
+        ["my_vm_data2"],
+        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>
+        """,
+    )
+    assert 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",
+            },
+        ],
+        graphics={
+            "type": "spice",
+            "listen": {"type": "address", "address": "127.0.0.1"},
+        },
+    ) == {
+        "definition": False,
+        "disk": {"attached": [], "detached": [], "updated": []},
+        "interface": {"attached": [], "detached": []},
+    }
+
+
+def test_update_failure(make_mock_vm):
+    """
+    Test virt.update() with errors
+    """
+    domain_mock = make_mock_vm()
+    virt.libvirt.openAuth().defineXML.side_effect = virt.libvirt.libvirtError(
+        "Test error"
+    )
+    with pytest.raises(virt.libvirt.libvirtError):
+        virt.update("my_vm", mem=2048)
+
+    # Failed single update failure case
+    virt.libvirt.openAuth().defineXML = MagicMock(return_value=True)
+    domain_mock.setMemoryFlags.side_effect = virt.libvirt.libvirtError(
+        "Failed to live change memory"
+    )
+
+    domain_mock.setVcpusFlags.return_value = 0
+    assert virt.update("my_vm", cpu=4, mem=2048) == {
+        "definition": True,
+        "errors": ["Failed to live change memory"],
+        "cpu": True,
+        "disk": {"attached": [], "detached": [], "updated": []},
+        "interface": {"attached": [], "detached": []},
+    }
diff --git a/tests/pytests/unit/modules/virt/test_helpers.py b/tests/pytests/unit/modules/virt/test_helpers.py
index 5410f45603..70f5a8a31f 100644
--- a/tests/pytests/unit/modules/virt/test_helpers.py
+++ b/tests/pytests/unit/modules/virt/test_helpers.py
@@ -34,3 +34,10 @@ def assert_called(mock, condition):
     I know it's a simple XOR, but makes the tests easier to read
     """
     assert not condition and not mock.called or condition and mock.called
+
+def assertEqualUnit(actual, expected, unit="KiB"):
+    """
+    Assert that two ElementTree nodes have the same value and unit
+    """
+    assert actual.get("unit") == unit
+    assert actual.text == str(expected)
diff --git a/tests/unit/modules/test_virt.py b/tests/unit/modules/test_virt.py
index f717513944..cac83e8717 100644
--- a/tests/unit/modules/test_virt.py
+++ b/tests/unit/modules/test_virt.py
@@ -2206,1124 +2206,6 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
                         stream_mock.finish.assert_called_once()
                         vol_mock.upload.assert_called_once_with(stream_mock, 0, 0, 0)
 
-    def test_update(self):
-        """
-        Test virt.update()
-        """
-        root_dir = os.path.join(salt.syspaths.ROOT_DIR, "srv", "salt-images")
-        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>
-              <on_reboot>restart</on_reboot>
-              <os>
-                <type arch='x86_64' machine='pc-i440fx-2.6'>hvm</type>
-                <boot dev="hd"/>
-              </os>
-              <devices>
-                <disk type='file' device='disk'>
-                  <driver name='qemu' type='qcow2'/>
-                  <source file='{}{}my_vm_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='volume' device='disk'>
-                  <driver name='qemu' type='qcow2'/>
-                  <source pool='default' volume='my_vm_data'/>
-                  <backingStore/>
-                  <target dev='vdb' bus='virtio'/>
-                  <alias name='virtio-disk1'/>
-                  <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x1'/>
-                </disk>
-                <disk type="network" device="disk">
-                  <driver name='raw' type='qcow2'/>
-                  <source protocol='rbd' name='libvirt-pool/my_vm_data2'>
-                    <host name='ses2.tf.local'/>
-                    <host name='ses3.tf.local' port='1234'/>
-                    <auth username='libvirt'>
-                      <secret type='ceph' usage='pool_test-rbd'/>
-                    </auth>
-                  </source>
-                  <target dev='vdc' bus='virtio'/>
-                  <alias name='virtio-disk2'/>
-                  <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x2'/>
-                </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' listen='127.0.0.1' autoport='yes'>
-                  <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>
-                <serial type='pty'/>
-                <console type='pty'/>
-              </devices>
-            </domain>
-        """.format(
-            root_dir, os.sep
-        )
-        domain_mock = self.set_mock_vm("my_vm", xml)
-        domain_mock.OSType = MagicMock(return_value="hvm")
-        define_mock = MagicMock(return_value=True)
-        self.mock_conn.defineXML = define_mock
-
-        # No parameter passed case
-        self.assertEqual(
-            {
-                "definition": False,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            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": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update(
-                "my_vm",
-                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,
-                numatune=None,
-            ),
-        )
-
-        # test cpu passed as an integer case
-        setvcpus_mock = MagicMock(return_value=0)
-        domain_mock.setVcpusFlags = setvcpus_mock
-        self.assertEqual(
-            {
-                "definition": True,
-                "cpu": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            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)
-        define_mock.reset_mock()
-
-        # test updating vcpu attribute
-        vcpu = {
-            "placement": "static",
-            "cpuset": "0-11",
-            "current": 5,
-            "maximum": 12,
-        }
-        self.assertEqual(
-            {
-                "definition": True,
-                "cpu": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", cpu=vcpu),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(setxml.find("vcpu").text, "12")
-        self.assertEqual(setxml.find("vcpu").attrib["placement"], "static")
-        self.assertEqual(
-            setxml.find("vcpu").attrib["cpuset"], "0,1,2,3,4,5,6,7,8,9,10,11"
-        )
-        self.assertEqual(setxml.find("vcpu").attrib["current"], "5")
-
-        # test adding vcpus elements
-        vcpus = {
-            "vcpus": {
-                "0": {"enabled": True, "hotpluggable": False, "order": 1},
-                "1": {"enabled": False, "hotpluggable": True},
-            }
-        }
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", cpu=vcpus),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(setxml.find("./vcpus/vcpu/[@id='0']").attrib["id"], "0")
-        self.assertEqual(setxml.find("./vcpus/vcpu/[@id='0']").attrib["enabled"], "yes")
-        self.assertEqual(
-            setxml.find("./vcpus/vcpu/[@id='0']").attrib["hotpluggable"], "no"
-        )
-        self.assertEqual(setxml.find("./vcpus/vcpu/[@id='0']").attrib["order"], "1")
-        self.assertEqual(setxml.find("./vcpus/vcpu/[@id='1']").attrib["id"], "1")
-        self.assertEqual(setxml.find("./vcpus/vcpu/[@id='1']").attrib["enabled"], "no")
-        self.assertEqual(
-            setxml.find("./vcpus/vcpu/[@id='1']").attrib["hotpluggable"], "yes"
-        )
-        self.assertEqual(
-            setxml.find("./vcpus/vcpu/[@id='1']").attrib.get("order"), None
-        )
-
-        # test adding cpu attribute
-        cpu_atr = {"mode": "custom", "match": "exact", "check": "full"}
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", cpu=cpu_atr),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(setxml.find("cpu").attrib["mode"], "custom")
-        self.assertEqual(setxml.find("cpu").attrib["match"], "exact")
-        self.assertEqual(setxml.find("cpu").attrib["check"], "full")
-
-        # test adding cpu model
-        cpu_model = {
-            "model": {
-                "name": "coreduo",
-                "fallback": "allow",
-                "vendor_id": "Genuine20201",
-            }
-        }
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", cpu=cpu_model),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(
-            setxml.find("cpu").find("model").attrib.get("vendor_id"), "Genuine20201"
-        )
-        self.assertEqual(
-            setxml.find("cpu").find("model").attrib.get("fallback"), "allow"
-        )
-        self.assertEqual(setxml.find("cpu").find("model").text, "coreduo")
-
-        # test adding cpu vendor
-        cpu_vendor = {"vendor": "Intel"}
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", cpu=cpu_vendor),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(setxml.find("cpu").find("vendor").text, "Intel")
-
-        # test adding cpu topology
-        cpu_topology = {"topology": {"sockets": 1, "cores": 12, "threads": 1}}
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", cpu=cpu_topology),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(setxml.find("cpu").find("topology").attrib.get("sockets"), "1")
-        self.assertEqual(setxml.find("cpu").find("topology").attrib.get("cores"), "12")
-        self.assertEqual(setxml.find("cpu").find("topology").attrib.get("threads"), "1")
-
-        # test adding cache
-        cpu_cache = {"cache": {"mode": "emulate", "level": 3}}
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", cpu=cpu_cache),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(setxml.find("cpu").find("cache").attrib.get("level"), "3")
-        self.assertEqual(setxml.find("cpu").find("cache").attrib.get("mode"), "emulate")
-
-        # test adding feature
-        cpu_feature = {"features": {"lahf": "optional", "pcid": "disable"}}
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", cpu=cpu_feature),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(
-            setxml.find("./cpu/feature[@name='pcid']").attrib.get("policy"), "disable"
-        )
-        self.assertEqual(
-            setxml.find("./cpu/feature[@name='lahf']").attrib.get("policy"), "optional"
-        )
-
-        # test adding numa cell
-        numa_cell = {
-            "numa": {
-                "0": {
-                    "cpus": "0-3",
-                    "memory": "1g",
-                    "discard": True,
-                    "distances": {0: 10, 1: 21, 2: 31, 3: 41},
-                },
-                "1": {
-                    "cpus": "4-6",
-                    "memory": "0.5g",
-                    "discard": False,
-                    "memAccess": "shared",
-                    "distances": {0: 21, 1: 10, 2: 15, 3: 30},
-                },
-            }
-        }
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", cpu=numa_cell),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(
-            setxml.find("./cpu/numa/cell/[@id='0']").attrib["cpus"], "0,1,2,3"
-        )
-        self.assertEqual(
-            setxml.find("./cpu/numa/cell/[@id='0']").attrib["memory"], str(1024 ** 3)
-        )
-        self.assertEqual(setxml.find("./cpu/numa/cell/[@id='0']").get("unit"), "bytes")
-        self.assertEqual(
-            setxml.find("./cpu/numa/cell/[@id='0']").attrib["discard"], "yes"
-        )
-        self.assertEqual(
-            setxml.find("./cpu/numa/cell/[@id='0']/distances/sibling/[@id='0']").attrib[
-                "value"
-            ],
-            "10",
-        )
-        self.assertEqual(
-            setxml.find("./cpu/numa/cell/[@id='0']/distances/sibling/[@id='1']").attrib[
-                "value"
-            ],
-            "21",
-        )
-        self.assertEqual(
-            setxml.find("./cpu/numa/cell/[@id='0']/distances/sibling/[@id='2']").attrib[
-                "value"
-            ],
-            "31",
-        )
-        self.assertEqual(
-            setxml.find("./cpu/numa/cell/[@id='0']/distances/sibling/[@id='3']").attrib[
-                "value"
-            ],
-            "41",
-        )
-        self.assertEqual(
-            setxml.find("./cpu/numa/cell/[@id='1']").attrib["cpus"], "4,5,6"
-        )
-        self.assertEqual(
-            setxml.find("./cpu/numa/cell/[@id='1']").attrib["memory"],
-            str(int(1024 ** 3 / 2)),
-        )
-        self.assertEqual(
-            setxml.find("./cpu/numa/cell/[@id='1']").get("unit"), "bytes",
-        )
-        self.assertEqual(
-            setxml.find("./cpu/numa/cell/[@id='1']").attrib["discard"], "no"
-        )
-        self.assertEqual(
-            setxml.find("./cpu/numa/cell/[@id='1']").attrib["memAccess"], "shared"
-        )
-        self.assertEqual(
-            setxml.find("./cpu/numa/cell/[@id='1']/distances/sibling/[@id='0']").attrib[
-                "value"
-            ],
-            "21",
-        )
-        self.assertEqual(
-            setxml.find("./cpu/numa/cell/[@id='1']/distances/sibling/[@id='1']").attrib[
-                "value"
-            ],
-            "10",
-        )
-        self.assertEqual(
-            setxml.find("./cpu/numa/cell/[@id='1']/distances/sibling/[@id='2']").attrib[
-                "value"
-            ],
-            "15",
-        )
-        self.assertEqual(
-            setxml.find("./cpu/numa/cell/[@id='1']/distances/sibling/[@id='3']").attrib[
-                "value"
-            ],
-            "30",
-        )
-
-        # Update boot parameter case
-        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.assertEqualUnit(
-            setxml.find("memtune").find("soft_limit"), int(0.5 * 1024 ** 3), "bytes"
-        )
-        self.assertEqualUnit(
-            setxml.find("memtune").find("hard_limit"), 1024 * 1024 ** 2, "bytes"
-        )
-        self.assertEqualUnit(
-            setxml.find("memtune").find("swap_hard_limit"), 2048 * 1024 ** 2, "bytes"
-        )
-        self.assertEqualUnit(
-            setxml.find("memtune").find("min_guarantee"), 1 * 1024 ** 3, "bytes"
-        )
-
-        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 numatune case
-        numatune = {
-            "memory": {"mode": "strict", "nodeset": 1},
-            "memnodes": {
-                0: {"mode": "strict", "nodeset": 1},
-                1: {"mode": "preferred", "nodeset": 2},
-            },
-        }
-
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", numatune=numatune),
-        )
-
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(
-            setxml.find("numatune").find("memory").attrib.get("mode"), "strict"
-        )
-
-        self.assertEqual(
-            setxml.find("numatune").find("memory").attrib.get("nodeset"), "1"
-        )
-
-        self.assertEqual(
-            setxml.find("./numatune/memnode/[@cellid='0']").attrib.get("mode"), "strict"
-        )
-
-        self.assertEqual(
-            setxml.find("./numatune/memnode/[@cellid='0']").attrib.get("nodeset"), "1"
-        )
-
-        self.assertEqual(
-            setxml.find("./numatune/memnode/[@cellid='1']").attrib.get("mode"),
-            "preferred",
-        )
-
-        self.assertEqual(
-            setxml.find("./numatune/memnode/[@cellid='1']").attrib.get("nodeset"), "2"
-        )
-
-        # 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 memory backing case
-        mem_back = {
-            "hugepages": [
-                {"nodeset": "1-5,^4", "size": "1g"},
-                {"nodeset": "4", "size": "2g"},
-            ],
-            "nosharepages": True,
-            "locked": True,
-            "source": "file",
-            "access": "shared",
-            "allocation": "immediate",
-            "discard": True,
-        }
-
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", mem=mem_back),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertDictEqual(
-            {
-                p.get("nodeset"): {"size": p.get("size"), "unit": p.get("unit")}
-                for p in setxml.findall("memoryBacking/hugepages/page")
-            },
-            {
-                "1,2,3,5": {"size": str(1024 ** 3), "unit": "bytes"},
-                "4": {"size": str(2 * 1024 ** 3), "unit": "bytes"},
-            },
-        )
-        self.assertNotEqual(setxml.find("./memoryBacking/nosharepages"), None)
-        self.assertIsNone(setxml.find("./memoryBacking/nosharepages").text)
-        self.assertEqual([], setxml.find("./memoryBacking/nosharepages").keys())
-        self.assertNotEqual(setxml.find("./memoryBacking/locked"), None)
-        self.assertIsNone(setxml.find("./memoryBacking/locked").text)
-        self.assertEqual([], setxml.find("./memoryBacking/locked").keys())
-        self.assertEqual(setxml.find("./memoryBacking/source").attrib["type"], "file")
-        self.assertEqual(setxml.find("./memoryBacking/access").attrib["mode"], "shared")
-        self.assertNotEqual(setxml.find("./memoryBacking/discard"), None)
-
-        # test adding iothreads
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", cpu={"iothreads": 5}),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(setxml.find("iothreads").text, "5")
-
-        # test adding cpu tune parameters
-        cputune = {
-            "shares": 2048,
-            "period": 122000,
-            "quota": -1,
-            "global_period": 1000000,
-            "global_quota": -3,
-            "emulator_period": 1200000,
-            "emulator_quota": -10,
-            "iothread_period": 133000,
-            "iothread_quota": -1,
-            "vcpupin": {0: "1-4,^2", 1: "0,1", 2: "2,3", 3: "0,4"},
-            "emulatorpin": "1-3",
-            "iothreadpin": {1: "5-6", 2: "7-8"},
-            "vcpusched": [
-                {"scheduler": "fifo", "priority": 1, "vcpus": "0"},
-                {"scheduler": "fifo", "priotity": 2, "vcpus": "1"},
-                {"scheduler": "idle", "priotity": 3, "vcpus": "2"},
-            ],
-            "iothreadsched": [{"scheduler": "batch", "iothreads": "7"}],
-            "cachetune": {
-                "0-3": {
-                    0: {"level": 3, "type": "both", "size": 3},
-                    1: {"level": 3, "type": "both", "size": 3},
-                    "monitor": {1: 3, "0-3": 3},
-                },
-                "4-5": {"monitor": {4: 3, 5: 2}},
-            },
-            "memorytune": {"0-2": {0: 60}, "3-4": {0: 50, 1: 70}},
-        }
-        self.assertEqual(
-            {
-                "definition": True,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", cpu={"tuning": cputune}),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(setxml.find("cputune").find("shares").text, "2048")
-        self.assertEqual(setxml.find("cputune").find("period").text, "122000")
-        self.assertEqual(setxml.find("cputune").find("quota").text, "-1")
-        self.assertEqual(setxml.find("cputune").find("global_period").text, "1000000")
-        self.assertEqual(setxml.find("cputune").find("global_quota").text, "-3")
-        self.assertEqual(setxml.find("cputune").find("emulator_period").text, "1200000")
-        self.assertEqual(setxml.find("cputune").find("emulator_quota").text, "-10")
-        self.assertEqual(setxml.find("cputune").find("iothread_period").text, "133000")
-        self.assertEqual(setxml.find("cputune").find("iothread_quota").text, "-1")
-        self.assertEqual(
-            setxml.find("cputune").find("vcpupin[@vcpu='0']").attrib.get("cpuset"),
-            "1,3,4",
-        )
-        self.assertEqual(
-            setxml.find("cputune").find("vcpupin[@vcpu='1']").attrib.get("cpuset"),
-            "0,1",
-        )
-        self.assertEqual(
-            setxml.find("cputune").find("vcpupin[@vcpu='2']").attrib.get("cpuset"),
-            "2,3",
-        )
-        self.assertEqual(
-            setxml.find("cputune").find("vcpupin[@vcpu='3']").attrib.get("cpuset"),
-            "0,4",
-        )
-        self.assertEqual(
-            setxml.find("cputune").find("emulatorpin").attrib.get("cpuset"), "1,2,3"
-        )
-        self.assertEqual(
-            setxml.find("cputune")
-            .find("iothreadpin[@iothread='1']")
-            .attrib.get("cpuset"),
-            "5,6",
-        )
-        self.assertEqual(
-            setxml.find("cputune")
-            .find("iothreadpin[@iothread='2']")
-            .attrib.get("cpuset"),
-            "7,8",
-        )
-        self.assertEqual(
-            setxml.find("cputune").find("vcpusched[@vcpus='0']").attrib.get("priority"),
-            "1",
-        )
-        self.assertEqual(
-            setxml.find("cputune")
-            .find("vcpusched[@vcpus='0']")
-            .attrib.get("scheduler"),
-            "fifo",
-        )
-        self.assertEqual(
-            setxml.find("cputune").find("iothreadsched").attrib.get("iothreads"), "7"
-        )
-        self.assertEqual(
-            setxml.find("cputune").find("iothreadsched").attrib.get("scheduler"),
-            "batch",
-        )
-        self.assertIsNotNone(setxml.find("./cputune/cachetune[@vcpus='0,1,2,3']"))
-        self.assertEqual(
-            setxml.find(
-                "./cputune/cachetune[@vcpus='0,1,2,3']/cache[@id='0']"
-            ).attrib.get("level"),
-            "3",
-        )
-        self.assertEqual(
-            setxml.find(
-                "./cputune/cachetune[@vcpus='0,1,2,3']/cache[@id='0']"
-            ).attrib.get("type"),
-            "both",
-        )
-        self.assertEqual(
-            setxml.find(
-                "./cputune/cachetune[@vcpus='0,1,2,3']/monitor[@vcpus='1']"
-            ).attrib.get("level"),
-            "3",
-        )
-        self.assertNotEqual(
-            setxml.find("./cputune/cachetune[@vcpus='0,1,2,3']/monitor[@vcpus='1']"),
-            None,
-        )
-        self.assertNotEqual(
-            setxml.find("./cputune/cachetune[@vcpus='4,5']").attrib.get("vcpus"), None
-        )
-        self.assertEqual(
-            setxml.find("./cputune/cachetune[@vcpus='4,5']/cache[@id='0']"), None
-        )
-        self.assertEqual(
-            setxml.find(
-                "./cputune/cachetune[@vcpus='4,5']/monitor[@vcpus='4']"
-            ).attrib.get("level"),
-            "3",
-        )
-        self.assertEqual(
-            setxml.find(
-                "./cputune/cachetune[@vcpus='4,5']/monitor[@vcpus='5']"
-            ).attrib.get("level"),
-            "2",
-        )
-        self.assertNotEqual(setxml.find("./cputune/memorytune[@vcpus='0,1,2']"), None)
-        self.assertEqual(
-            setxml.find(
-                "./cputune/memorytune[@vcpus='0,1,2']/node[@id='0']"
-            ).attrib.get("bandwidth"),
-            "60",
-        )
-        self.assertNotEqual(setxml.find("./cputune/memorytune[@vcpus='3,4']"), None)
-        self.assertEqual(
-            setxml.find("./cputune/memorytune[@vcpus='3,4']/node[@id='0']").attrib.get(
-                "bandwidth"
-            ),
-            "50",
-        )
-        self.assertEqual(
-            setxml.find("./cputune/memorytune[@vcpus='3,4']/node[@id='1']").attrib.get(
-                "bandwidth"
-            ),
-            "70",
-        )
-
-        # 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,
-                            "io": "threads",
-                            "iothread_id": 2,
-                        },
-                    ],
-                )
-                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)
-
-                setxml = ET.fromstring(define_mock.call_args[0][0])
-                self.assertEqual(
-                    "threads", setxml.find("devices/disk[3]/driver").get("io")
-                )
-                self.assertEqual(
-                    "2", setxml.find("devices/disk[3]/driver").get("iothread")
-                )
-
-        # 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.assertFalse(ret["interface"].get("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.assertFalse(ret["disk"].get("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"))
-
-        # Serial and console test case
-        self.assertEqual(
-            {
-                "definition": False,
-                "disk": {"attached": [], "detached": [], "updated": []},
-                "interface": {"attached": [], "detached": []},
-            },
-            virt.update("my_vm", serials=[{"type": "tcp"}], consoles=[{"type": "tcp"}]),
-        )
-        setxml = ET.fromstring(define_mock.call_args[0][0])
-        self.assertEqual(setxml.find("devices/serial").attrib["type"], "pty")
-        self.assertEqual(setxml.find("devices/console").attrib["type"], "pty")
-
-        # 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
-- 
2.30.0

Places

File virt-uefi-fix-backport-312.patch of Package salt.19538

Places