Re: [rhel6-branch] edd: refactor and enhance the edd module.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Comments bellow. Honestly, I think the guessing part shouldn't be part of anaconda, but probably either edd itself or parted or..., but who am I to complain..

> +re_host_bus = re.compile("^PCI\s*(\S*)\s*channel: (\S*)\s*$")
> +re_interface_scsi = re.compile("^SCSI\s*id: (\S*)\s*lun: (\S*)\s*$")
> +re_interface_ata = re.compile("^ATA\s*device: (\S*)\s*$")

You probably should use raw string here. As we found out \s works because it is not considered to be known escape sequence atm.. but that might change in the future and better be safe than sorry..


> +def collect_edd_data():
> + edd_data_dict = {}
> + for biosdev in range(0x80, 0x80+16):

Although I am familiar with bios numbering, we should probably add comments explaining those constants here, so when we get older and senile we know what this does.. It also applies to the info we are getting from different sysfs files, the heuristic logic should be documented at least in some overview form..



> + fd = os.open(dev.path, os.O_RDONLY)
> + os.lseek(fd, 440, 0)
> + mbrsig = struct.unpack('I', os.read(fd, 4))
> + os.close(fd)

This ought to be documented as well, what is at the 440th byte in that file? I know I means some kind of integer, but that is it, I have no clue what this does.


> + mbrsig_str = "0x%08x" % mbrsig
> + # sanity check
> + if mbrsig_str == '0x00000000':


So we are trying to compare string representation with it's binary form from two different sources right.. this should be probably explained as well.. 


Overall, the code is a hack, but probably the best one we can have at the moment. I would vote for a detailed description of the sources of information we use (edd, /sys, binary partition table, pciids, ...) and what we are looking for there. I would like to know more about the assumptions we use for the heuristic matching as we might get into the situation when we need to change it and understanding code like this takes some time.

I am glad you wrote the tests for this, we might need them...

--
Martin Sivák
msivak@xxxxxxxxxx
Red Hat Czech
Anaconda team / Brno, CZ

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list



[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux