Re: [PATCH 2/2] Fix EDD BIOS disk order detection in general and make it work with dmraid

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

 





On 04/08/2009 10:40 AM, Joel Granados wrote:
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,&currentSig))<  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.


That is what dm_node_from_name() does, it is just a wrapper around pyblock.
I used it because it seemed right to use the devicelibs functions in new
code rather then calling pyblock directly.

Regards,

Hans

_______________________________________________
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