From 86e7e81f13134382d50e7a12f544d7b25cae4a4b Mon Sep 17 00:00:00 2001 From: Cedric Bosdonnat 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 = """ + + my_vm + 1048576 + 1048576 + 1 + restart + + hvm + + + """ + 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 = "".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( + """ + + my_vm + 1048576 + 1048576 + 1 + restart + + hvm + + + + """ + ) + 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( + """ + + my_vm + 27434df0-706d-4603-8ad7-5a88d19a3417 + 524288 + 524288 + 1 + + /machine + + + hvm + /usr/share/qemu/edk2-x86_64-code.fd + /var/lib/libvirt/qemu/nvram/vm01_VARS.fd + + restart + + """ + ) + + 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( + """ + + my_vm + 1048576 + 1048576 + 1 + restart + + hvm + + + + + + + + """ + ) + 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( + """ + + my_vm + 1048576 + 1048576 + 1 + restart + + hvm + + + + + + + """ + ) + + 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 = """ + + my_vm + 1048576 + 1048576 + 1 + restart + + hvm + + + + + + + + +
+ + + + + + + + +
+ + + + """.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(" -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( + """ + + my_vm + 1048576 + 1048576 + 1 + restart + + hvm + + + + + + + + +
+ + + + + + + +
+ + + + """ + ) + 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 = """ + + my_vm + 1048576 + 1048576 + 1 + restart + + hvm + + + + + + + + +
+ + + + + + + +
+ + + + """.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 = """ + + my_vm + 1048576 + 1048576 + 1 + restart + + hvm + + + + + + + + + +
+ + + + + + + +
+ + + + + + + + + + + + +
+ + + + + + + +
+ + + + +