Both of these look okay to me, but I'd feel better having all of this
new code validated by the reporter of the bug.
Dave
On Thu, 2010-12-16 at 09:59 +0100, Ales Kozumplik wrote:
> In general there is not always mbrsig on a disk (new device), so we can't
> always succeed in creating the disk -> bios id mapping. On the other hand,
> the pci_dev symlink in edd does not give us complete information about the
> device. So we have to consider both in compareDisks().
> ---
> kickstart.py | 4 +-
> storage/__init__.py | 18 ++++--
> storage/devicelibs/edd.py | 167 ++++++++++++++++++++++++++++++---------------
> 3 files changed, 126 insertions(+), 63 deletions(-)
>
> diff --git a/kickstart.py b/kickstart.py
> index b606ab8..67b1480 100644
> --- a/kickstart.py
> +++ b/kickstart.py
> @@ -665,8 +665,8 @@ class PartitionData(commands.partition.F12_PartData):
> storage.doAutoPart = False
>
> if self.onbiosdisk != "":
> - for (disk, biosdisk) in storage.eddDict.iteritems():
> - if str(biosdisk) == self.onbiosdisk:
> + for (disk, eddinfo) in storage.eddDict.iteritems():
> + if str(eddinfo.biosdisk) == self.onbiosdisk:
> self.disk = disk
> break
>
> diff --git a/storage/__init__.py b/storage/__init__.py
> index 464eebf..09dab1f 100644
> --- a/storage/__init__.py
> +++ b/storage/__init__.py
> @@ -1180,18 +1180,26 @@ class Storage(object):
> return self.fsset.rootDevice
>
> def compareDisks(self, first, second):
> - if self.eddDict.has_key(first) and self.eddDict.has_key(second):
> - one = self.eddDict[first]
> - two = self.eddDict[second]
> + # if exactly one of the devices has a pcidev, prefer it
> + if bool(self.eddDict[first].pcidev) ^ bool(self.eddDict[second].pcidev):
> + if self.eddDict[first].pcidev:
> + return -1
> + if self.eddDict[second].pcidev:
> + return 1
> +
> + # next, compare the biosdev numbers
> + if self.eddDict[first].biosdev and self.eddDict[second].biosdev:
> + one = self.eddDict[first].biosdev
> + two = self.eddDict[second].biosdev
> if (one < two):
> return -1
> elif (one > two):
> return 1
>
> # if one is in the BIOS and the other not prefer the one in the BIOS
> - if self.eddDict.has_key(first):
> + if self.eddDict[first].biosdev:
> return -1
> - if self.eddDict.has_key(second):
> + if self.eddDict[second].biosdev:
> return 1
>
> if first.startswith("hd"):
> diff --git a/storage/devicelibs/edd.py b/storage/devicelibs/edd.py
> index da03914..db2ba57 100644
> --- a/storage/devicelibs/edd.py
> +++ b/storage/devicelibs/edd.py
> @@ -21,77 +21,132 @@
> #
>
> import os
> +import os.path
> import struct
>
> import logging
> log = logging.getLogger("storage")
>
> +class EddInfo:
> + def __init__(self, biosdev):
> + self.biosdev = biosdev
> + self.pcidev = None
> +
> def get_edd_dict(devices):
> - """Given an array of devices return a dict with the BIOS ID for them."""
> + """
> + Given an array of devices return a mapping from their devices names to their
> + EddInfo.
> +
> + CAVEATS: It is important to note that the current implementation of the edd
> + kernel module is incomplete and does not give us a reliable method about
> + discovering the complete device <-> BIOS device mapping. Checking the MBR
> + signature works in most cases but will always fail e.g. on new disks that do
> + not contain an MBR yet. On the other hand, the pcidev entry which is
> + guaranteed to exist is shared across all disks connected through the same
> + PCI device and so does not provide a one-to-one relation.
> + """
> +
> + # initialize the dict with blank info
> edd_dict = {}
> + for dev in devices:
> + edd_dict[dev.name] = EddInfo(None)
>
> for biosdev in range(80, 80 + 15):
> +
> sysfspath = "/sys/firmware/edd/int13_dev%d" % biosdev
> if not os.path.exists(sysfspath):
> break # We are done
>
> - sysfspath = "/sys/firmware/edd/int13_dev%d/mbr_signature" % biosdev
> - if not os.path.exists(sysfspath):
> - log.warning("No mbrsig for biosdev: %d" % biosdev)
> - continue
> + dev = _find_device_from_mbrsig(devices, biosdev)
> + if dev:
> + edd_dict[dev].biosdev = biosdev
> +
> + (pcidev, devs_for_pcidev) = _devices_for_pcidev(devices, biosdev)
> + for d in devs_for_pcidev:
> + edd_dict[d.name].pcidev = pcidev
> +
> + _dump_dict(edd_dict)
> + return edd_dict
> +
> +def _dump_dict(edd_dict):
> + log.debug("edd:generated dict:")
> + for key in edd_dict:
> + log.debug("%s: biosdev: %s, pcidev: %s" %
> + (key, edd_dict[key].biosdev, edd_dict[key].pcidev))
> + log.debug("edd:end of generated dict dump")
> +
> +def _find_device_from_mbrsig(devices, biosdev):
> + sysfspath = "/sys/firmware/edd/int13_dev%d/mbr_signature" % biosdev
> + if not os.path.exists(sysfspath):
> + log.info("edd: no mbrsig for biosdev: %d" % biosdev)
> + return None
> +
> + try:
> + file = open(sysfspath, "r")
> + eddsig = file.read()
> + file.close()
> + except (IOError, OSError) as e:
> + log.info("edd: error reading EDD mbrsig for %d: %s" %
> + (biosdev, str(e)))
> + return None
>
> + sysfspath = "/sys/firmware/edd/int13_dev%d/sectors" % biosdev
> + try:
> + file = open(sysfspath, "r")
> + eddsize = file.read()
> + file.close()
> + except (IOError, OSError) as e:
> + eddsize = None
> +
> + found = []
> + for dev in devices:
> try:
> - file = open(sysfspath, "r")
> - eddsig = file.read()
> - file.close()
> - except (IOError, OSError) as e:
> - log.warning("Error reading EDD mbrsig for %d: %s" %
> - (biosdev, str(e)))
> + fd = os.open(dev.path, os.O_RDONLY)
> + os.lseek(fd, 440, 0)
> + mbrsig = struct.unpack('I', os.read(fd, 4))
> + os.close(fd)
> + except OSError as e:
> + log.info("edd: error reading mbrsig from disk %s: %s" %
> + (dev.name, str(e)))
> continue
>
> - sysfspath = "/sys/firmware/edd/int13_dev%d/sectors" % biosdev
> - try:
> - file = open(sysfspath, "r")
> - eddsize = file.read()
> - file.close()
> - except (IOError, OSError) as e:
> - eddsize = None
> -
> - found = []
> - for dev in devices:
> - try:
> - fd = os.open(dev.path, os.O_RDONLY)
> - os.lseek(fd, 440, 0)
> - mbrsig = struct.unpack('I', os.read(fd, 4))
> - os.close(fd)
> - except OSError as e:
> - log.warning("Error reading mbrsig from disk %s: %s" %
> - (dev.name, str(e)))
> - continue
> -
> - mbrsigStr = "0x%08x\n" % mbrsig
> - if mbrsigStr == eddsig:
> - if eddsize:
> - sysfspath = "/sys%s/size" % dev.sysfsPath
> - try:
> - file = open(sysfspath, "r")
> - size = file.read()
> - file.close()
> - except (IOError, OSError) as e:
> - log.warning("Error getting size for: %s" % dev.name)
> - continue
> - if eddsize != size:
> - continue
> - found.append(dev.name)
> -
> - if not found:
> - log.error("No matching mbr signature found for biosdev %d" %
> - biosdev)
> - elif len(found) > 1:
> - log.error("Multiple signature matches found for biosdev %d: %s" %
> - (biosdev, str(found)))
> - else:
> - log.info("Found %s for biosdev %d" %(found[0], biosdev))
> - edd_dict[found[0]] = biosdev
> + mbrsigStr = "0x%08x\n" % mbrsig
> + if mbrsigStr == eddsig:
> + if eddsize:
> + sysfspath = "/sys%s/size" % dev.sysfsPath
> + try:
> + file = open(sysfspath, "r")
> + size = file.read()
> + file.close()
> + except (IOError, OSError) as e:
> + log.warning("Error getting size for: %s" % dev.name)
> + continue
> + if eddsize != size:
> + continue
> + found.append(dev.name)
>
> - return edd_dict
> + if not found:
> + log.error("edd: no matching mbr signature found for biosdev %d" %
> + biosdev)
> + return None
> + elif len(found) > 1:
> + log.error("edd: multiple signature matches found for biosdev %d: %s" %
> + (biosdev, str(found)))
> + return None
> + else:
> + return found[0]
> +
> +def _devices_for_pcidev(devices, biosdev):
> + edd_dir = "/sys/firmware/edd/int13_dev%d" % biosdev
> + try:
> + dereferenced = os.readlink("%s/pci_dev" % edd_dir)
> + except OSError as e:
> + log.info ("edd: no pci_dev for biosdev %d" % biosdev)
> + return (None, [])
> + # get absolute and normalized path
> + sysfs_path = os.path.normpath(os.path.join(edd_dir, dereferenced))
> + if sysfs_path.startswith("/sys/"):
> + sysfs_path = sysfs_path[4:]
> + return (sysfs_path,
> + [dev for dev in devices if dev.sysfsPath.startswith(sysfs_path)]
> + )
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
