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

@@ -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:
-        if extra_args:
+        installer.set_extra_args(extra_args)
-            installer.extra_args = extra_args
+    if options.initrd_inject:
-        if options.initrd_inject:
+        installer.set_initrd_injections(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

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
 

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 <cmdline> 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 kernel_args:
-        if self._install_kernel_args:
+            guest.os.kernel_args = kernel_args
-            guest.os.kernel_args = self._install_kernel_args
-        elif self.extra_args:
-            guest.os.kernel_args = " ".join(self.extra_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:
+        if not self._treemedia:
-            self._treemedia.initrd_injections = initrd_injections
+            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):
         """

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._install_kernel_args = install_kernel_args
-        self.initrd_injections = []
+        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()
+        if not script:
-        logging.debug("Generated unattended cmdline: %s", unattended_cmdline)
+            return
-
         expected_filename = script.get_expected_filename()
         scriptpath = script.write(guest)
         self._tmpfiles.append(scriptpath)
-        self.initrd_injections.append((scriptpath, expected_filename))
+        self._initrd_injections.append((scriptpath, expected_filename))
-        return unattended_cmdline
+
+    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 = ""
+        self._prepare_unattended_data(guest, unattended_script)
-        if unattended_script:
+        kernel_args = self._prepare_kernel_args(cache, 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)
 
         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