[master 16/22] edd: Try to detect multiple ATA/SATA matches and log them.

vathpela installerbot-noreply at redhat.com
Wed Oct 28 15:55:16 UTC 2015


From: Peter Jones <pjones at redhat.com>

This is probably always indicative of a bug in firmware or our matcher,
but that's still worth the logging.

Signed-off-by: Peter Jones <pjones at redhat.com>
---
 blivet/devicelibs/edd.py          |  74 ++++++++++++++---
 tests/devicelibs_test/edd_test.py | 169 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 232 insertions(+), 11 deletions(-)

diff --git a/blivet/devicelibs/edd.py b/blivet/devicelibs/edd.py
index 3d39e59..1e4a16b 100644
--- a/blivet/devicelibs/edd.py
+++ b/blivet/devicelibs/edd.py
@@ -354,6 +354,7 @@ def __init__(self, edd_entry, root=None):
 
     def devname_from_ata_pci_dev(self):
         pattern = util.Path('/sys/block/*', root=self.root)
+        answers = []
         for path in pattern.glob():
             emptyslash = util.Path("/", root=self.root)
             path = util.Path(path, root=self.root)
@@ -435,13 +436,28 @@ def devname_from_ata_pci_dev(self):
                         channel = int(match.group(1))
                         if (self.edd.channel == 255 and channel == 0) or \
                                 (self.edd.channel == channel):
-                            self.edd.sysfslink = util.Path(link, root=self.root)
-                            return path.split('/')[-1]
+                            answers.append({
+                                    'link': util.Path(link, root=self.root),
+                                    'path': path.split('/')[-1]})
                     else:
                         pmp = int(match.group(1))
                         if self.edd.ata_pmp == pmp:
-                            self.edd.sysfslink = util.Path(link, root=self.root)
-                            return path.split('/')[-1]
+                            answers.append({
+                                    'link': util.Path(link, root=self.root),
+                                    'path': path.split('/')[-1]})
+
+        if len(answers) > 1:
+            log.error("edd: Found too many ATA devices for EDD device 0x%x: %s",
+                      self.edd.bios_device_number,
+                      [a['link'] for a in answers])
+        if len(answers) > 0:
+            self.edd.sysfslink = answers[0]['link']
+            return answers[0]['path']
+        else:
+            log.warning(
+                "edd: Could not find ATA device for pci dev %s channel %s ata %d pmp %d",
+                self.edd.pci_dev, self.edd.channel,
+                self.edd.ata_device, self.edd.ata_pmp)
 
         return None
 
@@ -458,12 +474,25 @@ def devname_from_virtio_scsi_pci_dev(self):
             'lun' : self.edd.scsi_lun,
             }
         pattern = util.Path(tmpl % args, self.root + "/sys/block/")
+        answers = []
         for mp in pattern.glob():
             # Normal VirtIO devices just have the block link right there...
             block_entries = os.listdir(mp.ondisk)
             for be in block_entries:
-                self.edd.sysfslink = mp + be
-                return be
+                link = mp + be
+                answers.append({'link':link, 'path':be})
+
+        if len(answers) > 1:
+            log.error("Found too many VirtIO SCSI devices for EDD device 0x%x: %s",
+                      self.edd.bios_device_number,
+                      [a['link'] for a in answers])
+        if len(answers) > 0:
+            self.edd.sysfslink = answers[0]['link']
+            return answers[0]['path']
+        else:
+            log.info("edd: Could not find VirtIO SCSI device for pci dev %s "\
+                        "channel %s scsi id %s lun %s", self.edd.pci_dev,
+                        self.edd.channel, self.edd.scsi_id, self.edd.scsi_lun)
 
     def devname_from_scsi_pci_dev(self):
         tmpl = "../devices/pci0000:00/0000:%(pci_dev)s/" \
@@ -481,9 +510,20 @@ def devname_from_scsi_pci_dev(self):
             # Normal VirtIO devices just have the block link right there...
             block_entries = os.listdir(mp.ondisk)
             for be in block_entries:
-                self.edd.sysfslink = mp + be
-                return be
-
+                link = mp + be
+                answers.append({'link':link, 'path':be})
+
+        if len(answers) > 1:
+            log.error("Found too many SCSI devices for EDD device 0x%x: %s",
+                      self.edd.bios_device_number,
+                      [a['link'] for a in answers])
+        if len(answers) > 0:
+            self.edd.sysfslink = answers[0]['link']
+            return answers[0]['path']
+        else:
+            log.warning("edd: Could not find SCSI device for pci dev %s "\
+                        "channel %s scsi id %s lun %s", self.edd.pci_dev,
+                        self.edd.channel, self.edd.scsi_id, self.edd.scsi_lun)
         return None
 
     def devname_from_virt_pci_dev(self):
@@ -494,8 +534,20 @@ def devname_from_virt_pci_dev(self):
             # Normal VirtIO devices just have the block link right there...
             block_entries = os.listdir(mp.ondisk)
             for be in block_entries:
-                self.edd.sysfslink = mp + be
-                return be
+                link = mp + be
+                answers.append({'link':link, 'path':be})
+
+        if len(answers) > 1:
+            log.error("Found too many VirtIO devices for EDD device 0x%x: %s",
+                      self.edd.bios_device_number,
+                      [a['link'] for a in answers])
+        if len(answers) > 0:
+            self.edd.sysfslink = answers[0]['link']
+            return answers[0]['path']
+        else:
+            log.info(
+                "edd: Could not find Virtio device for pci dev %s channel %s",
+                self.edd.pci_dev, self.edd.channel)
 
         return None
 
diff --git a/tests/devicelibs_test/edd_test.py b/tests/devicelibs_test/edd_test.py
index 29e6d1a..2bc756a 100644
--- a/tests/devicelibs_test/edd_test.py
+++ b/tests/devicelibs_test/edd_test.py
@@ -231,6 +231,8 @@ def test_get_edd_dict_sata_usb(self):
             ("edd: collected mbr signatures: %s",{'sdb': '0x96a20d28'}),
             ("edd: matched 0x%x to %s using PCI dev", 0x80, "sda"),
             ("edd: matched 0x%x to %s using MBR sig", 0x81, "sdb"),
+            ('edd: Could not find Virtio device for pci dev %s channel %s', '00:1f.2', 255),
+            ('edd: Could not find Virtio device for pci dev %s channel %s', 'ff:ff.255', 255),
             ]
         warnings = [
             ("edd: interface type %s is not implemented (%s)", "USB",
@@ -238,3 +240,170 @@ def test_get_edd_dict_sata_usb(self):
             ("edd: interface details: %s", "USB     \tserial_number: 30302e31"),
             ]
         self.checkLogs(debugs=debugs, infos=infos, warnings=warnings)
+
+    def test_collect_edd_data_absurd_virt(self):
+        self._edd_logger.debug("starting test %s", self._testMethodName)
+        edd.testdata_log.debug("starting test %s", self._testMethodName)
+        # siiiigh - this is actually the data out of sysfs on a virt I have
+        # created.  Apparently even qemu claims 3.0 sometimes and gives us
+        # bad data.
+        fakeedd = {
+            0x80: FakeEddEntry(version="0x30", mbr_sig="0x86531966",
+                               sectors=10485760, host_bus="PCI", type="SCSI",
+                               pci_dev="00:07.0", channel=0, scsi_id=0,
+                               scsi_lun=0, interface="SCSI    \tid: 0  lun: 0",
+                               sysfspath="/sys/firmware/edd/int13_dev80/"),
+            0x81: FakeEddEntry(version="0x30", mbr_sig="0x7dfff0db",
+                               sectors=209716, host_bus="PCI", type="ATA",
+                               pci_dev="00:01.1", channel=0,
+                               interface="ATA     \tdevice: 1", ata_device=1,
+                               sysfspath="/sys/firmware/edd/int13_dev81/"),
+            0x82: FakeEddEntry(version="0x30", mbr_sig="0xe3bf124b",
+                               sectors=419432,
+                               sysfspath="/sys/firmware/edd/int13_dev82/"),
+            0x83: FakeEddEntry(version="0x30", mbr_sig="0xfa0a111d",
+                               sectors=629146,
+                               sysfspath="/sys/firmware/edd/int13_dev83/"),
+            0x84: FakeEddEntry(version="0x30", mbr_sig="0x63f1d7d8",
+                               sectors=838862, host_bus="PCI", type="SCSI",
+                               pci_dev="00:0b.0", channel=0, scsi_id=0,
+                               scsi_lun=0, interface="SCSI    \tid: 0  lun: 0",
+                               sysfspath="/sys/firmware/edd/int13_dev84/"),
+            0x85: FakeEddEntry(version="0x30", mbr_sig="0xee331b19",
+                               sectors=1258292,
+                               sysfspath="/sys/firmware/edd/int13_dev85/"),
+            }
+        edd_dict = edd.collect_edd_data(root=self.root('absurd_virt'))
+        self.debug('edd_dict: %s', edd_dict)
+        self.assertEqual(len(edd_dict), 6)
+        self.assertEqual(fakeedd[0x80], edd_dict[0x80])
+        self.assertEqual(fakeedd[0x81], edd_dict[0x81])
+        self.assertEqual(fakeedd[0x82], edd_dict[0x82])
+        self.assertEqual(fakeedd[0x83], edd_dict[0x83])
+        self.assertEqual(fakeedd[0x84], edd_dict[0x84])
+        self.assertEqual(fakeedd[0x85], edd_dict[0x85])
+        debugs = [
+            ("edd: found device 0x%x at %s", 0x80,
+                    '/sys/firmware/edd/int13_dev80/'),
+            ("edd: found device 0x%x at %s", 0x81,
+                    '/sys/firmware/edd/int13_dev81/'),
+            ("edd: found device 0x%x at %s", 0x82,
+                    '/sys/firmware/edd/int13_dev82/'),
+            ("edd: found device 0x%x at %s", 0x83,
+                    '/sys/firmware/edd/int13_dev83/'),
+            ("edd: found device 0x%x at %s", 0x84,
+                    '/sys/firmware/edd/int13_dev84/'),
+            ("edd: found device 0x%x at %s", 0x85,
+                    '/sys/firmware/edd/int13_dev85/'),
+            ]
+        self.checkLogs(debugs=debugs)
+
+    def test_get_edd_dict_absurd_virt(self):
+        self._edd_logger.debug("starting test %s", self._testMethodName)
+        edd.testdata_log.debug("starting test %s", self._testMethodName)
+        # siiiigh - this is actually the data out of sysfs on a virt I have
+        # created.  Apparently even qemu claims 3.0 sometimes and gives us
+        # bad data.
+        fakeedd = {
+            0x80: FakeEddEntry(version="0x30", mbr_sig="0x86531966",
+                               sectors=10485760, host_bus="PCI", type="SCSI",
+                               pci_dev="00:07.0", channel=0, scsi_id=0,
+                               scsi_lun=0, interface="SCSI    \tid: 0  lun: 0",
+                               sysfspath="/sys/firmware/edd/int13_dev80/",
+                               sysfslink="../devices/pci0000:00"\
+                                         "/0000:00:07.0/virtio1/block/vda"),
+            0x81: FakeEddEntry(version="0x30", mbr_sig="0x7dfff0db",
+                               sectors=209716, host_bus="PCI", type="ATA",
+                               pci_dev="00:01.1", channel=0,
+                               interface="ATA     \tdevice: 1", ata_device=1,
+                               sysfslink="../devices/pci0000:00/0000:00:01.1/"\
+                                         "ata7/host6/target6:0:1/6:0:1:0/"\
+                                         "block/sdb",
+                               sysfspath="/sys/firmware/edd/int13_dev81/"),
+            0x82: FakeEddEntry(version="0x30", mbr_sig="0xe3bf124b",
+                               sectors=419432,
+                               sysfslink="../devices/pci0000:00/0000:00:03.0/"\
+                                         "ata1/host0/target0:0:0/0:0:0:0/"\
+                                         "block/sda",
+                               sysfspath="/sys/firmware/edd/int13_dev82/"),
+            0x83: FakeEddEntry(version="0x30", mbr_sig="0xfa0a111d",
+                               sectors=629146,
+                               sysfslink="../devices/pci0000:00/0000:00:0a.0/"\
+                                         "host10/target10:0:1/10:0:1:0/"\
+                                         "block/sde",
+                               sysfspath="/sys/firmware/edd/int13_dev83/"),
+            0x84: FakeEddEntry(version="0x30", mbr_sig="0x63f1d7d8",
+                               sectors=838862, host_bus="PCI", type="SCSI",
+                               pci_dev="00:0b.0", channel=0, scsi_id=0,
+                               scsi_lun=0, interface="SCSI    \tid: 0  lun: 0",
+                               sysfslink="../devices/pci0000:00/0000:00:0b.0/"\
+                                         "virtio3/host8/target8:0:0/8:0:0:0/"\
+                                         "block/sdc",
+                               sysfspath="/sys/firmware/edd/int13_dev84/"),
+            0x85: FakeEddEntry(version="0x30", mbr_sig="0xee331b19",
+                               sectors=1258292,
+                               sysfslink="../devices/pci0000:00/0000:00:06.7/"\
+                                         "usb1/1-1/1-1:1.0/host9/target9:0:0/"\
+                                         "9:0:0:0/block/sdd",
+                               sysfspath="/sys/firmware/edd/int13_dev85/"),
+            }
+        devices=(FakeDevice("sda"),
+                 FakeDevice("sdb"),
+                 FakeDevice("sdc"),
+                 FakeDevice("sdd"),
+                 FakeDevice("sde"),
+                 FakeDevice("vda"),
+                 )
+
+        edd_dict = edd.get_edd_dict(devices, root=self.root("absurd_virt"))
+        self.debug('edd_dict: %s', edd_dict)
+        self.assertEqual(len(edd_dict), 6)
+        # this order is *completely unlike* the order in virt-manager,
+        # but it does appear to be what EDD is displaying.
+        self.assertEqual(edd_dict["vda"], 0x80)
+        self.assertEqual(edd_dict["sdb"], 0x81)
+        self.assertEqual(edd_dict["sda"], 0x82)
+        self.assertEqual(edd_dict["sde"], 0x83)
+        self.assertEqual(edd_dict["sdc"], 0x84)
+        self.assertEqual(edd_dict["sdd"], 0x85)
+        debugs = [
+            ("edd: data extracted from 0x%x:%r", 0x80, fakeedd[0x80]),
+            ("edd: data extracted from 0x%x:%r", 0x81, fakeedd[0x81]),
+            ("edd: data extracted from 0x%x:%r", 0x82, fakeedd[0x82]),
+            ("edd: data extracted from 0x%x:%r", 0x83, fakeedd[0x83]),
+            ("edd: data extracted from 0x%x:%r", 0x84, fakeedd[0x84]),
+            ("edd: data extracted from 0x%x:%r", 0x85, fakeedd[0x85]),
+            ("edd: found device 0x%x at %s", 0x80,
+                    '/sys/firmware/edd/int13_dev80/'),
+            ("edd: found device 0x%x at %s", 0x81,
+                    '/sys/firmware/edd/int13_dev81/'),
+            ("edd: found device 0x%x at %s", 0x82,
+                    '/sys/firmware/edd/int13_dev82/'),
+            ("edd: found device 0x%x at %s", 0x83,
+                    '/sys/firmware/edd/int13_dev83/'),
+            ("edd: found device 0x%x at %s", 0x84,
+                    '/sys/firmware/edd/int13_dev84/'),
+            ("edd: found device 0x%x at %s", 0x85,
+                    '/sys/firmware/edd/int13_dev85/'),
+            ]
+        infos = [
+            ("edd: collected mbr signatures: %s",{ 'vda': '0x86531966',
+                                                   'sda': '0xe3bf124b',
+                                                   'sdb': '0x7dfff0db',
+                                                   'sdc': '0x63f1d7d8',
+                                                   'sdd': '0xee331b19',
+                                                   'sde': '0xfa0a111d',
+                                                   }),
+            ("edd: matched 0x%x to %s using PCI dev", 0x80, "vda"),
+            ("edd: matched 0x%x to %s using PCI dev", 0x81, "sdb"),
+            ("edd: matched 0x%x to %s using MBR sig", 0x82, "sda"),
+            ("edd: matched 0x%x to %s using MBR sig", 0x83, "sde"),
+            ("edd: matched 0x%x to %s using PCI dev", 0x84, "sdc"),
+            ("edd: matched 0x%x to %s using MBR sig", 0x85, "sdd"),
+            ('edd: Could not find Virtio device for pci dev %s channel %s', '00:01.1', 0),
+            ('edd: Could not find Virtio device for pci dev %s channel %s', '00:0b.0', 0),
+            ]
+        errors = [
+            ('edd: Found too many ATA devices for EDD device 0x%x: %s', 129, ['../devices/pci0000:00/0000:00:01.1/ata7/host6/target6:0:1/6:0:1:0/block/sdb', '../devices/pci0000:00/0000:00:01.1/ata7/host6/target6:0:0/6:0:0:0/block/sr0']),
+            ]
+        self.checkLogs(debugs=debugs, infos=infos, errors=errors)


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


More information about the anaconda-patches mailing list