From 83d36d43664342d09f00543be9aaf79e97ac9339 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Fri, 16 Sep 2005 04:33:14 +0000 Subject: [PATCH] Cleanup a couple of serious issues with vinum. * 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 | 4 +- sys/dev/raid/vinum/vinuminterrupt.c | 4 +- sys/dev/raid/vinum/vinumio.c | 139 +++++++++++++++++----------- 3 files changed, 89 insertions(+), 58 deletions(-) diff --git a/sys/dev/raid/vinum/vinumconfig.c b/sys/dev/raid/vinum/vinumconfig.c index 7aba00d41f..8c61937dd3 100644 --- a/sys/dev/raid/vinum/vinumconfig.c +++ b/sys/dev/raid/vinum/vinumconfig.c @@ -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, diff --git a/sys/dev/raid/vinum/vinuminterrupt.c b/sys/dev/raid/vinum/vinuminterrupt.c index 2f6357db46..9d3ac0f97f 100644 --- a/sys/dev/raid/vinum/vinuminterrupt.c +++ b/sys/dev/raid/vinum/vinuminterrupt.c @@ -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 */ diff --git a/sys/dev/raid/vinum/vinumio.c b/sys/dev/raid/vinum/vinumio.c index c8fdc06efb..b4ed4f1714 100644 --- a/sys/dev/raid/vinum/vinumio.c +++ b/sys/dev/raid/vinum/vinumio.c @@ -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++; + } + } } } -- 2.41.0