Cleanup a couple of serious issues with vinum.
authorMatthew Dillon <dillon@dragonflybsd.org>
Fri, 16 Sep 2005 04:33:14 +0000 (04:33 +0000)
committerMatthew Dillon <dillon@dragonflybsd.org>
Fri, 16 Sep 2005 04:33:14 +0000 (04:33 +0000)
* If someone specifies a partition or a slice instead of a raw drive, such as
  'vinum read /dev/da0d' or 'vinum read /dev/da0s1', vinum gets really confused
  when it constructs device names while doing the drive scan.  For example,
  it was constructing device names like '/dev/da0as1d'.  Fixed.

  Note that vinum cannot recover the complete configuration when vinum read
  is used to scan a single partition such as /dev/da0d.  You have to tell
  it to scan the all drives associated with the vinum volume for it to be
  able to complete configure the volume.  If a 'vinum list' shows drives
  as 'referenced' without a device path, the related volumes/plexes/whatever
  cannot be used until the remaining subdisks are loaded.

* vinum was attempting to construct the device major/minor number manually
  instead of using the appropriate macros, which prevented it from properly
  handling partitions greater then 'i' through 'p'.  Fixed.

* vinum was issuing BUF_STRATEGY calls without initializing b_dev.  In
  DragonFly, b_dev must be initialized prior to every strategy call, even
  when reusing a buffer (this requirement will later be used as part of
  the block number translation caching subsystem).

Reported-by: Dave Hayes
sys/dev/raid/vinum/vinumconfig.c
sys/dev/raid/vinum/vinuminterrupt.c
sys/dev/raid/vinum/vinumio.c

index 7aba00d..8c61937 100644 (file)
@@ -47,7 +47,7 @@
  *
  * $Id: vinumconfig.c,v 1.30 2000/05/01 09:45:50 grog Exp grog $
  * $FreeBSD: src/sys/dev/vinum/vinumconfig.c,v 1.32.2.6 2002/02/03 00:43:35 grog Exp $
- * $DragonFly: src/sys/dev/raid/vinum/vinumconfig.c,v 1.6 2004/07/02 15:47:56 joerg Exp $
+ * $DragonFly: src/sys/dev/raid/vinum/vinumconfig.c,v 1.7 2005/09/16 04:33:14 dillon Exp $
  */
 
 #define STATIC static
@@ -1168,7 +1168,7 @@ config_subdisk(int update)
            parameter++;                                    /* skip the keyword */
            if ((strlen(token[parameter]) != 1)
                || (token[parameter][0] < 'a')
-               || (token[parameter][0] > 'h'))
+               || (token[parameter][0] > 'p'))
                throw_rude_remark(EINVAL,
                    "%s: invalid partition %c",
                    sd->name,
index 2f6357d..9d3ac0f 100644 (file)
@@ -41,7 +41,7 @@
  *
  * $Id: vinuminterrupt.c,v 1.12 2000/11/24 03:41:42 grog Exp grog $
  * $FreeBSD: src/sys/dev/vinum/vinuminterrupt.c,v 1.25.2.3 2001/05/28 05:56:27 grog Exp $
- * $DragonFly: src/sys/dev/raid/vinum/vinuminterrupt.c,v 1.4 2005/08/03 16:36:33 hmp Exp $
+ * $DragonFly: src/sys/dev/raid/vinum/vinuminterrupt.c,v 1.5 2005/09/16 04:33:14 dillon Exp $
  */
 
 #include "vinumhdr.h"
@@ -392,6 +392,7 @@ complete_raid5_write(struct rqelement *rqe)
                    rqe->b.b_bufsize = rqe->b.b_bcount;     /* don't claim more */
                    rqe->b.b_resid = rqe->b.b_bcount;       /* nothing transferred */
                    rqe->b.b_blkno += rqe->dataoffset;      /* point to the correct block */
+                   rqe->b.b_dev = DRIVE[rqe->driveno].dev;
                    rqg->active++;                          /* another active request */
                    drive = &DRIVE[rqe->driveno];           /* drive to access */
 
@@ -429,6 +430,7 @@ complete_raid5_write(struct rqelement *rqe)
     rqe->b.b_bcount = rqe->buflen << DEV_BSHIFT;           /* length to write */
     rqe->b.b_bufsize = rqe->b.b_bcount;                            /* don't claim we have more */
     rqe->b.b_resid = rqe->b.b_bcount;                      /* nothing transferred */
+    rqe->b.b_dev = DRIVE[rqe->driveno].dev;
     rqg->active++;                                         /* another active request */
     drive = &DRIVE[rqe->driveno];                          /* drive to access */
 
index c8fdc06..b4ed4f1 100644 (file)
@@ -35,7 +35,7 @@
  *
  * $Id: vinumio.c,v 1.30 2000/05/10 23:23:30 grog Exp grog $
  * $FreeBSD: src/sys/dev/vinum/vinumio.c,v 1.52.2.6 2002/05/02 08:43:44 grog Exp $
- * $DragonFly: src/sys/dev/raid/vinum/vinumio.c,v 1.8 2005/06/28 18:38:06 joerg Exp $
+ * $DragonFly: src/sys/dev/raid/vinum/vinumio.c,v 1.9 2005/09/16 04:33:14 dillon Exp $
  */
 
 #include "vinumhdr.h"
@@ -119,21 +119,19 @@ open_drive(struct drive *drive, struct proc *p, int verbose)
 
     if (*dname == 's') {                                   /* slice */
        if (((dname[1] < '1') || (dname[1] > '4'))          /* invalid slice */
-       ||((dname[2] < 'a') || (dname[2] > 'h')))           /* or invalid partition */
+       ||((dname[2] < 'a') || (dname[2] > 'p')))           /* or invalid partition */
            return ENODEV;
-       devminor = ((unit & 31) << 3)                       /* unit */
-       +(dname[2] - 'a')                                   /* partition */
-       +((dname[1] - '0' + 1) << 16)                       /* slice */
-       +((unit & ~31) << 16);                              /* high-order unit bits */
+       devminor = dkmakeminor(unit, dname[1] - '0' + 1, (dname[2] - 'a'));
     } else {                                               /* compatibility partition */
-       if ((*dname < 'a') || (*dname > 'h'))               /* or invalid partition */
+       if ((*dname < 'a') || (*dname > 'p'))               /* or invalid partition */
            return ENODEV;
-       devminor = (*dname - 'a')                           /* partition */
-       +((unit & 31) << 3)                                 /* unit */
-       +((unit & ~31) << 16);                              /* high-order unit bits */
+       devminor = dkmakeminor(unit, 0, (dname[0] - 'a'));
     }
 
-    if ((devminor & 7) == 2)                               /* partition c */
+    /*
+     * Disallow partition c
+     */
+    if ((((devminor >> 17) & 0x08) | (devminor & 7)) == 2)
        return ENOTTY;                                      /* not buying that */
 
     drive->dev = udev2dev(makeudev(devmajor, devminor), 0);
@@ -856,56 +854,87 @@ vinum_scandisk(char *devicename[], int drives)
        char part;                                          /* UNIX partition */
        int slice;
        int founddrive;                                     /* flag when we find a vinum drive */
+       int has_slice = 0;
+       int has_part = 0;
+       char *tmp;
 
        founddrive = 0;                                     /* no vinum drive found yet on this spindle */
-       /* first try the partition table */
-       for (slice = 1; slice < 5; slice++)
-           for (part = 'a'; part < 'i'; part++) {
-               if (part != 'c') {                          /* don't do the c partition */
-                   snprintf(partname,
-                       DRIVENAMELEN,
-                       "%ss%d%c",
-                       devicename[driveno],
-                       slice,
-                       part);
-                   drive = check_drive(partname);          /* try to open it */
-                   if ((drive->lasterror != 0)             /* didn't work, */
+
+       /*
+        * If the device path contains a slice we do not try to tack on
+        * another slice.  If the device path has a partition we only check
+        * that partition.
+        */
+       if ((tmp = rindex(devicename[driveno], '/')) == NULL)
+           tmp = devicename[driveno];
+       while (*tmp && (*tmp < '0' || *tmp > '9'))
+           ++tmp;
+       while (*tmp && *tmp >= '0' && *tmp <= '9')
+           ++tmp;
+       if (*tmp == 's')
+           has_slice = strtol(tmp + 1, &tmp, 0);
+       if (*tmp >= 'a' && *tmp <= 'p')
+           has_part = *tmp;
+
+       /*
+        * Scan slices if no slice was specified, only if no partition was
+        * specified.
+        */
+       if (has_slice == 0 && has_part == 0)
+       for (slice = 1; slice < 5; slice++) {
+           if (has_slice && slice != has_slice)
+               continue;
+
+           for (part = 'a'; part <= 'p'; part++) {
+               if (has_part && part != has_part)
+                   continue;
+               if (part == 'c')
+                   continue;
+               snprintf(partname, DRIVENAMELEN,
+                       "%ss%d%c", devicename[driveno], slice, part);
+               drive = check_drive(partname);      /* try to open it */
+               if ((drive->lasterror != 0)                 /* didn't work, */
                    ||(drive->state != drive_up))
-                       free_drive(drive);                  /* get rid of it */
-                   else if (drive->flags & VF_CONFIGURED)  /* already read this config, */
-                       log(LOG_WARNING,
-                           "vinum: already read config from %s\n", /* say so */
-                           drive->label.name);
-                   else {
-                       drivelist[gooddrives] = drive->driveno; /* keep the drive index */
-                       drive->flags &= ~VF_NEWBORN;        /* which is no longer newly born */
-                       gooddrives++;
-                       founddrive++;
-                   }
+                   free_drive(drive);              /* get rid of it */
+               else if (drive->flags & VF_CONFIGURED)  /* already read this config, */
+                   log(LOG_WARNING,
+                       "vinum: already read config from %s\n", /* say so */
+                       drive->label.name);
+               else {
+                   drivelist[gooddrives] = drive->driveno;     /* keep the drive index */
+                   drive->flags &= ~VF_NEWBORN;            /* which is no longer newly born */
+                   gooddrives++;
+                   founddrive++;
                }
            }
-       if (founddrive == 0) {                              /* didn't find anything, */
-           for (part = 'a'; part < 'i'; part++)            /* try the compatibility partition */
-               if (part != 'c') {                          /* don't do the c partition */
-                   snprintf(partname,                      /* /dev/sd0a */
-                       DRIVENAMELEN,
-                       "%s%c",
-                       devicename[driveno],
-                       part);
-                   drive = check_drive(partname);          /* try to open it */
-                   if ((drive->lasterror != 0)             /* didn't work, */
-                   ||(drive->state != drive_up))
-                       free_drive(drive);                  /* get rid of it */
-                   else if (drive->flags & VF_CONFIGURED)  /* already read this config, */
-                       log(LOG_WARNING,
-                           "vinum: already read config from %s\n", /* say so */
-                           drive->label.name);
-                   else {
-                       drivelist[gooddrives] = drive->driveno; /* keep the drive index */
-                       drive->flags &= ~VF_NEWBORN;        /* which is no longer newly born */
-                       gooddrives++;
-                   }
+       }
+       if (founddrive == 0 && has_slice == 0) {            /* didn't find anything, */
+           for (part = 'a'; part <= 'p'; part++) {         /* try the compatibility partition */
+               if (has_part && has_part != part)
+                   continue;
+               if (part == 'c')
+                   continue;
+               if (has_part) {
+                   snprintf(partname, DRIVENAMELEN,
+                           "%s", devicename[driveno]);
+               } else {
+                   snprintf(partname, DRIVENAMELEN,
+                           "%s%c", devicename[driveno], part);
                }
+               drive = check_drive(partname);      /* try to open it */
+               if ((drive->lasterror != 0)                 /* didn't work, */
+               ||(drive->state != drive_up))
+                   free_drive(drive);              /* get rid of it */
+               else if (drive->flags & VF_CONFIGURED)  /* already read this config, */
+                   log(LOG_WARNING,
+                       "vinum: already read config from %s\n", /* say so */
+                       drive->label.name);
+               else {
+                   drivelist[gooddrives] = drive->driveno;     /* keep the drive index */
+                   drive->flags &= ~VF_NEWBORN;            /* which is no longer newly born */
+                   gooddrives++;
+               }
+           }
        }
     }