Date: Sat, 05 Oct 2002 20:33:40 +0100 From: Ian Dowse <iedowse@maths.tcd.ie> To: Poul-Henning Kamp <phk@critter.freebsd.dk> Cc: Robert Watson <rwatson@FreeBSD.ORG>, Greg 'groggy' Lehey <grog@FreeBSD.ORG>, current <freebsd-current@FreeBSD.ORG> Subject: Re: [ GEOM tests ] disklabel warnings and vinum drives lost Message-ID: <200210052033.aa95708@salmon.maths.tcd.ie> In-Reply-To: Your message of "Sat, 05 Oct 2002 08:15:04 %2B0200." <81130.1033798504@critter.freebsd.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
In message <81130.1033798504@critter.freebsd.dk>, Poul-Henning Kamp writes:
>Make that _three_ bugs: vinum opens devices directly at the cdevsw
>level, bypassing in the process the vnodes and specfs.
Here is a patch that makes it use vn_open/vn_close/VOP_IOCTL,
bringing it much closer to the way ccd(4) does things. I have only
lightly tested this so far - I saw one problem where a md(4)
vnode-backed device got stuck in mddestroy(), but I haven't tracked
down if that is related (the md vnode was just a file on a vinum-backed
filesystem).
Ian
Index: vinumio.c
===================================================================
RCS file: /dump/FreeBSD-CVS/src/sys/dev/vinum/vinumio.c,v
retrieving revision 1.76
diff -u -r1.76 vinumio.c
--- vinumio.c 5 Oct 2002 03:44:00 -0000 1.76
+++ vinumio.c 5 Oct 2002 14:12:56 -0000
@@ -51,33 +51,26 @@
open_drive(struct drive *drive, struct thread *td, int verbose)
{
struct nameidata nd;
- struct cdevsw *dsw; /* pointer to cdevsw entry */
- int error;
+ int flags;
if (drive->flags & VF_OPEN) /* open already, */
return EBUSY; /* don't do it again */
- NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, drive->devicename,
- curthread);
- error = namei(&nd);
- if (error)
- return (error);
- if (!vn_isdisk(nd.ni_vp, &error)) {
- NDFREE(&nd, 0);
- return (error);
- }
- drive->dev = udev2dev(nd.ni_vp->v_rdev->si_udev, 0);
- NDFREE(&nd, 0);
-
- if (drive->dev == NULL) /* didn't find anything */
- return ENODEV;
-
- drive->dev->si_iosize_max = DFLTPHYS;
- dsw = devsw(drive->dev);
- if (dsw == NULL)
- drive->lasterror = ENOENT;
- else
- drive->lasterror = (dsw->d_open) (drive->dev, FWRITE | FREAD, 0, NULL);
+ drive->devvp = NULL;
+ NDINIT(&nd, LOOKUP, FOLLOW, UIO_SYSSPACE, drive->devicename, td);
+ flags = FREAD | FWRITE;
+ drive->lasterror = vn_open(&nd, &flags, 0);
+ if (drive->lasterror == 0) {
+ (void) vn_isdisk(nd.ni_vp, &drive->lasterror);
+ if (drive->lasterror != 0 && vrefcnt(nd.ni_vp) > 1)
+ drive->lasterror = EBUSY;
+ VOP_UNLOCK(nd.ni_vp, 0, td);
+ NDFREE(&nd, NDF_ONLY_PNBUF);
+ if (drive->lasterror == 0)
+ drive->devvp = nd.ni_vp;
+ else
+ (void) vn_close(nd.ni_vp, flags, td->td_ucred, td);
+ }
if (drive->lasterror != 0) { /* failed */
drive->state = drive_down; /* just force it down */
@@ -85,8 +78,11 @@
log(LOG_WARNING,
"vinum open_drive %s: failed with error %d\n",
drive->devicename, drive->lasterror);
- } else
+ } else {
+ drive->dev = vn_todev(drive->devvp);
+ drive->dev->si_iosize_max = DFLTPHYS;
drive->flags |= VF_OPEN; /* we're open now */
+ }
return drive->lasterror;
}
@@ -145,6 +141,9 @@
int
init_drive(struct drive *drive, int verbose)
{
+ struct thread *td;
+
+ td = curthread;
if (drive->devicename[0] != '/') {
drive->lasterror = EINVAL;
log(LOG_ERR, "vinum: Can't open drive without drive name\n");
@@ -154,17 +153,17 @@
if (drive->lasterror)
return drive->lasterror;
- drive->lasterror = (*devsw(drive->dev)->d_ioctl) (drive->dev,
+ drive->lasterror = VOP_IOCTL(drive->devvp,
DIOCGSECTORSIZE,
(caddr_t) & drive->sectorsize,
FREAD,
- curthread);
+ td->td_ucred, td);
if (drive->lasterror == 0)
- drive->lasterror = (*devsw(drive->dev)->d_ioctl) (drive->dev,
+ drive->lasterror = VOP_IOCTL(drive->devvp,
DIOCGMEDIASIZE,
(caddr_t) & drive->mediasize,
FREAD,
- curthread);
+ td->td_ucred, td);
if (drive->lasterror) {
if (verbose)
log(LOG_WARNING,
@@ -211,14 +210,16 @@
void
close_locked_drive(struct drive *drive)
{
+ struct thread *td;
int error;
+ td = curthread;
/*
* If we can't access the drive, we can't flush
* the queues, which spec_close() will try to
* do. Get rid of them here first.
*/
- error = (*devsw(drive->dev)->d_close) (drive->dev, 0, 0, NULL);
+ error = vn_close(drive->devvp, FREAD | FWRITE, td->td_ucred, td);
drive->flags &= ~VF_OPEN; /* no longer open */
if (drive->lasterror == 0)
drive->lasterror = error;
@@ -561,11 +562,13 @@
int error;
int written_config; /* set when we first write the config to disk */
int driveno;
+ struct thread *td;
struct drive *drive; /* point to current drive info */
struct vinum_hdr *vhdr; /* and as header */
char *config; /* point to config data */
int wlabel_on; /* to set writing label on/off */
+ td = curthread;
/* don't save the configuration while we're still working on it */
if (vinum_conf.flags & VF_CONFIGURING)
return;
@@ -615,22 +618,22 @@
if ((drive->state != drive_unallocated)
&& (drive->state != drive_referenced)) { /* and it's a real drive */
wlabel_on = 1; /* enable writing the label */
- (void) (*devsw(drive->dev)->d_ioctl) (drive->dev, /* make the label writeable */
+ (void) VOP_IOCTL(drive->devvp, /* make the label writeable */
DIOCWLABEL,
(caddr_t) & wlabel_on,
FWRITE,
- curthread);
+ td->td_ucred, td);
error = write_drive(drive, (char *) vhdr, VINUMHEADERLEN, VINUM_LABEL_OFFSET);
if (error == 0)
error = write_drive(drive, config, MAXCONFIG, VINUM_CONFIG_OFFSET); /* first config copy */
if (error == 0)
error = write_drive(drive, config, MAXCONFIG, VINUM_CONFIG_OFFSET + MAXCONFIG); /* second copy */
wlabel_on = 0; /* enable writing the label */
- (void) (*devsw(drive->dev)->d_ioctl) (drive->dev, /* make the label non-writeable again */
+ (void) VOP_IOCTL(drive->devvp, /* make the label non-writeable again */
DIOCWLABEL,
(caddr_t) & wlabel_on,
FWRITE,
- curthread);
+ td->td_ucred, td);
unlockdrive(drive);
if (error) {
log(LOG_ERR,
Index: vinumobj.h
===================================================================
RCS file: /dump/FreeBSD-CVS/src/sys/dev/vinum/vinumobj.h,v
retrieving revision 1.6
diff -u -r1.6 vinumobj.h
--- vinumobj.h 16 May 2002 21:23:45 -0000 1.6
+++ vinumobj.h 5 Oct 2002 14:07:47 -0000
@@ -178,6 +178,7 @@
#ifdef _KERNEL
u_int sectorsize;
off_t mediasize;
+ struct vnode *devvp; /* device vnode */
dev_t dev; /* device information */
#ifdef VINUMDEBUG
char lockfilename[16]; /* name of file from which we were locked */
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200210052033.aa95708>