From ce8d431fad81547d1e76f264a172d27e2b4fccb1 Mon Sep 17 00:00:00 2001 From: Larry Dewey Date: Tue, 7 Jan 2020 02:48:11 -0700 Subject: [PATCH] virt: adding kernel boot parameters to libvirt xml #55245 (#197) * virt: adding kernel boot parameters to libvirt xml SUSE's autoyast and Red Hat's kickstart take advantage of kernel paths, initrd paths, and kernel boot command line parameters. These changes provide the option of using these, and will allow salt and autoyast/kickstart to work together. Signed-off-by: Larry Dewey * virt: Download linux and initrd Signed-off-by: Larry Dewey --- salt/modules/virt.py | 129 ++++++++++++++++++++++- salt/states/virt.py | 29 ++++- salt/templates/virt/libvirt_domain.jinja | 12 ++- salt/utils/virt.py | 45 +++++++- tests/unit/modules/test_virt.py | 79 +++++++++++++- tests/unit/states/test_virt.py | 19 +++- 6 files changed, 302 insertions(+), 11 deletions(-) diff --git a/salt/modules/virt.py b/salt/modules/virt.py index dedcf8cb6f5..0f62856f5c5 100644 --- a/salt/modules/virt.py +++ b/salt/modules/virt.py @@ -106,6 +106,8 @@ import salt.utils.validate.net import salt.utils.versions import salt.utils.yaml + +from salt.utils.virt import check_remote, download_remote from salt.exceptions import CommandExecutionError, SaltInvocationError from salt.ext import six from salt.ext.six.moves import range # pylint: disable=import-error,redefined-builtin @@ -119,6 +121,8 @@ ) ) +CACHE_DIR = '/var/lib/libvirt/saltinst' + VIRT_STATE_NAME_MAP = {0: 'running', 1: 'running', 2: 'running', @@ -532,6 +536,7 @@ def _gen_xml(name, os_type, arch, graphics=None, + boot=None, **kwargs): ''' Generate the XML string to define a libvirt VM @@ -568,11 +573,15 @@ def _gen_xml(name, else: context['boot_dev'] = ['hd'] + context['boot'] = boot if boot else {} + if os_type == 'xen': # Compute the Xen PV boot method if __grains__['os_family'] == 'Suse': - context['kernel'] = '/usr/lib/grub2/x86_64-xen/grub.xen' - context['boot_dev'] = [] + if not boot or not boot.get('kernel', None): + context['boot']['kernel'] = \ + '/usr/lib/grub2/x86_64-xen/grub.xen' + context['boot_dev'] = [] if 'serial_type' in kwargs: context['serial_type'] = kwargs['serial_type'] @@ -1115,6 +1124,34 @@ def _get_merged_nics(hypervisor, profile, interfaces=None, dmac=None): return nicp +def _handle_remote_boot_params(orig_boot): + """ + Checks if the boot parameters contain a remote path. If so, it will copy + the parameters, download the files specified in the remote path, and return + a new dictionary with updated paths containing the canonical path to the + kernel and/or initrd + + :param orig_boot: The original boot parameters passed to the init or update + functions. + """ + saltinst_dir = None + new_boot = orig_boot.copy() + + try: + for key in ['kernel', 'initrd']: + if 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 + except Exception as err: + raise err + + def init(name, cpu, mem, @@ -1136,6 +1173,7 @@ def init(name, graphics=None, os_type=None, arch=None, + boot=None, **kwargs): ''' Initialize a new vm @@ -1266,6 +1304,22 @@ def init(name, :param password: password to connect with, overriding defaults .. versionadded:: 2019.2.0 + :param boot: + Specifies kernel for the virtual machine, as well as boot parameters + for the virtual machine. This is an optionl parameter, and all of the + keys are optional within the dictionary. If a remote path is provided + to kernel or initrd, salt will handle the downloading of the specified + remote fild, and will modify the XML accordingly. + + .. code-block:: python + + { + 'kernel': '/root/f8-i386-vmlinuz', + 'initrd': '/root/f8-i386-initrd', + 'cmdline': 'console=ttyS0 ks=http://example.com/f8-i386/os/' + } + + .. versionadded:: neon .. _init-nic-def: @@ -1513,7 +1567,11 @@ def init(name, if arch is None: arch = 'x86_64' if 'x86_64' in arches else arches[0] - vm_xml = _gen_xml(name, cpu, mem, diskp, nicp, hypervisor, os_type, arch, graphics, **kwargs) + if boot is not None: + boot = _handle_remote_boot_params(boot) + + vm_xml = _gen_xml(name, cpu, mem, diskp, nicp, hypervisor, os_type, arch, + graphics, boot, **kwargs) conn = __get_conn(**kwargs) try: conn.defineXML(vm_xml) @@ -1692,6 +1750,7 @@ def update(name, interfaces=None, graphics=None, live=True, + boot=None, **kwargs): ''' Update the definition of an existing domain. @@ -1727,6 +1786,23 @@ def update(name, :param username: username to connect with, overriding defaults :param password: password to connect with, overriding defaults + :param boot: + Specifies kernel for the virtual machine, as well as boot parameters + for the virtual machine. This is an optionl parameter, and all of the + keys are optional within the dictionary. If a remote path is provided + to kernel or initrd, salt will handle the downloading of the specified + remote fild, and will modify the XML accordingly. + + .. code-block:: python + + { + 'kernel': '/root/f8-i386-vmlinuz', + 'initrd': '/root/f8-i386-initrd', + 'cmdline': 'console=ttyS0 ks=http://example.com/f8-i386/os/' + } + + .. versionadded:: neon + :return: Returns a dictionary indicating the status of what has been done. It is structured in @@ -1767,6 +1843,10 @@ def update(name, # Compute the XML to get the disks, interfaces and graphics hypervisor = desc.get('type') all_disks = _disk_profile(disk_profile, hypervisor, disks, name, **kwargs) + + if boot is not None: + boot = _handle_remote_boot_params(boot) + new_desc = ElementTree.fromstring(_gen_xml(name, cpu, mem, @@ -1776,6 +1856,7 @@ def update(name, domain.OSType(), desc.find('.//os/type').get('arch'), graphics, + boot, **kwargs)) # Update the cpu @@ -1785,6 +1866,48 @@ def update(name, cpu_node.set('current', six.text_type(cpu)) need_update = True + # Update the kernel boot parameters + boot_tags = ['kernel', 'initrd', 'cmdline'] + parent_tag = desc.find('os') + + # We need to search for each possible subelement, and update it. + for tag in boot_tags: + # The Existing Tag... + found_tag = desc.find(tag) + + # The new value + boot_tag_value = boot.get(tag, None) if boot else None + + # Existing tag is found and values don't match + if found_tag and found_tag.text != boot_tag_value: + + # If the existing tag is found, but the new value is None + # remove it. If the existing tag is found, and the new value + # doesn't match update it. In either case, mark for update. + if boot_tag_value is None \ + and boot is not None \ + and parent_tag is not None: + ElementTree.remove(parent_tag, tag) + else: + found_tag.text = boot_tag_value + + need_update = True + + # Existing tag is not found, but value is not None + elif found_tag is None and boot_tag_value is not None: + + # Need to check for parent tag, and add it if it does not exist. + # Add a subelement and set the value to the new value, and then + # mark for update. + if parent_tag is not None: + child_tag = ElementTree.SubElement(parent_tag, tag) + else: + new_parent_tag = ElementTree.Element('os') + child_tag = ElementTree.SubElement(new_parent_tag, tag) + + child_tag.text = boot_tag_value + need_update = True + # Update the memory, note that libvirt outputs all memory sizes in KiB for mem_node_name in ['memory', 'currentMemory']: mem_node = desc.find(mem_node_name) diff --git a/salt/states/virt.py b/salt/states/virt.py index 68e9ac6fb6a..c700cae849f 100644 --- a/salt/states/virt.py +++ b/salt/states/virt.py @@ -264,7 +264,8 @@ def running(name, username=None, password=None, os_type=None, - arch=None): + arch=None, + boot=None): ''' Starts an existing guest, or defines and starts a new VM with specified arguments. @@ -349,6 +350,23 @@ def running(name, .. versionadded:: Neon + :param boot: + Specifies kernel for the virtual machine, as well as boot parameters + for the virtual machine. This is an optionl parameter, and all of the + keys are optional within the dictionary. If a remote path is provided + to kernel or initrd, salt will handle the downloading of the specified + remote fild, and will modify the XML accordingly. + + .. code-block:: python + + { + 'kernel': '/root/f8-i386-vmlinuz', + 'initrd': '/root/f8-i386-initrd', + 'cmdline': 'console=ttyS0 ks=http://example.com/f8-i386/os/' + } + + .. versionadded:: neon + .. rubric:: Example States Make sure an already-defined virtual machine called ``domain_name`` is running: @@ -413,7 +431,8 @@ def running(name, live=False, connection=connection, username=username, - password=password) + password=password, + boot=boot) if status['definition']: action_msg = 'updated and started' __salt__['virt.start'](name) @@ -431,7 +450,8 @@ def running(name, graphics=graphics, connection=connection, username=username, - password=password) + password=password, + boot=boot) ret['changes'][name] = status if status.get('errors', None): ret['comment'] = 'Domain {0} updated, but some live update(s) failed'.format(name) @@ -466,7 +486,8 @@ def running(name, priv_key=priv_key, connection=connection, username=username, - password=password) + password=password, + boot=boot) ret['changes'][name] = 'Domain defined and started' ret['comment'] = 'Domain {0} defined and started'.format(name) except libvirt.libvirtError as err: diff --git a/salt/templates/virt/libvirt_domain.jinja b/salt/templates/virt/libvirt_domain.jinja index 0b4c3fc2d6e..fdaea168f2b 100644 --- a/salt/templates/virt/libvirt_domain.jinja +++ b/salt/templates/virt/libvirt_domain.jinja @@ -5,7 +5,17 @@ {{ mem }} {{ os_type }} - {% if kernel %}{{ kernel }}{% endif %} + {% if boot %} + {% if 'kernel' in boot %} + {{ boot.kernel }} + {% endif %} + {% if 'initrd' in boot %} + {{ boot.initrd }} + {% endif %} + {% if 'cmdline' in boot %} + {{ boot.cmdline }} + {% endif %} + {% endif %} {% for dev in boot_dev %} {% endfor %} diff --git a/salt/utils/virt.py b/salt/utils/virt.py index 9dad849c0e2..b36adba81c3 100644 --- a/salt/utils/virt.py +++ b/salt/utils/virt.py @@ -6,16 +6,59 @@ # Import python libs import os +import re import time import logging +import hashlib + +# pylint: disable=E0611 +from salt.ext.six.moves.urllib.parse import urlparse +from salt.ext.six.moves.urllib import request # Import salt libs import salt.utils.files - log = logging.getLogger(__name__) +def download_remote(url, dir): + """ + Attempts to download a file specified by 'url' + + :param url: The full remote path of the file which should be downloaded. + :param dir: The path the file should be downloaded to. + """ + + try: + rand = hashlib.md5(os.urandom(32)).hexdigest() + remote_filename = urlparse(url).path.split('/')[-1] + full_directory = \ + os.path.join(dir, "{}-{}".format(rand, remote_filename)) + with salt.utils.files.fopen(full_directory, 'wb') as file,\ + request.urlopen(url) as response: + file.write(response.rease()) + + return full_directory + + except Exception as err: + raise err + + +def check_remote(cmdline_path): + """ + Checks to see if the path provided contains ftp, http, or https. Returns + the full path if it is found. + + :param cmdline_path: The path to the initrd image or the kernel + """ + regex = re.compile('^(ht|f)tps?\\b') + + if regex.match(urlparse(cmdline_path).scheme): + return True + + return False + + class VirtKey(object): ''' Used to manage key signing requests. diff --git a/tests/unit/modules/test_virt.py b/tests/unit/modules/test_virt.py index 6f594a8ff34..4bdb933a2d0 100644 --- a/tests/unit/modules/test_virt.py +++ b/tests/unit/modules/test_virt.py @@ -10,6 +10,7 @@ import os import re import datetime +import shutil # Import Salt Testing libs from tests.support.mixins import LoaderModuleMockMixin @@ -23,6 +24,7 @@ from salt._compat import ElementTree as ET import salt.config import salt.syspaths +import tempfile from salt.exceptions import CommandExecutionError # Import third party libs @@ -30,7 +32,6 @@ # pylint: disable=import-error from salt.ext.six.moves import range # pylint: disable=redefined-builtin - # pylint: disable=invalid-name,protected-access,attribute-defined-outside-init,too-many-public-methods,unused-argument @@ -610,6 +611,7 @@ def test_gen_xml_for_xen_default_profile(self): 'xen', 'xen', 'x86_64', + boot=None ) root = ET.fromstring(xml_data) self.assertEqual(root.attrib['type'], 'xen') @@ -1123,6 +1125,67 @@ def test_init(self): self.assertFalse('