[master 13/16] edd: Fix the error case in the "absurd_virt" test.
vathpela
installerbot-noreply at redhat.com
Fri Nov 6 20:58:34 UTC 2015
From: Peter Jones <pjones at redhat.com>
The problem is that SeaBIOS (and thus our qemu VMs) doesn't enumerate ata
devices separately from PCI devices. So on a real system,
EddEntry.ata_device should reset to 0 for each new PCI device, but on a
virt running SeaBIOS, they just count up monotonically.
So do the /right/ thing when we can, and if we don't find a correct
device, try relaxing the requirement on the scsi target id in sysfs, and
see if we find the right port number anyway.
Signed-off-by: Peter Jones <pjones at redhat.com>
---
blivet/devicelibs/edd.py | 137 ++++++++++++++++++++++++--------------
tests/devicelibs_test/edd_test.py | 7 +-
2 files changed, 92 insertions(+), 52 deletions(-)
diff --git a/blivet/devicelibs/edd.py b/blivet/devicelibs/edd.py
index e5ac338..44b4be8 100644
--- a/blivet/devicelibs/edd.py
+++ b/blivet/devicelibs/edd.py
@@ -358,47 +358,9 @@ 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)
- link = util.sysfs_readlink(path=emptyslash, link=path)
- testdata_log.debug("sysfs link: \"%s\" -> \"%s\"", path, link)
- # just add /sys/block/ at the beginning so it's always valid
- # paths in the filesystem...
- components = ['/sys/block'] + link.split('/')
- if len(components) != 11:
- 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 3's "channel" doesn't really mean anything at all on SATA,
- # and 255 means "not in use". Any other value should be an ATA
- # device (but might be a SATA device in compat mode), and that
- # matches N in devM.N . So basically "channel" means master/slave
- # for ATA (non-SATA) devices. Also in EDD 3, SATA port multipliers
- # aren't represented in any way.
- #
- # In EDD 4, which unfortunately says to leave 0x30 as the version
- # number, the port multiplier id is an additional field on the
- # interface. So basically we should try to use the value the
- # kernel gives us*, but we can't trust it. Thankfully there
- # won't be a devX.Y.Z (i.e. a port multiplier device) in sysfs
- # that collides with devX.Z (a non-port-multiplied device),
- # so if we get a value from the kernel, we can try with and
- # without it.
- #
- # * When the kernel finally learns of these facts...
- #
- if components[4] != '0000:%s' % (self.edd.pci_dev,):
- continue
- if not components[5].startswith('ata'):
- continue
- ata_port = components[5]
- ata_port_idx = int(components[5][3:])
+ retries = []
+ def match_port(components, ata_port, ata_port_idx, path, link):
fn = util.Path(util.join_paths(components[0:6] \
+ ['ata_port', ata_port]), root=self.root)
port_no = int(util.get_sysfs_attr(fn, 'port_no'))
@@ -407,13 +369,13 @@ def devname_from_ata_pci_dev(self):
# On ATA, port_no is kernel's ata_port->local_port_no, which
# should be the same as the ata device number.
if port_no != self.edd.ata_device:
- continue
+ return
else:
# On SATA, "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
+ return
fn = components[0:6] + ['link%d' % (ata_port_idx,),]
exp = [r'.*']+fn+[r'dev%d\.(\d+)(\.(\d+)){0,1}$' % (ata_port_idx,)]
@@ -440,15 +402,92 @@ 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):
- answers.append({
- 'link': util.Path(link, root=self.root),
- 'path': path.split('/')[-1]})
+ yield ({'link': util.Path(link, root=self.root),
+ 'path': path.split('/')[-1]})
else:
pmp = int(match.group(1))
if self.edd.ata_pmp == pmp:
- answers.append({
- 'link': util.Path(link, root=self.root),
- 'path': path.split('/')[-1]})
+ yield ({'link': util.Path(link, root=self.root),
+ 'path': path.split('/')[-1]})
+
+ answers = []
+ for path in pattern.glob():
+ emptyslash = util.Path("/", self.root)
+ path = util.Path(path, self.root)
+ link = util.sysfs_readlink(path=emptyslash, link=path)
+ testdata_log.debug("sysfs link: \"%s\" -> \"%s\"", path, link)
+ # just add /sys/block/ at the beginning so it's always valid
+ # paths in the filesystem...
+ components = ['/sys/block'] + link.split('/')
+ if len(components) != 11:
+ 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 3's "channel" doesn't really mean anything at all on SATA,
+ # and 255 means "not in use". Any other value should be an ATA
+ # device (but might be a SATA device in compat mode), and that
+ # matches N in devM.N . So basically "channel" means master/slave
+ # for ATA (non-SATA) devices. Also in EDD 3, SATA port multipliers
+ # aren't represented in any way.
+ #
+ # In EDD 4, which unfortunately says to leave 0x30 as the version
+ # number, the port multiplier id is an additional field on the
+ # interface. So basically we should try to use the value the
+ # kernel gives us*, but we can't trust it. Thankfully there
+ # won't be a devX.Y.Z (i.e. a port multiplier device) in sysfs
+ # that collides with devX.Z (a non-port-multiplied device),
+ # so if we get a value from the kernel, we can try with and
+ # without it.
+ #
+ # * When the kernel finally learns of these facts...
+ #
+ if components[4] != '0000:%s' % (self.edd.pci_dev,):
+ continue
+ if not components[5].startswith('ata'):
+ continue
+ ata_port = components[5]
+ ata_port_idx = int(components[5][3:])
+
+ # strictly this should always be required, but #!@#!@#!@ seabios
+ # iterates the sata device number /independently/ of the host
+ # bridge it claims things are attached to. In that case this
+ # the scsi target will always have "0" as the ID component.
+ args = { 'device': self.edd.ata_device }
+ exp = r"target\d+:0:%(device)s/\d+:0:%(device)s:0/block/.*" % args
+ matcher = re.compile(exp)
+ match = matcher.match("/".join(components[7:]))
+ if not match:
+ retries.append({
+ 'components': components,
+ 'ata_port': ata_port,
+ 'ata_port_idx': ata_port_idx,
+ 'path': path,
+ 'link': link,
+ })
+ continue
+
+ for port in match_port(components, ata_port, ata_port_idx, path,
+ link):
+ answers.append(port)
+
+ # now handle the ones we discarded because libata's scsi id doesn't
+ # match the ata_device.
+ for retry in retries:
+ for port in match_port(**retry):
+ if answers:
+ log.warning("edd: ignoring possible extra match for ATA device %s channel %s ata %d pmp %s: %s",
+ self.edd.pci_dev, self.edd.channel,
+ self.edd.ata_device, self.edd.ata_pmp,
+ retry['path'])
+ else:
+ log.warning("edd: using possible extra match for ATA device %s channel %s ata %d pmp %s: %s",
+ self.edd.pci_dev, self.edd.channel,
+ self.edd.ata_device, self.edd.ata_pmp,
+ retry['path'])
+ answers.append(port)
if len(answers) > 1:
log.error("edd: Found too many ATA devices for EDD device 0x%x: %s",
@@ -459,7 +498,7 @@ def devname_from_ata_pci_dev(self):
return answers[0]['path']
else:
log.warning(
- "edd: Could not find ATA device for pci dev %s channel %s ata %d pmp %d",
+ "edd: Could not find ATA device for pci dev %s channel %s ata %d pmp %s",
self.edd.pci_dev, self.edd.channel,
self.edd.ata_device, self.edd.ata_pmp)
diff --git a/tests/devicelibs_test/edd_test.py b/tests/devicelibs_test/edd_test.py
index 713e1ba..b81befd 100644
--- a/tests/devicelibs_test/edd_test.py
+++ b/tests/devicelibs_test/edd_test.py
@@ -197,6 +197,7 @@ def test_get_edd_dict_sata_usb(self):
("edd: interface type %s is not implemented (%s)", "USB",
"/sys/firmware/edd/int13_dev81/"),
("edd: interface details: %s", "USB \tserial_number: 30302e31"),
+ ('edd: using possible extra match for ATA device %s channel %s ata %d pmp %s: %s', '00:1f.2', 255, 1, None, '/sys/block/sda')
]
self.check_logs(debugs=debugs, infos=infos, warnings=warnings)
@@ -358,7 +359,7 @@ def test_get_edd_dict_absurd_virt(self):
('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']),
+ warnings = [
+ ('edd: ignoring possible extra match for ATA device %s channel %s ata %d pmp %s: %s', '00:01.1', 0, 1, None, '/sys/block/sr0'),
]
- self.check_logs(debugs=debugs, infos=infos, errors=errors)
+ self.check_logs(debugs=debugs, infos=infos, warnings=warnings)
--
To view this commit on github, visit https://github.com/rhinstaller/blivet/commit/8cbd219a362353215343788b12345f59f499994b
More information about the anaconda-patches
mailing list