to me it looks ok.
On Tue, Apr 07, 2009 at 09:13:57PM +0200, Hans de Goede wrote:
> Our C-code isys EDD BIOS disk order detection is broken in 2 ways atm:
> 1) It tries to open "sda" instead of "/dev/sda"
> 2) It tries to open floppies and when it fails, it aborts further device
> scanning
>
> Besides that it didn't work with dmraid setups due to the following issues:
> 3) The C-code will not identify a device by the mbr signature from the
> EDD BIOS info, if multiple devices match the signature, however
> if we have a fakeraid mirror for example we will have 3 matching devices
> both the 2 raw disks and the device mapper device.
> 4) The python code was using /dev/mapper/longName names where as the
> C-code uses /dev/dm-#
> 5) The python code was calling the C-code (which builds a cache) before it
> setup the dmraid sets, so those were not known to the C-code
> ---
> isys/eddsupport.c | 33 +++++++++++++++++++++++++++------
> isys/isys.py | 27 +++++++++++++++++++++------
> 2 files changed, 48 insertions(+), 12 deletions(-)
>
> diff --git a/isys/eddsupport.c b/isys/eddsupport.c
> index ae974c5..5356491 100644
> --- a/isys/eddsupport.c
> +++ b/isys/eddsupport.c
> @@ -106,8 +106,10 @@ static struct device ** createDiskList(){
>
> static int readDiskSig(char *device, uint32_t *disksig) {
> int fd, rc;
> + char devnodeName[64];
>
> - fd = open(device, O_RDONLY);
> + snprintf(devnodeName, sizeof(devnodeName), "/dev/%s", device);
> + fd = open(devnodeName, O_RDONLY);
> if (fd < 0) {
> #ifdef STANDALONE
> fprintf(stderr, "Error opening device %s: %s\n ", device,
> @@ -147,7 +149,7 @@ static int mapBiosDisks(struct device** devices,const char *path) {
> char * sigFileName;
> uint32_t mbrSig, biosNum, currentSig;
> struct device **currentDev, **foundDisk;
> - int i, rc, ret;
> + int i, rc, ret, dm_nr, highest_dm;
>
> dirHandle = opendir(path);
> if(!dirHandle){
> @@ -176,22 +178,41 @@ static int mapBiosDisks(struct device** devices,const char *path) {
> sigFileName = malloc(strlen(path) + strlen(entry->d_name) + 20);
> sprintf(sigFileName, "%s/%s/%s", path, entry->d_name, SIG_FILE);
> if (readMbrSig(sigFileName, &mbrSig) == 0) {
> - for (currentDev = devices, i = 0, foundDisk=NULL;
> - (*currentDev) != NULL && i<2;
> + for (currentDev = devices, i = 0, foundDisk=NULL, highest_dm=-1;
> + (*currentDev) != NULL;
> currentDev++) {
> if (!(*currentDev)->device)
> continue;
>
> if ((rc=readDiskSig((*currentDev)->device, ¤tSig)) < 0) {
> - if (rc == -ENOMEDIUM)
> + if (rc == -ENOMEDIUM || rc == -ENXIO)
> continue;
> closedir(dirHandle);
> return 0;
> }
>
> if (mbrSig == currentSig) {
> - foundDisk=currentDev;
> + /* When we have a fakeraid setup we will find multiple hits
> + a number for the raw disks (1 when striping, 2 when
> + mirroring, more with raid on raid like raid 01 or 10)
> + and a number for the dm devices (normally only one dm
> + device will match, but more with raid on raid).
> + Since with raid on raid the last dm device created
> + will be the top layer raid, we want the highest matching
> + dm device. */
> + if (!strncmp((*currentDev)->device, "dm-", 3) &&
> + sscanf((*currentDev)->device+3, "%d", &dm_nr) == 1) {
> + if (dm_nr > highest_dm) {
> + highest_dm = dm_nr;
> + foundDisk=currentDev;
> + i = 1;
> + }
> + } else if (!foundDisk ||
> + strncmp((*foundDisk)->device, "dm-", 3)) {
> + printf("using non dm device %s\n", (*currentDev)->device);
> + foundDisk=currentDev;
> i++;
> + }
> }
> }
>
> diff --git a/isys/isys.py b/isys/isys.py
> index d5ae223..88bfaf1 100755
> --- a/isys/isys.py
> +++ b/isys/isys.py
> @@ -308,14 +308,23 @@ def doGetBiosDisk(mbrSig):
>
> handleSegv = _isys.handleSegv
>
> -biosdisks = {}
> -for d in range(80, 80 + 15):
> - disk = doGetBiosDisk("%d" %(d,))
> - #print("biosdisk of %s is %s" %(d, disk))
> - if disk is not None:
> - biosdisks[disk] = d
>
> def compareDrives(first, second):
> + from storage.devicelibs.dm import dm_node_from_name
> +
> + biosdisks = {}
> + for d in range(80, 80 + 15):
> + disk = doGetBiosDisk("%d" %(d,))
> + #print("biosdisk of %s is %s" %(d, disk))
> + if disk is not None:
> + biosdisks[disk] = d
> +
Just a little suggestion. why not use getDmNodeFromName from pyblock?
the pyblock function would need some change also, so it can accept
strings with /dev/mapper as suffix. It might be worth it.
With that said, I'm ok either way.
> + # convert /dev/mapper/foo -> /dev/dm-#, as that is what is in biosdisks
> + if os.access("/dev/mapper/%s" % first, os.F_OK):
> + first = dm_node_from_name(first)
> + if os.access("/dev/mapper/%s" % second, os.F_OK):
> + second = dm_node_from_name(second)
> +
> if biosdisks.has_key(first) and biosdisks.has_key(second):
> one = biosdisks[first]
> two = biosdisks[second]
> @@ -324,6 +333,12 @@ def compareDrives(first, second):
> elif (one > two):
> return 1
>
> + # if one is in the BIOS and the other not prefer the one in the BIOS
> + if biosdisks.has_key(first):
> + return -1
> + if biosdisks.has_key(second):
> + return 1
> +
> if first.startswith("hd"):
> type1 = 0
> elif first.startswith("sd"):
> --
> 1.6.2
>
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list
--
Joel Andres Granados
Brno, Czech Republic, Red Hat.
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
