[master 1/2] Try to make EDD support actually match what the kernel does.

vathpela installerbot-noreply at redhat.com
Mon Oct 5 15:56:48 UTC 2015


From: Peter Jones <pjones at redhat.com>

Basically the parser for ATA will kind of work with SCSI if lun==0, but
it won't work with SATA or ATA at all, and the parser we had for SCSI
will only work with virtio.

This patch separates those all out by which kernel driver is actually
providing the sysfs devices, and tries to match the EDD fields up with
how they're actually provided by the kernel.

Signed-off-by: Peter Jones <pjones at redhat.com>
---
 blivet/devicelibs/edd.py | 201 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 149 insertions(+), 52 deletions(-)

diff --git a/blivet/devicelibs/edd.py b/blivet/devicelibs/edd.py
index 71fb271..55b005e 100644
--- a/blivet/devicelibs/edd.py
+++ b/blivet/devicelibs/edd.py
@@ -32,9 +32,10 @@
 
 log = logging.getLogger("blivet")
 
-re_host_bus = re.compile(r'^PCI\s*(\S*)\s*channel: (\S*)\s*$')
+re_host_bus = re.compile(r'^PCI\S*\s*(\S*)\s*channel: (\S*)\s*$')
 re_interface_scsi = re.compile(r'^SCSI\s*id: (\S*)\s*lun: (\S*)\s*$')
 re_interface_ata = re.compile(r'^ATA\s*device: (\S*)\s*$')
+re_interface_sata = re.compile(r'^SATA\s*device: (\S*)\s*$')
 
 class EddEntry(object):
     """ This object merely collects what the /sys/firmware/edd/* entries can
@@ -45,53 +46,62 @@ def __init__(self, sysfspath):
 
         self.ata_device = None
         self.channel = None
-        self.mbr_signature = None
+        self.mbr_sig = None
         self.pci_dev = None
         self.scsi_id = None
         self.scsi_lun = None
         self.sectors = None
+        self.sysfslink = ""
+        self.sysfspath = sysfspath
+        self.version = util.get_sysfs_attr(sysfspath, "version")
 
-        self.load(sysfspath)
+        self.load()
 
     def __str__(self):
         return \
+            "\tpath: %(sysfspath)s version: %(version)s\n" \
+            "\tsysfs pci path: %(sysfslink)s\n" \
             "\ttype: %(type)s, ata_device: %(ata_device)s\n" \
-            "\tchannel: %(channel)s, mbr_signature: %(mbr_signature)s\n" \
-            "\tpci_dev: %(pci_dev)s, scsi_id: %(scsi_id)s\n" \
-            "\tscsi_lun: %(scsi_lun)s, sectors: %(sectors)s" % self.__dict__
+            "\tchannel: %(channel)s, mbr_signature: %(mbr_sig)s\n" \
+            "\tpci_dev: %(pci_dev)s, scsi_id: %(scsi_id)s," \
+            " scsi_lun: %(scsi_lun)s, sectors: %(sectors)s" % self.__dict__
 
-    def _read_file(self, filename):
-        contents = None
-        if os.path.exists(filename):
-            with open(filename) as f:
-                contents = f.read().rstrip()
-        return contents
-
-    def load(self, sysfspath):
-        interface = self._read_file(os.path.join(sysfspath, "interface"))
+    def load(self):
+        interface = util.get_sysfs_attr(self.sysfspath, "interface")
         if interface:
             self.type = interface.split()[0]
-            if self.type == "SCSI":
+            unsupported = ("ATAPI", "USB", "1394", "FIBRE", "I2O", "RAID")
+            if self.type in unsupported:
+                log.warning("edd: interface type %s is not implemented (%s)",
+                    self.type, self.sysfspath)
+                log.warning("edd: interface details: %s", interface)
+            elif self.type == "ATA":
+                match = re_interface_ata.match(interface)
+                self.ata_device = int(match.group(1))
+            elif self.type == "SCSI":
                 match = re_interface_scsi.match(interface)
                 self.scsi_id = int(match.group(1))
                 self.scsi_lun = int(match.group(2))
-            elif self.type == "ATA":
-                match = re_interface_ata.match(interface)
+            elif self.type == "SATA":
+                match = re_interface_sata.match(interface)
                 self.ata_device = int(match.group(1))
+            else:
+                log.warning("edd: can not match interface for %s: %s",
+                    self.sysfspath, interface)
 
-        self.mbr_signature = self._read_file(
-            os.path.join(sysfspath, "mbr_signature"))
-        sectors = self._read_file(os.path.join(sysfspath, "sectors"))
+        self.mbr_sig = util.get_sysfs_attr(self.sysfspath, "mbr_signature")
+        sectors = util.get_sysfs_attr(self.sysfspath, "sectors")
         if sectors:
             self.sectors = int(sectors)
-        hbus = self._read_file(os.path.join(sysfspath, "host_bus"))
+        hbus = util.get_sysfs_attr(self.sysfspath, "host_bus")
         if hbus:
             match = re_host_bus.match(hbus)
             if match:
                 self.pci_dev = match.group(1)
                 self.channel = int(match.group(2))
             else:
-                log.warning("edd: can not match host_bus: %s", hbus)
+                log.warning("edd: can not match host_bus for %s: %s",
+                    self.sysfspath, hbus)
 
 class EddMatcher(object):
     """ This object tries to match given entry to a disk device name.
@@ -101,32 +111,116 @@ class EddMatcher(object):
     def __init__(self, edd_entry):
         self.edd = edd_entry
 
-    def devname_from_pci_dev(self):
+    def _read_file(self, filename):
+        contents = None
+        if os.path.exists(filename):
+            with open(filename) as f:
+                contents = f.read().rstrip()
+        return contents
+
+    def devname_from_ata_pci_dev(self):
+        pattern = '/sys/block/*'
+        for path in glob.iglob(pattern):
+            link = os.readlink(path)
+            # just add /sys/block/ at the beginning so it's always valid
+            # paths in the filesystem...
+            components = ['/sys', 'block'] + link.split('/')
+            if len(components) != 10:
+                continue
+            # ATA and SATA paths look like:
+            # ../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda
+            # where literally the only pieces of data here are
+            # "pci0000:00:1f.2", "ata1", and "sda".
+            #
+            # EDD's "channel" may or may not mean anything at all on SATA
+            # (there's no spec...), and it's not clear if SATA Port
+            # Multipliers are represented in any way, or if they happen to
+            # be the channel (but in a malformed way).  I'm just assuming
+            # channel = 255 really means pmp_id == 65535, aka there isn't
+            # one.  If I'm wrong, then there's just no way to support SATA
+            # Port Multipliers, in which case the value will still say
+            # 255, and the device just won't be matched correctly ever.
+
+            if components[5] != '0000:%s' % (self.edd.pci_dev,):
+                continue
+            if not components[6].startswith('ata'):
+                continue
+            ata_port = components[6]
+            ata_port_idx = int(components[6][3:])
+
+            fn = components[0:7] + ['ata_port', ata_port, 'port_no']
+            port_no = int(self._read_file(os.path.join(*fn)))
+
+            # "port_no" is the kernel's ata_port->print_id, which is awesomely
+            # ata_port->id + 1, where ata_port->id is edd's ata_device
+            if port_no != self.edd.ata_device+1:
+                continue
+
+            fn = components[0:7] + ['link%d' % (ata_port_idx,),]
+            if self.edd.channel == 255:
+                # find all the things that are linkN/devN.X
+                # but filter out devN.M.X
+                exp = fn + [r'dev%d\.\d+$' % (ata_port_idx,)]
+                pattern = fn + ['dev%d.*' % (ata_port_idx,)]
+            else:
+                # find all the things that are linkN/devN.M.X
+                # but filter out devN.X
+                exp = fn + [r'dev%d\.(\d+)\.\d+$' % (ata_port_idx,)]
+                pattern = fn + ['dev%d.*.*' % (ata_port_idx,)]
+
+            exp = '^' + os.path.join(*exp)
+            exp = re.compile(exp)
+            pattern = os.path.join(*pattern)
+            for atapath in glob.iglob(pattern):
+                match = exp.match(atapath)
+                if match is None:
+                    continue
+                self.edd.sysfslink = link
+                if self.edd.channel == 255:
+                    return path.split('/')[-1]
+                elif self.edd.channel == int(match.group(1)):
+                    return path.split('/')[-1]
+        return None
+
+    def devname_from_scsi_pci_dev(self):
+        name = None
+        path = "/sys/devices/pci0000:00/0000:%(pci_dev)s/host%(chan)d/"\
+            "target%(chan)d:0:%(dev)d/%(chan)d:0:%(dev)d:%(lun)d/block" % {
+            'pci_dev' : self.edd.pci_dev,
+            'chan' : self.edd.channel,
+            'dev' : self.edd.scsi_id,
+            'lun' : self.edd.scsi_lun,
+            }
+        if os.path.isdir(path):
+            block_entries = os.listdir(path)
+            if len(block_entries) == 1:
+                name = block_entries[0]
+        else:
+            log.warning("edd: directory does not exist: %s", path)
+        return name
+
+    def devname_from_virt_pci_dev(self):
         name = None
-        if self.edd.type == "ATA" and \
+        pattern = "/sys/devices/pci0000:00/0000:%(pci_dev)s/virtio*/block" % \
+            {'pci_dev' : self.edd.pci_dev}
+        matching_paths = glob.glob(pattern)
+        if len(matching_paths) != 1 or not os.path.exists(matching_paths[0]):
+            return None
+        block_entries = os.listdir(matching_paths[0])
+        if len(block_entries) == 1:
+            name = block_entries[0]
+        return name
+
+    def devname_from_pci_dev(self):
+        name = self.devname_from_virt_pci_dev()
+        if not name is None:
+            return name
+        if self.edd.type in ("ATA", "SATA") and \
                 self.edd.channel is not None and \
                 self.edd.ata_device is not None:
-            path = "/sys/devices/pci0000:00/0000:%(pci_dev)s/host%(chan)d/"\
-                "target%(chan)d:0:%(dev)d/%(chan)d:0:%(dev)d:0/block" % {
-                'pci_dev' : self.edd.pci_dev,
-                'chan' : self.edd.channel,
-                'dev' : self.edd.ata_device
-                }
-            if os.path.isdir(path):
-                block_entries = os.listdir(path)
-                if len(block_entries) == 1:
-                    name = block_entries[0]
-            else:
-                log.warning("edd: directory does not exist: %s", path)
+            name = self.devname_from_ata_pci_dev()
         elif self.edd.type == "SCSI":
-            pattern = "/sys/devices/pci0000:00/0000:%(pci_dev)s/virtio*/block" % \
-                {'pci_dev' : self.edd.pci_dev}
-            matching_paths = glob.glob(pattern)
-            if len(matching_paths) != 1 or not os.path.exists(matching_paths[0]):
-                return None
-            block_entries = os.listdir(matching_paths[0])
-            if len(block_entries) == 1:
-                name = block_entries[0]
+            name = self.devname_from_scsi_pci_dev()
         return name
 
     def match_via_mbrsigs(self, mbr_dict):
@@ -135,8 +229,8 @@ def match_via_mbrsigs(self, mbr_dict):
             This will obviously fail for a fresh drive/image, but in extreme
             cases can also show false positives for randomly matching data.
         """
-        for (name, mbr_signature) in mbr_dict.items():
-            if mbr_signature == self.edd.mbr_signature:
+        for (name, mbr_sig) in mbr_dict.items():
+            if mbr_sig == self.edd.mbr_sig:
                 return name
         return None
 
@@ -145,8 +239,11 @@ def biosdev_to_edd_dir(biosdev):
 
 def collect_edd_data():
     edd_data_dict = {}
-    # the hard drive numbering starts at 0x80 (128 decimal):
-    for biosdev in range(0x80, 0x80+16):
+    # BIOS dev numbers are 0-127 , with 0x80 unset for floppies and set
+    # for hard drives.  In practice, the kernel has had EDDMAXNR (the limit
+    # on how many EDD structures it will load) set to 6 since before the
+    # import into git, so that's good enough.
+    for biosdev in range(0x80, 0x86):
         sysfspath = biosdev_to_edd_dir(biosdev)
         if not os.path.exists(sysfspath):
             break
@@ -206,13 +303,13 @@ def get_edd_dict(devices):
     mbr_dict = collect_mbrs(devices)
     edd_entries_dict = collect_edd_data()
     for (edd_number, edd_entry) in edd_entries_dict.items():
-        log.debug("edd: data extracted from 0x%x:\n%s", edd_number, edd_entry)
         matcher = EddMatcher(edd_entry)
         # first try to match through the pci dev etc.
         name = matcher.devname_from_pci_dev()
-        # next try to compare mbr signatures
+        log.debug("edd: data extracted from 0x%x:\n%s", edd_number, edd_entry)
         if name:
-            log.debug("edd: matched 0x%x to %s using pci_dev", edd_number, name)
+            log.info("edd: matched 0x%x to %s using pci_dev", edd_number, name)
+        # next try to compare mbr signatures
         else:
             name = matcher.match_via_mbrsigs(mbr_dict)
             if name:


-- 
To view this commit on github, visit https://github.com/rhinstaller/blivet/commit/4fb5487651012ae215cc4ecea7ca99e5dc553fa2


More information about the anaconda-patches mailing list