Date: Wed, 9 Jul 2008 17:40:05 GMT From: Ulf Lilleengen <lulf@stud.ntnu.no> To: freebsd-geom@FreeBSD.org Subject: Re: kern/124969: gvinum(8): gvinum raid5 plex does not detect missing subdisk Message-ID: <200807091740.m69He5M6056322@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/124969; it has been noted by GNATS.
From: Ulf Lilleengen <lulf@stud.ntnu.no>
To: bug-followup@FreeBSD.org, drkp-f@ambulatoryclam.net
Cc:
Subject: Re: kern/124969: gvinum(8): gvinum raid5 plex does not detect
missing subdisk
Date: Wed, 9 Jul 2008 19:38:24 +0200
--gBBFr7Ir9EOA20Yy
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Hello,
I think I managed to solve the issue, at least it works in CURRENT. This
patch is for RELENG_6, but I've not been able to test it. Be careful, I make
no guarantees of what it will do :) However, it doesn't touch any data, just
handles some extra fields and flags in the structures. I ended up making it
possible to have "referenced" drives (drives with no real device), as well as
subdisks that didn't have a "real" drive yet. Also, I fixed some issues with
the plex states when things are tasted, but please be careful and watch the
states and make sure they are what they "should" be (E.g. a failed drive that
is returning should neved have the UP state, since it must be synchronized
first).
I even found a "real" bug in CURRENT while making this patch as well. Please
tell me how it fares.
--
Ulf Lilleengen
--gBBFr7Ir9EOA20Yy
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="gvinum_detect_down_disk.diff"
Index: sys/geom/vinum/geom_vinum_drive.c
===================================================================
--- sys/geom/vinum/geom_vinum_drive.c (revision 180387)
+++ sys/geom/vinum/geom_vinum_drive.c (working copy)
@@ -476,6 +476,13 @@
*/
d = gv_find_drive(sc, vhdr->label.name);
+ /* If it's referenced, remove it before we re-create it. */
+ if (d != NULL && d->flags & GV_DRIVE_REFERENCED) {
+ LIST_REMOVE(d, drive);
+ g_free(d);
+ d = NULL;
+ }
+
/* We already know about this drive. */
if (d != NULL) {
/* Check if this drive already has a geom. */
@@ -536,10 +543,12 @@
* them.
*/
LIST_FOREACH(s, &sc->subdisks, sd) {
- if (!strncmp(s->drive, d->name, GV_MAXDRIVENAME))
+ if (!strncmp(s->drive, d->name, GV_MAXDRIVENAME)) {
/* XXX: errors ignored */
gv_sd_to_drive(sc, d, s, errstr,
sizeof(errstr));
+ s->flags &= ~GV_SD_NODRIVE;
+ }
}
/* This drive is now up for sure. */
Index: sys/geom/vinum/geom_vinum_list.c
===================================================================
--- sys/geom/vinum/geom_vinum_list.c (revision 180387)
+++ sys/geom/vinum/geom_vinum_list.c (working copy)
@@ -314,6 +314,8 @@
i = 0;
LIST_FOREACH(s, &p->subdisks, in_plex) {
sbuf_printf(sb, "\t\tSubdisk %d:\t%s\n", i, s->name);
+ if (s->flags & GV_SD_NODRIVE)
+ continue;
sbuf_printf(sb, "\t\t state: %s\tsize %11jd "
"(%jd MB)\n", gv_sdstate(s->state),
(intmax_t)s->size, (intmax_t)s->size / MEGABYTE);
@@ -361,6 +363,12 @@
{
if (flags & GV_FLAG_V) {
sbuf_printf(sb, "Subdisk %s:\n", s->name);
+ if (s->flags & GV_SD_NODRIVE) {
+ sbuf_printf(sb, "\t\tSize: 0 bytes (0 MB)\n");
+ sbuf_printf(sb, "\t\tState: %s\n",
+ gv_sdstate(s->state));
+ return;
+ }
sbuf_printf(sb, "\t\tSize: %16jd bytes (%jd MB)\n",
(intmax_t)s->size, (intmax_t)s->size / MEGABYTE);
sbuf_printf(sb, "\t\tState: %s\n", gv_sdstate(s->state));
@@ -390,6 +398,11 @@
gv_roughlength(s->drive_offset, 1));
} else {
sbuf_printf(sb, "S %-21s State: ", s->name);
+ if (s->flags & GV_SD_NODRIVE) {
+ sbuf_printf(sb, "%s\t", gv_sdstate(s->state));
+ sbuf_printf(sb, "D: %-12s Size: 0\n", s->drive);
+ return;
+ }
if (s->state == GV_SD_INITIALIZING ||
s->state == GV_SD_REVIVING) {
if (s->state == GV_SD_INITIALIZING)
@@ -439,6 +452,8 @@
/* Verbose listing. */
if (flags & GV_FLAG_V) {
sbuf_printf(sb, "Drive %s:\tDevice %s\n", d->name, d->device);
+ if (d->flags & GV_DRIVE_REFERENCED)
+ return;
sbuf_printf(sb, "\t\tSize: %16jd bytes (%jd MB)\n",
(intmax_t)d->size, (intmax_t)d->size / MEGABYTE);
sbuf_printf(sb, "\t\tUsed: %16jd bytes (%jd MB)\n",
@@ -458,8 +473,13 @@
(intmax_t)fl->offset, (intmax_t)fl->size);
}
} else {
- sbuf_printf(sb, "D %-21s State: %s\t/dev/%s\tA: %jd/%jd MB "
- "(%d%%)\n", d->name, gv_drivestate(d->state), d->device,
+ sbuf_printf(sb, "D %-21s State: %s\t/dev/%s\t", d->name,
+ gv_drivestate(d->state), d->device);
+ if (d->flags & GV_DRIVE_REFERENCED) {
+ sbuf_printf(sb, "\n");
+ return;
+ }
+ sbuf_printf(sb, "A: %jd/%jd MB (%d%%)\n",
(intmax_t)d->avail / MEGABYTE, (intmax_t)d->size / MEGABYTE,
(int)((d->avail * 100) / d->size));
}
Index: sys/geom/vinum/geom_vinum_subr.c
===================================================================
--- sys/geom/vinum/geom_vinum_subr.c (revision 180387)
+++ sys/geom/vinum/geom_vinum_subr.c (working copy)
@@ -87,6 +87,7 @@
struct gv_volume *v, *v2;
struct gv_plex *p, *p2;
struct gv_sd *s, *s2;
+ struct gv_drive *d;
int tokens;
char *token[GV_MAXARGS];
@@ -144,6 +145,7 @@
p->vinumconf = sc;
LIST_INIT(&p->subdisks);
+ LIST_INIT(&p->delayed_sd);
LIST_INSERT_HEAD(&sc->plexes, p, plex);
} else if (!strcmp(token[0], "sd")) {
@@ -154,6 +156,28 @@
break;
}
+ /* The drive might not exist. */
+ d = gv_find_drive(sc, s->drive);
+ if (d == NULL) {
+ s->flags |= GV_SD_NODRIVE;
+ s->state = GV_SD_DOWN;
+ d = g_malloc(sizeof(struct gv_drive),
+ M_WAITOK | M_ZERO);
+ d->state = GV_DRIVE_DOWN;
+ d->flags |= GV_DRIVE_REFERENCED;
+ strlcpy(d->name, s->drive,
+ GV_MAXDRIVENAME);
+ s->drive_sc = d;
+ snprintf(d->device, GV_MAXDRIVENAME,
+ "???");
+ LIST_INSERT_HEAD(&sc->drives, d, drive);
+ p = gv_find_plex(sc, s->plex);
+ if (p != NULL) {
+ /* Register delayed plex. */
+ LIST_INSERT_HEAD(&p->delayed_sd,
+ s, in_plex_delay);
+ }
+ }
if (merge) {
s2 = gv_find_sd(sc, s->name);
if (s2 != NULL) {
@@ -236,9 +260,10 @@
}
int
-gv_sd_to_plex(struct gv_plex *p, struct gv_sd *s, int check)
+gv_sd_to_plex(struct gv_plex *p, struct gv_sd *s, int check, int taste)
{
struct gv_sd *s2;
+ struct gv_drive *d;
g_topology_assert();
@@ -246,6 +271,13 @@
if (s->plex_sc == p)
return (0);
+ /* If we're coming from a taste, mark the subdisk up if drive is up. */
+ if (taste) {
+ d = gv_find_drive(p->vinumconf, s->drive);
+ if (d->state = GV_DRIVE_UP)
+ gv_set_sd_state(s, GV_SD_UP, GV_SETSTATE_FORCE);
+ }
+
/* Find the correct plex offset for this subdisk, if needed. */
if (s->plex_offset == -1) {
if (p->sdcount) {
@@ -325,7 +357,7 @@
{
struct gv_sd *s, *s2;
off_t remainder;
- int required_sds, state;
+ int required_sds, state, sdcount;
KASSERT(p != NULL, ("gv_update_plex_config: NULL p"));
@@ -349,10 +381,18 @@
break;
}
+ sdcount = p->sdcount;
+ LIST_FOREACH(s2, &p->subdisks, in_plex) {
+ if (s2->flags & GV_SD_NODRIVE)
+ sdcount--;
+ }
if (required_sds) {
- if (p->sdcount < required_sds) {
+ if (sdcount < required_sds) {
state = GV_PLEX_DOWN;
}
+ if ((sdcount == (required_sds - 1)) &&
+ (p->org == GV_PLEX_RAID5)) {
+ state = GV_PLEX_DEGRADED;
/*
* The subdisks in striped plexes must all have the same size.
@@ -412,8 +452,9 @@
s->state = GV_SD_STALE;
p->flags &= ~GV_PLEX_ADDED;
p->flags &= ~GV_PLEX_NEWBORN;
- p->state = GV_PLEX_DOWN;
+ state = GV_PLEX_DOWN;
}
+ p->state = state;
}
/*
@@ -441,7 +482,7 @@
errlen));
/* Check if this subdisk was already given to this drive. */
- if (s->drive_sc == d)
+ if (s->drive_sc == d && !(s->flags & GV_SD_NODRIVE))
return (0);
/* Preliminary checks. */
Index: sys/geom/vinum/geom_vinum.c
===================================================================
--- sys/geom/vinum/geom_vinum.c (revision 180387)
+++ sys/geom/vinum/geom_vinum.c (working copy)
@@ -248,6 +248,7 @@
p->vinumconf = sc;
p->flags |= GV_PLEX_NEWBORN;
LIST_INIT(&p->subdisks);
+ LIST_INIT(&p->delayed_sd);
LIST_INSERT_HEAD(&sc->plexes, p, plex);
}
@@ -302,7 +303,7 @@
* Then, we give the subdisk to the plex; we check if the
* given values are correct and maybe adjust them.
*/
- error = gv_sd_to_plex(p, s, 1);
+ error = gv_sd_to_plex(p, s, 1, 0);
if (error) {
printf("FOO: couldn't give sd '%s' to plex '%s'\n",
s->name, p->name);
Index: sys/geom/vinum/geom_vinum_state.c
===================================================================
--- sys/geom/vinum/geom_vinum_state.c (revision 180387)
+++ sys/geom/vinum/geom_vinum_state.c (working copy)
@@ -171,7 +171,8 @@
case GV_SD_UP:
/* We can't bring the subdisk up if our drive is dead. */
d = s->drive_sc;
- if ((d == NULL) || (d->state != GV_DRIVE_UP))
+ if ((d == NULL) || (d->state != GV_DRIVE_UP) ||
+ (d->flags & GV_DRIVE_REFERENCED))
return (-1);
/* Check from where we want to be brought up. */
@@ -276,6 +277,8 @@
s->flags &= ~GV_SD_NEWBORN;
} else if (s->state != GV_SD_UP)
s->state = GV_SD_STALE;
+ else if (s->flags & GV_SD_NODRIVE)
+ s->state = GV_SD_DOWN;
else
s->state = GV_SD_UP;
@@ -391,6 +394,10 @@
p->sddown++; /* XXX: Another unusable subdisk? */
break;
}
+ if (s->flags & GV_SD_NODRIVE)
+ p->sddown++;
}
+ LIST_FOREACH(s, &p->delayed_sd, in_plex_delay)
+ p->sddown++;
return (statemap);
}
Index: sys/geom/vinum/geom_vinum.h
===================================================================
--- sys/geom/vinum/geom_vinum.h (revision 180387)
+++ sys/geom/vinum/geom_vinum.h (working copy)
@@ -86,7 +86,7 @@
void gv_parse_config(struct gv_softc *, u_char *, int);
int gv_sd_to_drive(struct gv_softc *, struct gv_drive *, struct gv_sd *,
char *, int);
-int gv_sd_to_plex(struct gv_plex *, struct gv_sd *, int);
+int gv_sd_to_plex(struct gv_plex *, struct gv_sd *, int, int);
void gv_update_plex_config(struct gv_plex *);
void gv_update_vol_size(struct gv_volume *, off_t);
Index: sys/geom/vinum/geom_vinum_var.h
===================================================================
--- sys/geom/vinum/geom_vinum_var.h (revision 180387)
+++ sys/geom/vinum/geom_vinum_var.h (working copy)
@@ -190,6 +190,7 @@
#define GV_DRIVE_THREAD_DIE 0x02 /* Signal the worker thread to die. */
#define GV_DRIVE_THREAD_DEAD 0x04 /* The worker thread has died. */
#define GV_DRIVE_NEWBORN 0x08 /* The drive was just created. */
+#define GV_DRIVE_REFERENCED 0x10 /* The drive does not yet exist. */
struct gv_hdr *hdr; /* The drive header. */
@@ -226,6 +227,7 @@
int flags;
#define GV_SD_NEWBORN 0x01 /* Subdisk was just created. */
#define GV_SD_INITCANCEL 0x02 /* Cancel initialization process. */
+#define GV_SD_NODRIVE 0x04 /* The subdisk does'nt have a drive. */
char drive[GV_MAXDRIVENAME]; /* Name of underlying drive. */
char plex[GV_MAXPLEXNAME]; /* Name of associated plex. */
@@ -239,6 +241,7 @@
LIST_ENTRY(gv_sd) from_drive; /* Subdisk list of underlying drive. */
LIST_ENTRY(gv_sd) in_plex; /* Subdisk list of associated plex. */
LIST_ENTRY(gv_sd) sd; /* Entry in the vinum config. */
+ LIST_ENTRY(gv_sd) in_plex_delay; /* Delayed entry. */
struct gv_softc *vinumconf; /* Pointer to the vinum config. */
};
@@ -281,6 +284,7 @@
TAILQ_HEAD(,gv_bioq) wqueue; /* Waiting BIO queue. */
TAILQ_HEAD(,gv_raid5_packet) packets; /* RAID5 sub-requests. */
+ LIST_HEAD(,gv_sd) delayed_sd; /* List of delayed subdisks. */
LIST_HEAD(,gv_sd) subdisks; /* List of attached subdisks. */
LIST_ENTRY(gv_plex) in_volume; /* Plex list of associated volume. */
LIST_ENTRY(gv_plex) plex; /* Entry in the vinum config. */
Index: sys/geom/vinum/geom_vinum_move.c
===================================================================
--- sys/geom/vinum/geom_vinum_move.c (revision 180387)
+++ sys/geom/vinum/geom_vinum_move.c (working copy)
@@ -190,7 +190,7 @@
}
}
- gv_sd_to_plex(p, newsd, 1);
+ gv_sd_to_plex(p, newsd, 1, 0);
/* Creates the new providers.... */
gv_drive_modify(d);
Index: sys/geom/vinum/geom_vinum_plex.c
===================================================================
--- sys/geom/vinum/geom_vinum_plex.c (revision 180387)
+++ sys/geom/vinum/geom_vinum_plex.c (working copy)
@@ -273,7 +273,7 @@
{
struct bio *bp;
struct gv_plex *p;
- struct gv_sd *s;
+ struct gv_sd *s, *s2;
struct gv_bioq *bq;
p = arg;
@@ -285,6 +285,17 @@
if (p->flags & GV_PLEX_THREAD_DIE)
break;
+ /* First make sure we got all subdisks. */
+ LIST_FOREACH_SAFE(s, &p->delayed_sd, in_plex_delay, s2) {
+ if (s->flags & GV_SD_NODRIVE) {
+ gv_set_sd_state(s, GV_SD_STALE,
+ GV_SETSTATE_FORCE);
+ }
+ gv_sd_to_plex(p, s, 0, 0);
+ LIST_REMOVE(s, in_plex_delay);
+ gv_update_plex_config(p);
+ }
+
/* Take the first BIO from our queue. */
bq = TAILQ_FIRST(&p->bqueue);
if (bq == NULL) {
@@ -737,7 +748,7 @@
* configuration, we don't check the given value (should we?).
* XXX: shouldn't be done here
*/
- gv_sd_to_plex(p, s, 0);
+ gv_sd_to_plex(p, s, 0, 1);
/* Now check if there's already a geom for this plex. */
gp = p->geom;
--gBBFr7Ir9EOA20Yy--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200807091740.m69He5M6056322>