[master 22/22] edd: Fix the error case in the "absurd_virt" test.
vathpela
installerbot-noreply at redhat.com
Wed Oct 28 15:55:22 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 | 133 ++++++++++++++++++++++++--------------
tests/devicelibs_test/edd_test.py | 1 +
2 files changed, 87 insertions(+), 47 deletions(-)
diff --git a/blivet/devicelibs/edd.py b/blivet/devicelibs/edd.py
index 1e4a16b..9b3fdb2 100644
--- a/blivet/devicelibs/edd.py
+++ b/blivet/devicelibs/edd.py
@@ -354,47 +354,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'))
@@ -403,13 +365,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,)]
@@ -436,16 +398,93 @@ 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),
+ 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),
+ 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",
self.edd.bios_device_number,
@@ -455,7 +494,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 2bc756a..c40e13e 100644
--- a/tests/devicelibs_test/edd_test.py
+++ b/tests/devicelibs_test/edd_test.py
@@ -238,6 +238,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.checkLogs(debugs=debugs, infos=infos, warnings=warnings)
--
To view this commit on github, visit https://github.com/rhinstaller/blivet/commit/da5df9c6638200ed39d11682f65fbe6308f2f1a4
More information about the anaconda-patches
mailing list