vinum patch to test - (was Re: vinum problems)

Matthew Dillon dillon at apollo.backplane.com
Sun Jul 29 16:32:32 PDT 2007


    This patch needs some testing with various vinum topologies.

    I was not able to fix all the calls to dev_dstrategy() but I got most
    of them.   The changes I had to make to the codebase started snowballing
    oweing to the bad design the vinum -- it constantly translates between
    device numbers and SDs and volumes and god knows what, back and forth.
    Its impossible to keep track of it all.

    Also update your /sbin/vinum with a commit I just made which fixes a
    run-time seg-fault introduced by the libedit changes.

						-Matt

Index: vinuminterrupt.c
===================================================================
RCS file: /cvs/src/sys/dev/raid/vinum/vinuminterrupt.c,v
retrieving revision 1.11
diff -u -p -r1.11 vinuminterrupt.c
--- vinuminterrupt.c	10 Sep 2006 01:26:36 -0000	1.11
+++ vinuminterrupt.c	29 Jul 2007 22:25:02 -0000
@@ -305,8 +305,6 @@     struct request *rq;					    /* point
     struct rqgroup *rqg;				    /* and to the request group */
     struct rqelement *prqe;				    /* point to the parity block */
     struct drive *drive;				    /* drive to access */
-    cdev_t dev;
-
     rqg = rqe->rqg;					    /* and to our request group */
     rq = rqg->rq;					    /* point to our request */
     ubio = rq->bio;					    /* user's buffer header */
@@ -393,10 +391,9 @@ 		    rqe->b.b_data = &ubio->bio_buf->b_
 		    rqe->b.b_bcount = rqe->datalen << DEV_BSHIFT; /* length to write */
 		    rqe->b.b_resid = rqe->b.b_bcount;	    /* nothing transferred */
 		    rqe->b.b_bio1.bio_offset += (off_t)rqe->dataoffset << DEV_BSHIFT;	    /* point to the correct block */
-		    dev = DRIVE[rqe->driveno].dev;
-		    rqe->b.b_bio1.bio_driver_info = dev;
-		    rqg->active++;			    /* another active request */
 		    drive = &DRIVE[rqe->driveno];	    /* drive to access */
+		    rqe->b.b_bio1.bio_driver_info = drive->dev;
+		    rqg->active++;			    /* another active request */
 
 							    /* We can't sleep here, so we just increment the counters. */
 		    drive->active++;
@@ -408,10 +405,9 @@ 			vinum_conf.maxactive = vinum_conf.act
 #if VINUMDEBUG
 		    if (debug & DEBUG_ADDRESSES)
 			log(LOG_DEBUG,
-			    "  %s dev %d.%d, sd %d, offset 0x%llx, devoffset 0x%llx, length %d\n",
+			    "  %s dev %s, sd %d, offset 0x%llx, devoffset 0x%llx, length %d\n",
 			    (rqe->b.b_cmd == BUF_CMD_READ) ? "Read" : "Write",
-			    major(dev),
-			    minor(dev),
+			    drive->devicename,
 			    rqe->sdno,
 			    rqe->b.b_bio1.bio_offset - ((off_t)SD[rqe->sdno].driveoffset << DEV_BSHIFT),
 			    rqe->b.b_bio1.bio_offset,
@@ -419,7 +415,7 @@ 			    rqe->b.b_bcount);
 		    if (debug & DEBUG_LASTREQS)
 			logrq(loginfo_raid5_data, (union rqinfou) rqe, ubio);
 #endif
-		    dev_dstrategy(dev, &rqe->b.b_bio1);
+		    vn_strategy(drive->vp, &rqe->b.b_bio1);
 		}
 	    }
 	}
@@ -431,10 +427,9 @@     rqe->b.b_bio1.bio_done = complete_rq
     rqg->flags &= ~XFR_PARITYOP;			    /* reset flags that brought us here */
     rqe->b.b_bcount = rqe->buflen << DEV_BSHIFT;	    /* length to write */
     rqe->b.b_resid = rqe->b.b_bcount;			    /* nothing transferred */
-    dev = DRIVE[rqe->driveno].dev;
-    rqe->b.b_bio1.bio_driver_info = dev;
-    rqg->active++;					    /* another active request */
     drive = &DRIVE[rqe->driveno];			    /* drive to access */
+    rqe->b.b_bio1.bio_driver_info = drive->dev;
+    rqg->active++;					    /* another active request */
 
     /* We can't sleep here, so we just increment the counters. */
     drive->active++;
@@ -447,10 +442,9 @@ 
 #if VINUMDEBUG
     if (debug & DEBUG_ADDRESSES)
 	log(LOG_DEBUG,
-	    "  %s dev %d.%d, sd %d, offset 0x%llx, devoffset 0x%llx, length %d\n",
+	    "  %s dev %s, sd %d, offset 0x%llx, devoffset 0x%llx, length %d\n",
 	    (rqe->b.b_cmd == BUF_CMD_READ) ? "Read" : "Write",
-	    major(dev),
-	    minor(dev),
+	    device->devicename,
 	    rqe->sdno,
 	    rqe->b.b_bio1.bio_offset - ((off_t)SD[rqe->sdno].driveoffset << DEV_BSHIFT),
 	    rqe->b.b_bio1.bio_offset,
@@ -458,7 +452,7 @@ 	    rqe->b.b_bcount);
     if (debug & DEBUG_LASTREQS)
 	logrq(loginfo_raid5_parity, (union rqinfou) rqe, ubio);
 #endif
-    dev_dstrategy(dev, &rqe->b.b_bio1);
+    vn_strategy(drive->vp, &rqe->b.b_bio1);
 }
 
 /* Local Variables: */
Index: vinumio.c
===================================================================
RCS file: /cvs/src/sys/dev/raid/vinum/vinumio.c,v
retrieving revision 1.27
diff -u -p -r1.27 vinumio.c
--- vinumio.c	16 Jul 2007 21:31:06 -0000	1.27
+++ vinumio.c	29 Jul 2007 22:43:28 -0000
@@ -41,6 +41,7 @@ 
 #include "vinumhdr.h"
 #include "request.h"
 #include <vm/vm_zone.h>
+#include <sys/nlookup.h>
 
 static char *sappend(char *txt, char *s);
 static int drivecmp(const void *va, const void *vb);
@@ -52,113 +53,55 @@  */
 int
 open_drive(struct drive *drive, struct proc *p, int verbose)
 {
-    int devmajor;					    /* major devs for disk device */
-    int devminor;					    /* minor devs for disk device */
-    int unit;
-    int slice;
-    int part;
-    char *dname;
-
-    if (bcmp(drive->devicename, "/dev/", 5))		    /* device name doesn't start with /dev */
-	return ENOENT;					    /* give up */
-    if (drive->flags & VF_OPEN)				    /* open already, */
-	return EBUSY;					    /* don't do it again */
-
-    /*
-     * Yes, Bruce, I know this is horrible, but we
-     * don't have a root file system when we first
-     * try to do this.  If you can come up with a
-     * better solution, I'd really like it.  I'm
-     * just putting it in now to add ammuntion to
-     * moving the system to devfs.
-     */
-    dname = &drive->devicename[5];
-    drive->dev = NULL;					    /* no device yet */
-
-    /* Find the device */
-    if (bcmp(dname, "ad", 2) == 0)			    /* IDE disk */
-	devmajor = 116;
-    else if (bcmp(dname, "wd", 2) == 0)			    /* IDE disk */
-	devmajor = 3;
-    else if (bcmp(dname, "da", 2) == 0)
-	devmajor = 13;
-    else if (bcmp(dname, "vn", 2) == 0)
-	devmajor = 43;
-    else if (bcmp(dname, "md", 2) == 0)
-	devmajor = 95;
-    else if (bcmp(dname, "vkd", 3) == 0) {
-	devmajor = 97;
-	dname += 1;
-    } else if (bcmp(dname, "amrd", 4) == 0) {
-	devmajor = 133;
-	dname += 2;
-    } else if (bcmp(dname, "mlxd", 4) == 0) {
-	devmajor = 131;
-	dname += 2;
-    } else if (bcmp(dname, "idad", 4) == 0) {
-	devmajor = 109;
-	dname += 2;
-    } else if (bcmp(dname, "twed", 4) == 0) {               /* 3ware raid */
-      devmajor = 147;
-      dname += 2;
-    } else if (bcmp(dname, "ar", 2) == 0) {
-	devmajor = 157;
-    } else
-	return ENODEV;
-    dname += 2;						    /* point past */
+    struct nlookupdata nd;
+    int error;
+    const char *dname;
 
     /*
-     * Found the device.  Require the form
-     * <unit>s<slice><partition>
+     * Fail if already open
      */
-    if (*dname < '0' || *dname > '9')
-	return(ENODEV);
-    unit = strtol(dname, &dname, 10);
-    if (*dname != 's')
-	return(ENODEV);
-    ++dname;
+    if (drive->flags & VF_OPEN)
+	return EBUSY;
+    dname = drive->devicename;
 
     /*
-     * Convert slice number to value suitable for
-     * dkmakeminor().  0->0, 1->2, 2->3, etc.
+     * Severe hack to disallow opening partition 'c'
      */
-    slice = strtol(dname, &dname, 10);
-    if (slice > 0)
-	++slice;
+    if (strncmp(dname, "/dev", 4) == 0 && dname[strlen(dname)-1] == 'c')
+	return ENOTTY;
 
-    if (*dname < 'a' || *dname > 'p')
-	return ENODEV;
-
-    part = *dname - 'a';
-    devminor = dkmakeminor(unit, slice, part);
+    error = nlookup_init(&nd, drive->devicename, UIO_SYSSPACE, NLC_FOLLOW);
+    if (error)
+	return error;
+    error = vn_open(&nd, NULL, FREAD|FWRITE, 0);
+    drive->vp = nd.nl_open_vp;
+    nd.nl_open_vp = NULL;
+    nlookup_done(&nd);
+    if (error == 0 && drive->vp == NULL)
+	error = ENODEV;
 
     /*
-     * Disallow partition c
+     * A huge amount of pollution all over vinum requires that our low
+     * level drive be a device.
      */
-    if (part == 2)
-	return ENOTTY;					    /* not buying that */
-
-    drive->dev = udev2dev(makeudev(devmajor, devminor), 0);
-
-    if (drive->dev == NULL)
-	return ENODEV;
-
-    drive->dev->si_iosize_max = DFLTPHYS;
-    if (dev_is_good(drive->dev))
-	drive->lasterror = dev_dopen(drive->dev, FWRITE, 0, proc0.p_ucred);
-    else
-	drive->lasterror = ENOENT;
-
-    if (drive->lasterror != 0) {			    /* failed */
-	drive->state = drive_down;			    /* just force it down */
-	if (verbose)
+    if (error == 0 && drive->vp->v_type != VCHR) {
+	vn_close(drive->vp, FREAD|FWRITE);
+	drive->vp = NULL;
+	error = ENODEV;
+    }
+    if (error) {
+	drive->state = drive_down;
+	if (verbose) {
 	    log(LOG_WARNING,
 		"vinum open_drive %s: failed with error %d\n",
-		drive->devicename, drive->lasterror);
-    } else
-	drive->flags |= VF_OPEN;			    /* we're open now */
-
-    return drive->lasterror;
+		drive->devicename, error);
+	}
+    } else {
+	drive->dev = drive->vp->v_rdev;
+	drive->flags |= VF_OPEN;
+    }
+    drive->lasterror = error;
+    return error;
 }
 
 /*
@@ -224,12 +167,9 @@     drive->lasterror = open_drive(drive,
     if (drive->lasterror)
 	return drive->lasterror;
 
-    drive->lasterror = dev_dioctl(
-	drive->dev,
-	DIOCGPART,
-	(caddr_t) & drive->partinfo,
-	FREAD,
-	proc0.p_ucred);
+    drive->lasterror = VOP_IOCTL(drive->vp, DIOCGPART,
+				 (caddr_t)&drive->partinfo,
+				 FREAD|FWRITE, proc0.p_ucred);
     if (drive->lasterror) {
 	if (verbose)
 	    log(LOG_WARNING,
@@ -277,8 +217,11 @@      * If we can't access the drive, we 
      * the queues, which spec_close() will try to
      * do.  Get rid of them here first.
      */
-    drive->lasterror = dev_dclose(drive->dev, 0, 0);
-    drive->flags &= ~VF_OPEN;				    /* no longer open */
+    if (drive->vp) {
+	drive->lasterror = vn_close(drive->vp, FREAD|FWRITE);
+	drive->vp = NULL;
+    }
+    drive->flags &= ~VF_OPEN;
 }
 
 /*
@@ -337,7 +280,7 @@ 	bp->b_bio1.bio_offset = offset;			    /
 	saveaddr = bp->b_data;
 	bp->b_data = buf;
 	bp->b_bcount = len;
-	dev_dstrategy(drive->dev, &bp->b_bio1);
+	vn_strategy(drive->vp, &bp->b_bio1);
 	error = biowait(bp);
 	bp->b_data = saveaddr;
 	bp->b_flags |= B_INVAL | B_AGE;
@@ -673,11 +616,9 @@ 		    && (drive->state != drive_referenc
 		    wlabel_on = 1;			    /* enable writing the label */
 		    error = 0;
 #if 1
-		    error = dev_dioctl(drive->dev, /* make the label writeable */
-			DIOCWLABEL,
-			(caddr_t) & wlabel_on,
-			FWRITE,
-			proc0.p_ucred);
+		    error = VOP_IOCTL(drive->vp, DIOCWLABEL,
+				      (caddr_t)&wlabel_on,
+				      FREAD|FWRITE, proc0.p_ucred);
 #endif
 		    if (error == 0)
 			error = write_drive(drive, (char *) vhdr, VINUMHEADERLEN, VINUM_LABEL_OFFSET);
@@ -687,12 +628,11 @@ 		    if (error == 0)
 			error = write_drive(drive, config, MAXCONFIG, VINUM_CONFIG_OFFSET + MAXCONFIG);	/* second copy */
 		    wlabel_on = 0;			    /* enable writing the label */
 #if 1
-		    if (error == 0)
-			error = dev_dioctl(drive->dev, /* make the label non-writeable again */
-			    DIOCWLABEL,
-			    (caddr_t) & wlabel_on,
-			    FWRITE,
-			    proc0.p_ucred);
+		    if (error == 0) {
+			error = VOP_IOCTL(drive->vp, DIOCWLABEL,
+					  (caddr_t)&wlabel_on,
+					  FREAD|FWRITE, proc0.p_ucred);
+		    }
 #endif
 		    unlockdrive(drive);
 		    if (error) {
Index: vinumrequest.c
===================================================================
RCS file: /cvs/src/sys/dev/raid/vinum/vinumrequest.c,v
retrieving revision 1.19
diff -u -p -r1.19 vinumrequest.c
--- vinumrequest.c	7 Jun 2007 22:58:38 -0000	1.19
+++ vinumrequest.c	29 Jul 2007 22:32:45 -0000
@@ -436,6 +436,7 @@ 		if (debug & DEBUG_LASTREQS)
 		    logrq(loginfo_rqe, (union rqinfou) rqe, rq->bio);
 #endif
 		/* fire off the request */
+		/* XXX this had better not be a low level drive */
 		dev_dstrategy(dev, &rqe->b.b_bio1);
 	    }
 	}
@@ -899,7 +900,6 @@ void
 sdio(struct bio *bio)
 {
     cdev_t dev;
-    cdev_t sddev;
     struct sd *sd;
     struct sdbuf *sbp;
     daddr_t endoffset;
@@ -945,7 +945,6 @@ 	bp->b_flags |= B_ERROR;
 	biodone(bio);
 	return;
     }
-    sddev = DRIVE[sd->driveno].dev;		    /* device */
     bzero(sbp, sizeof(struct sdbuf));			    /* start with nothing */
     sbp->b.b_cmd = bp->b_cmd;
     sbp->b.b_bcount = bp->b_bcount;			    /* number of bytes to transfer */
@@ -975,10 +974,9 @@     }
 #if VINUMDEBUG
     if (debug & DEBUG_ADDRESSES)
 	log(LOG_DEBUG,
-	    "  %s dev %d.%d, sd %d, offset 0x%llx, devoffset 0x%llx, length %d\n",
+	    "  %s dev %s, sd %d, offset 0x%llx, devoffset 0x%llx, length %d\n",
 	    (sbp->b.b_cmd == BUF_CMD_READ) ? "Read" : "Write",
-	    major(sddev),
-	    minor(sddev),
+	    drive->devicename,
 	    sbp->sdno,
 	    sbp->b.b_bio1.bio_offset - ((off_t)SD[sbp->sdno].driveoffset << DEV_BSHIFT),
 	    sbp->b.b_bio1.bio_offset,
@@ -989,7 +987,7 @@ #if VINUMDEBUG
     if (debug & DEBUG_LASTREQS)
 	logrq(loginfo_sdiol, (union rqinfou) &sbp->b.b_bio1, &sbp->b.b_bio1);
 #endif
-    dev_dstrategy(sddev, &sbp->b.b_bio1);
+    vn_strategy(drive->vp, &sbp->b.b_bio1);
     crit_exit();
 }
 
Index: vinumvar.h
===================================================================
RCS file: /cvs/src/sys/dev/raid/vinum/vinumvar.h,v
retrieving revision 1.11
diff -u -p -r1.11 vinumvar.h
--- vinumvar.h	7 Jun 2007 22:58:38 -0000	1.11
+++ vinumvar.h	29 Jul 2007 23:20:58 -0000
@@ -433,8 +433,10 @@     } *freelist;
     struct partinfo partinfo;				    /* partition information */
 /* XXX kludge until we get this struct cleaned up */
 #if _KERNEL
-    cdev_t dev;						    /* device information */
+    struct vnode *vp;
+    struct cdev *dev;
 #else
+    char vp [sizeof (int *)];
     char dev [sizeof (int *)];
 #endif
 #ifdef VINUMDEBUG





More information about the Bugs mailing list