From 19bcec651cabb970b195b9dddb348eb6503cf27c Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Wed, 12 Jun 2019 17:55:30 -0400 Subject: [PATCH] virt-install: Push validation down into the installer It's hard to validate whether something like --extra-args or --initrd-inject is supported based on the command line arguments. It's easier to let the installer.py figure it out because it's the authoritative source --- virt-install | 30 +++++---------------- virtManager/create.py | 2 +- virtinst/installer.py | 28 ++++++++++---------- virtinst/installertreemedia.py | 48 ++++++++++++++++++++++------------ 4 files changed, 54 insertions(+), 54 deletions(-) diff --git a/virt-install b/virt-install index 5813d3a9e..a020fa8a8 100755 --- a/virt-install +++ b/virt-install @@ -341,7 +341,7 @@ _cdrom_location_man_page = _("See the man page for examples of " "using --location with CDROM media") -def check_option_collisions(options, guest, installer): +def check_option_collisions(options, guest): if options.noreboot and options.transient: fail(_("--noreboot and --transient can not be specified together")) @@ -354,20 +354,6 @@ def check_option_collisions(options, guest, installer): fail(_("Install methods (%s) cannot be specified for " "container guests") % install_methods) - cdrom_err = "" - if installer.cdrom: - cdrom_err = " " + _cdrom_location_man_page - if not options.location and options.extra_args: - fail(_("--extra-args only work if specified with --location.") + - cdrom_err) - if not options.location and options.initrd_inject: - fail(_("--initrd-inject only works if specified with --location.") + - cdrom_err) - - if options.unattended: - if options.initrd_inject or options.extra_args: - fail(_("--unattended does not support --initrd-inject nor --extra-args.")) - def _show_nographics_warnings(options, guest, installer): if guest.devices.graphics: @@ -494,18 +480,16 @@ def build_installer(options, guest): install_kernel=install_kernel, install_initrd=install_initrd, install_kernel_args=install_kernel_args) + if cdrom and options.livecd: installer.livecd = True if options.unattended: unattended_data = cli.parse_unattended(options.unattended) - options.unattended = None - installer.set_unattended_data(unattended_data) - else: - if extra_args: - installer.extra_args = extra_args - if options.initrd_inject: - installer.set_initrd_injections(options.initrd_inject) + if extra_args: + installer.set_extra_args(extra_args) + if options.initrd_inject: + installer.set_initrd_injections(options.initrd_inject) if options.autostart: installer.autostart = True @@ -604,7 +588,7 @@ def build_guest_instance(conn, options): cli.validate_disk(disk) validate_required_options(options, guest, installer) - check_option_collisions(options, guest, installer) + check_option_collisions(options, guest) show_warnings(options, guest, installer) return guest, installer diff --git a/virtManager/create.py b/virtManager/create.py index e307e10b1..5877a403e 100644 --- a/virtManager/create.py +++ b/virtManager/create.py @@ -1595,7 +1595,7 @@ class vmmCreate(vmmGObjectUI): # Validate media location try: if extra: - installer.extra_args = [extra] + installer.set_extra_args([extra]) if init: self._guest.os.init = init diff --git a/virtinst/installer.py b/virtinst/installer.py index 547336ee9..853dfc0f5 100644 --- a/virtinst/installer.py +++ b/virtinst/installer.py @@ -36,7 +36,7 @@ class Installer(object): :param install_kernel: Kernel to install off of :param install_initrd: Initrd to install off of :param install_kernel_args: Kernel args to use. This overwrites - whatever the installer might request, unlink extra_args which will + whatever the installer might request, unlike extra_args which will append arguments. """ def __init__(self, conn, cdrom=None, location=None, install_bootdev=None, @@ -45,13 +45,11 @@ class Installer(object): self.conn = conn self.livecd = False - self.extra_args = [] # Entry point for virt-manager 'Customize' wizard to change autostart self.autostart = False self._install_bootdev = install_bootdev - self._install_kernel_args = install_kernel_args self._install_cdrom_device_added = False self._unattended_install_cdrom_device = None self._tmpfiles = [] @@ -69,7 +67,7 @@ class Installer(object): install_kernel or install_initrd): self._treemedia = InstallerTreeMedia(self.conn, location, location_kernel, location_initrd, - install_kernel, install_initrd) + install_kernel, install_initrd, install_kernel_args) ################### @@ -164,20 +162,14 @@ class Installer(object): return kernel, initrd, kernel_args = self._treemedia_bootconfig - if kernel_args: - self.extra_args.append(kernel_args) - if kernel: guest.os.kernel = (self.conn.in_testsuite() and "/TESTSUITE_KERNEL_PATH" or kernel) if initrd: guest.os.initrd = (self.conn.in_testsuite() and "/TESTSUITE_INITRD_PATH" or initrd) - - if self._install_kernel_args: - guest.os.kernel_args = self._install_kernel_args - elif self.extra_args: - guest.os.kernel_args = " ".join(self.extra_args) + if kernel_args: + guest.os.kernel_args = kernel_args def _alter_bootconfig(self, guest): """ @@ -302,8 +294,16 @@ class Installer(object): return self._cdrom def set_initrd_injections(self, initrd_injections): - if self._treemedia: - self._treemedia.initrd_injections = initrd_injections + if not self._treemedia: + raise RuntimeError("Install method does not support " + "initrd injections.") + self._treemedia.set_initrd_injections(initrd_injections) + + def set_extra_args(self, extra_args): + if not self._treemedia: + raise RuntimeError("Kernel arguments are only supported with " + "location or kernel installs.") + self._treemedia.set_extra_args(extra_args) def set_install_defaults(self, guest): """ diff --git a/virtinst/installertreemedia.py b/virtinst/installertreemedia.py index 1e57a7aac..bf13fcfbc 100644 --- a/virtinst/installertreemedia.py +++ b/virtinst/installertreemedia.py @@ -100,15 +100,16 @@ class InstallerTreeMedia(object): return system_scratchdir # pragma: no cover def __init__(self, conn, location, location_kernel, location_initrd, - install_kernel, install_initrd): + install_kernel, install_initrd, install_kernel_args): self.conn = conn self.location = location self._location_kernel = location_kernel self._location_initrd = location_initrd self._install_kernel = install_kernel self._install_initrd = install_initrd - - self.initrd_injections = [] + self._install_kernel_args = install_kernel_args + self._initrd_injections = [] + self._extra_args = [] if location_kernel or location_initrd: if not location: @@ -211,7 +212,7 @@ class InstallerTreeMedia(object): self._tmpfiles.append(initrd) perform_initrd_injections(initrd, - self.initrd_injections, + self._initrd_injections, fetcher.scratchdir) system_scratchdir = InstallerTreeMedia.get_system_scratchdir(guest) @@ -228,28 +229,37 @@ class InstallerTreeMedia(object): ############## def _prepare_unattended_data(self, guest, script): - unattended_cmdline = script.generate_cmdline() - logging.debug("Generated unattended cmdline: %s", unattended_cmdline) - + if not script: + return expected_filename = script.get_expected_filename() scriptpath = script.write(guest) self._tmpfiles.append(scriptpath) - self.initrd_injections.append((scriptpath, expected_filename)) - return unattended_cmdline + self._initrd_injections.append((scriptpath, expected_filename)) + + def _prepare_kernel_args(self, cache, unattended_script): + install_args = None + if unattended_script: + install_args = unattended_script.generate_cmdline() + logging.debug("Generated unattended cmdline: %s", install_args) + elif self.is_network_url() and cache.kernel_url_arg: + install_args = "%s=%s" % (cache.kernel_url_arg, self.location) + + if install_args: + self._extra_args.append(install_args) + + if self._install_kernel_args: + return self._install_kernel_args + return " ".join(self._extra_args) def prepare(self, guest, meter, unattended_script): fetcher = self._get_fetcher(guest, meter) cache = self._get_cached_data(guest, fetcher) - kernel_args = "" - if unattended_script: - kernel_args = self._prepare_unattended_data( - guest, unattended_script) - elif self.is_network_url() and cache.kernel_url_arg: - kernel_args = "%s=%s" % (cache.kernel_url_arg, self.location) + self._prepare_unattended_data(guest, unattended_script) + kernel_args = self._prepare_kernel_args(cache, unattended_script) kernel, initrd = self._prepare_kernel_url(guest, cache, fetcher) - return kernel, initrd, kernel_args or "" + return kernel, initrd, kernel_args def cleanup(self, guest): ignore = guest @@ -264,6 +274,12 @@ class InstallerTreeMedia(object): self._tmpvols = [] self._tmpfiles = [] + def set_initrd_injections(self, initrd_injections): + self._initrd_injections = initrd_injections + + def set_extra_args(self, extra_args): + self._extra_args = extra_args + def cdrom_path(self): if self._media_type in [MEDIA_ISO]: return self.location