gistfile1.txt

commit c29354e8bf7e4579ac1c6865de1209e29db7309f
Author: Martin Kolman <[email protected]>
Date:   Tue May 10 15:53:48 2016 +0200

    Add support for alternative device specifications (#1200833)
    
    Make it possible to have multiple devices delimited by | in the device
    specification, for example:
    
    "sd*|hd*|vda"
    
    The sub-specs are processed individually from left to right
    and at least one match needs to happen for the | delimited
    spec to be considered successfully applied.
    
    Also make sure that callers of the deviceMatches() function
    really get disk devices when they expect them, not partitions
    or other devices that are not disks.
    
    Resolves: rhbz#1200833

diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py
index 406f4be..6f4ac6d 100644
--- a/pyanaconda/kickstart.py
+++ b/pyanaconda/kickstart.py
@@ -179,38 +179,69 @@ def getEscrowCertificate(escrowCerts, url):
 
     return escrowCerts[url]
 
-def deviceMatches(spec, devicetree=None):
+def deviceMatches(spec, devicetree=None, disks_only=False):
     """ Return names of block devices matching the provided specification.
 
         :param str spec: a device identifier (name, UUID=<uuid>, &c)
         :keyword devicetree: device tree to look up devices in (optional)
         :type devicetree: :class:`blivet.DeviceTree`
+        :param bool disks_only: if only disk devices maching the spec should be returned
         :returns: names of matching devices
         :rtype: list of str
 
-        parse methods will not have access to a devicetree, while execute
+        The spec can contain multiple "sub specs" delimited by a |, for example:
+
+        "sd*|hd*|vd*"
+
+        In such case we resolve the specs from left to right and return all
+        unique matches, for example:
+
+        ["sda", "sda1", "sda2", "sdb", "sdb1", "vdb"]
+
+        If disks_only is specified we only return
+        disk devices matching the spec. For the example above
+        the output with disks_only=True would be:
+
+        ["sda", "sdb", "vdb"]
+
+        Also note that parse methods will not have access to a devicetree, while execute
         methods will. The devicetree is superior in that it can resolve md
         array names and in that it reflects scheduled device removals, but for
         normal local disks udev.resolve_devspec should suffice.
     """
-    full_spec = spec
-    if not full_spec.startswith("/dev/"):
-        full_spec = os.path.normpath("/dev/" + full_spec)
 
-    # the regular case
-    matches = udev.resolve_glob(full_spec)
+    matches = []
+    # the device specifications might contain multiple "sub specs" separated by a |
+    # - the specs are processed from left to right
+    for single_spec in spec.split("|"):
+        full_spec = single_spec
+        if not full_spec.startswith("/dev/"):
+            full_spec = os.path.normpath("/dev/" + full_spec)
+
+        # the regular case
+        single_spec_matches = udev.resolve_glob(full_spec, disks_only=disks_only)
+        for match in single_spec_matches:
+            if match not in matches:
+                matches.append(match)
+
+        dev_name = None
+        # Use spec here instead of full_spec to preserve the spec and let the
+        # called code decide whether to treat the spec as a path instead of a name.
+        if devicetree is None:
+            dev_name = udev.resolve_devspec(single_spec)
+            if disks_only and not udev.is_disk(dev_name):
+                dev_name = None  # not a disk
 
-    # Use spec here instead of full_spec to preserve the spec and let the
-    # called code decide whether to treat the spec as a path instead of a name.
-    if devicetree is None:
-        dev = udev.resolve_devspec(spec)
-    else:
-        dev = getattr(devicetree.resolveDevice(spec), "name", None)
+        else:
+            device = devicetree.resolveDevice(single_spec)
+            dev_name = device.name
+            if disks_only and not device.isDisk:
+                dev_name = None  # not a disk
 
-    # udev.resolve_devspec returns None if there's no match, but we don't
-    # want that ending up in the list.
-    if dev and dev not in matches:
-        matches.append(dev)
+        # The dev_name variable can be None if the spec is not not found or is not valid,
+        # but we don't want that ending up in the list.
+        if dev_name and dev_name not in matches:
+            matches.append(dev_name)
 
     return matches
 
@@ -399,7 +430,7 @@ class Bootloader(commands.bootloader.RHEL7_Bootloader):
         diskSet = set(disk_names)
 
         for drive in self.driveorder[:]:
-            matches = set(deviceMatches(drive, devicetree=storage.devicetree))
+            matches = set(deviceMatches(drive, devicetree=storage.devicetree, disks_only=True))
             if matches.isdisjoint(diskSet):
                 log.warning("requested drive %s in boot drive order doesn't exist or cannot be used", drive)
                 self.driveorder.remove(drive)
@@ -407,8 +438,7 @@ class Bootloader(commands.bootloader.RHEL7_Bootloader):
         storage.bootloader.disk_order = self.driveorder
 
         if self.bootDrive:
-            matches = set(deviceMatches(self.bootDrive,
-                                        devicetree=storage.devicetree))
+            matches = set(deviceMatches(self.bootDrive, devicetree=storage.devicetree, disks_only=True))
             if len(matches) > 1:
                 raise KickstartValueError(formatErrorMsg(self.lineno,
                         msg=_("More than one match found for given boot drive \"%s\".") % self.bootDrive))
@@ -589,7 +619,7 @@ class ClearPart(commands.clearpart.F21_ClearPart):
         # disks available before the execute methods run.
         drives = []
         for spec in self.drives:
-            matched = deviceMatches(spec)
+            matched = deviceMatches(spec, disks_only=True)
             if matched:
                 drives.extend(matched)
             else:
@@ -602,7 +632,7 @@ class ClearPart(commands.clearpart.F21_ClearPart):
         # devices available before the execute methods run.
         devices = []
         for spec in self.devices:
-            matched = deviceMatches(spec)
+            matched = deviceMatches(spec, disks_only=True)
             if matched:
                 devices.extend(matched)
             else:
@@ -728,7 +758,7 @@ class IgnoreDisk(commands.ignoredisk.RHEL6_IgnoreDisk):
         # See comment in ClearPart.parse
         drives = []
         for spec in self.ignoredisk:
-            matched = deviceMatches(spec)
+            matched = deviceMatches(spec, disks_only=True)
             if matched:
                 drives.extend(matched)
             else:
@@ -739,7 +769,7 @@ class IgnoreDisk(commands.ignoredisk.RHEL6_IgnoreDisk):
 
         drives = []
         for spec in self.onlyuse:
-            matched = deviceMatches(spec)
+            matched = deviceMatches(spec, disks_only=True)
             if matched:
                 drives.extend(matched)
             else: