From a722eeac78d2db9e56703f5555bb48cd66e5b7a7 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Sun, 7 Sep 2014 11:57:04 -0400 Subject: [PATCH] virt-install: Enable libosinfo detection for --cdrom This streamlines virt-manager and virt-install implementations, requiring installer.distro_detect to be called if we want distro detection. As a side effect, we now get CDROM detection for free. --- man/virt-install.pod | 3 +- .../compare/virt-install-location-iso.xml | 6 +- tests/xmlconfig.py | 10 ++- virt-install | 64 +++++++++++++------ virtinst/cli.py | 29 --------- virtinst/distroinstaller.py | 28 +++----- virtinst/guest.py | 11 ++-- virtinst/installer.py | 7 +- 8 files changed, 73 insertions(+), 85 deletions(-) diff --git a/man/virt-install.pod b/man/virt-install.pod index e8d1d6d54..28f6962e9 100644 --- a/man/virt-install.pod +++ b/man/virt-install.pod @@ -409,7 +409,8 @@ by specifying virtio among other guest tweaks. By default, virt-install will attempt to auto detect this value from the install media (currently only supported for URL installs). Autodetection -can be disabled with the special value 'none'. +can be disabled with the special value 'none'. Autodetection can be +forced with the special value 'auto'. Use the command "osinfo-query os" to get the list of the accepted OS variants. diff --git a/tests/cli-test-xml/compare/virt-install-location-iso.xml b/tests/cli-test-xml/compare/virt-install-location-iso.xml index 21043b0c3..68a898895 100644 --- a/tests/cli-test-xml/compare/virt-install-location-iso.xml +++ b/tests/cli-test-xml/compare/virt-install-location-iso.xml @@ -46,9 +46,8 @@ - - + @@ -109,9 +108,8 @@ - - + diff --git a/tests/xmlconfig.py b/tests/xmlconfig.py index 914653d9d..96476c50c 100644 --- a/tests/xmlconfig.py +++ b/tests/xmlconfig.py @@ -86,13 +86,17 @@ class TestXMLConfig(unittest.TestCase): utils.test_create(guest.conn, actualXML) def _testInstall(self, guest, - instxml=None, bootxml=None, contxml=None): + instxml=None, bootxml=None, contxml=None, + detect_distro=False): instname = build_xmlfile(instxml) bootname = build_xmlfile(bootxml) contname = build_xmlfile(contxml) meter = None try: + if detect_distro: + guest.os_variant = guest.installer.detect_distro(guest) + guest.start_install(meter=meter) guest.domain.destroy() @@ -944,9 +948,9 @@ class TestXMLConfig(unittest.TestCase): g.add_device(utils.get_virtual_network()) g.add_device(VirtualAudio(g.conn)) g.add_device(VirtualVideoDevice(g.conn)) - g.os_autodetect = True - self._testInstall(g, "rhel6-kvm-stage1", "rhel6-kvm-stage2") + self._testInstall(g, "rhel6-kvm-stage1", "rhel6-kvm-stage2", + detect_distro=True) def testFullKVMWinxp(self): utils.set_conn(_plainkvm) diff --git a/virt-install b/virt-install index fdd8d9d12..2f8236849 100755 --- a/virt-install +++ b/virt-install @@ -163,6 +163,23 @@ def convert_old_disks(options): options.sparse = True +def convert_old_os_options(options): + distro_variant = options.distro_variant + distro_type = options.distro_type + if not distro_type and not distro_variant: + # Default to distro autodetection + options.distro_variant = "auto" + return + + distro_variant = distro_variant and str(distro_variant).lower() or None + distro_type = distro_type and str(distro_type).lower() or None + distkey = distro_variant or distro_type + if not distkey or distkey == "none": + options.distro_variant = "none" + else: + options.distro_variant = distkey + + ######################## # Virt type validation # ######################## @@ -220,27 +237,25 @@ def get_guest(conn, options): # Install media setup/validation # ################################## -def get_install_media(guest, location, cdpath): - manual_cdrom = cdrom_specified(guest) - cdinstall = bool(not location and (cdpath or cdrom_specified(guest))) - - if not (location or cdpath or manual_cdrom): - return - +def set_install_media(guest, location, cdpath, distro_variant): try: - validate_install_media(guest, location, cdpath, cdinstall) + cdinstall = bool(not location and (cdpath or cdrom_specified(guest))) + + if cdinstall or cdpath: + guest.installer.cdrom = True + if location or cdpath: + guest.installer.location = (location or cdpath) + + guest.installer.check_location(guest) + + if distro_variant == "auto": + guest.os_variant = guest.installer.detect_distro(guest) + elif distro_variant != "none": + guest.os_variant = distro_variant except ValueError, e: fail(_("Error validating install location: %s" % str(e))) -def validate_install_media(guest, location, cdpath, cdinstall=False): - if cdinstall or cdpath: - guest.installer.cdrom = True - if location or cdpath: - guest.installer.location = (location or cdpath) - guest.installer.check_location(guest) - - ############################# # General option validation # ############################# @@ -448,11 +463,11 @@ def build_guest_instance(conn, options, parsermap): cli.convert_old_features(options) cli.convert_old_cpuset(options) convert_old_init(options) + convert_old_os_options(options) # non-xml install options guest.installer.extraargs = options.extra_args guest.installer.initrd_injections = options.initrd_inject - cli.set_os_variant(guest, options.distro_type, options.distro_variant) guest.autostart = options.autostart if options.name: @@ -472,9 +487,10 @@ def build_guest_instance(conn, options, parsermap): for disk in guest.get_devices("disk"): cli.validate_disk(disk) - guest.add_default_devices() + set_install_media(guest, options.location, options.cdrom, + options.distro_variant) - get_install_media(guest, options.location, options.cdrom) + guest.add_default_devices() # Various little validations about option collisions. Need to do # this after setting guest.installer at least @@ -768,7 +784,15 @@ def parse_args(): "booted from --location")) insg.add_argument("--initrd-inject", action="append", help=_("Add given file to root of initrd from --location")) - cli.add_distro_options(insg) + + # Way back when, we required specifying both --os-type and --os-variant + # Nowadays the distinction is pointless, so hide the less useful + # --os-type option. + insg.add_argument("--os-type", dest="distro_type", help=argparse.SUPPRESS) + insg.add_argument("--os-variant", dest="distro_variant", + help=_("The OS variant being installed guests, " + "e.g. 'fedora18', 'rhel6', 'winxp', etc.")) + cli.add_boot_options(insg) insg.add_argument("--init", help=argparse.SUPPRESS) diff --git a/virtinst/cli.py b/virtinst/cli.py index 74a77ce32..0037f3475 100644 --- a/virtinst/cli.py +++ b/virtinst/cli.py @@ -530,24 +530,6 @@ def convert_old_features(options): options.features = opts or None -def set_os_variant(obj, distro_type, distro_variant): - # This is used for both Guest and virtconv VM, so be careful - if (not distro_type and - not distro_variant and - hasattr(obj, "os_autodetect")): - # Default to distro autodetection - obj.os_autodetect = True - return - - distro_variant = distro_variant and str(distro_variant).lower() or None - distro_type = distro_type and str(distro_type).lower() or None - distkey = distro_variant or distro_type - if not distkey or distkey == "none": - return - - obj.os_variant = distkey - - ########################### # Common CLI option/group # ########################### @@ -759,17 +741,6 @@ def add_fs_option(devg): "--filesystem template_name,/,type=template")) -def add_distro_options(g): - # Way back when, we required specifying both --os-type and --os-variant - # Nowadays the distinction is pointless, so hide the less useful - # --os-type option. - g.add_argument("--os-type", dest="distro_type", - help=argparse.SUPPRESS) - g.add_argument("--os-variant", dest="distro_variant", - help=_("The OS variant being installed guests, " - "e.g. 'fedora18', 'rhel6', 'winxp', etc.")) - - def add_old_feature_options(optg): optg.add_argument("--noapic", action="store_true", default=False, help=argparse.SUPPRESS) diff --git a/virtinst/distroinstaller.py b/virtinst/distroinstaller.py index 2d1cb73c2..9b3c56248 100644 --- a/virtinst/distroinstaller.py +++ b/virtinst/distroinstaller.py @@ -347,13 +347,6 @@ class DistroInstaller(Installer): def _prepare_kernel_url(self, guest, fetcher): store = urlfetcher.getDistroStore(guest, fetcher) kernel, initrd, args = store.acquireKernel(guest) - os_variant = store.get_osdict_info() - - if guest.os_autodetect: - if os_variant: - logging.debug("Auto detected OS variant as: %s", os_variant) - guest.os_variant = os_variant - self._tmpfiles.append(kernel) if initrd: self._tmpfiles.append(initrd) @@ -473,19 +466,14 @@ class DistroInstaller(Installer): return True def detect_distro(self, guest): + distro = None try: - if not _is_url(self.conn, self.location): - name = osdict.lookup_os_by_media(self.location) - if name: - logging.debug("installer.detect_distro returned=%s", name) - return name + if _is_url(self.conn, self.location): + distro = urlfetcher.detectMediaDistro(guest, self.location) + else: + distro = osdict.lookup_os_by_media(self.location) except: - logging.debug("libosinfo detect failed", exc_info=True) + logging.debug("Error attempting to detect distro.", exc_info=True) - try: - ret = urlfetcher.detectMediaDistro(guest, self.location) - logging.debug("installer.detect_distro returned=%s", ret) - return ret - except: - logging.exception("Error attempting to detect distro.") - return None + logging.debug("installer.detect_distro returned=%s", distro) + return distro diff --git a/virtinst/guest.py b/virtinst/guest.py index 1c3bbcd98..36c0625ac 100644 --- a/virtinst/guest.py +++ b/virtinst/guest.py @@ -104,7 +104,6 @@ class Guest(XMLBuilder): self.autostart = False self.replace = False - self.os_autodetect = False # Allow virt-manager to override the default graphics type self.default_graphics_type = cliconfig.default_graphics @@ -207,11 +206,13 @@ class Guest(XMLBuilder): def _get_os_variant(self): return self._os_variant def _set_os_variant(self, val): + if val: + val = val.lower() + if osdict.lookup_os(val) is None: + raise ValueError( + _("Distro '%s' does not exist in our dictionary") % val) + logging.debug("Setting Guest.os_variant to '%s'", val) - val = val.lower() - if osdict.lookup_os(val) is None: - raise ValueError(_("Distro '%s' does not exist in our dictionary") - % val) self._os_variant = val os_variant = property(_get_os_variant, _set_os_variant) diff --git a/virtinst/installer.py b/virtinst/installer.py index 1ae580be0..ac8f899e7 100644 --- a/virtinst/installer.py +++ b/virtinst/installer.py @@ -215,12 +215,13 @@ class Installer(object): """ Attempt to detect the distro for the Installer's 'location'. If an error is encountered in the detection process (or if detection - is not relevant for the Installer type), (None, None) is returned + is not relevant for the Installer type), None is returned. - @returns: (distro type, distro variant) tuple + @returns: distro variant string, or None """ ignore = guest - return (None, None) + logging.debug("distro detection not available for this installer.") + return None class ContainerInstaller(Installer):