Date: Thu, 3 Oct 2002 02:35:11 -0700 (PDT) From: Allan Saddi <allan@saddi.com> To: FreeBSD-gnats-submit@FreeBSD.org Subject: kern/43623: vinum: 'readpol prefer' option broken [patch included] Message-ID: <20021003093511.4EAAF15A49@kalahari.flup.org>
next in thread | raw e-mail | index | archive | help
>Number: 43623
>Category: kern
>Synopsis: vinum: 'readpol prefer' option broken [patch included]
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Thu Oct 03 02:40:04 PDT 2002
>Closed-Date:
>Last-Modified:
>Originator: Allan Saddi
>Release: FreeBSD 4.6.2-RELEASE-p2 i386
>Organization:
Saddi Enterprises
>Environment:
System: FreeBSD tranquility.caelum.net 4.6.2-RELEASE-p2 FreeBSD 4.6.2-RELEASE-p2 #0: Thu Sep 26 11:42:50 PDT 2002 root@tranquility.caelum.net:/usr/obj/usr/src/sys/TRANQUILITY i386
<machine, os, target, libraries (multiple lines)>
>Description:
Volumes consisting of multiple plexes can be configured to prefer a
specific plex for reading using the 'readpol prefer <plexname>' option.
I found that this option is broken in 4.6.2 (and after viewing the
CVS history, -current and -stable as well). I identified a few
problems:
1. Inconsistent initialization of plex->plexno.
2. plex->plexno is wrong when defining an already referenced plex.
3. The config dump/print code incorrectly assume that
vol->preferred_plex is an index into the global plex array.
It's an index into the volume's plex array.
I corrected the problems in the following patch and 'readpol prefer'
now seems to work.
>How-To-Repeat:
Attempt to configure a volume using the 'readpol prefer <plexname>'
option. The vinum lv command will show the volume as having the wrong
number of plexes and/or it will include the wrong plex. Issuing
vinum dumpconfig will show a configuration that is different than
what you expect. vinum printconfig core dumps.
>Fix:
Also available at http://www.saddi.com/allan/tmp/vinum-readpol.diff
--- sys/dev/vinum/vinumconfig.c.orig Sat Feb 2 16:43:35 2002
+++ sys/dev/vinum/vinumconfig.c Sun Sep 29 03:37:51 2002
@@ -568,6 +568,7 @@
/* initialize some things */
sd = &SD[sdno]; /* point to it */
bzero(sd, sizeof(struct sd)); /* initialize */
+ sd->sdno = sdno; /* point back to config */
sd->flags |= VF_NEWBORN; /* newly born subdisk */
sd->plexno = -1; /* no plex */
sd->sectors = -1; /* no space */
@@ -760,6 +761,7 @@
/* Found a plex. Give it an sd structure */
plex = &PLEX[plexno]; /* this one is ours */
bzero(plex, sizeof(struct plex)); /* polish it up */
+ plex->plexno = plexno; /* point back to config */
plex->sdnos = (int *) Malloc(sizeof(int) * INITIAL_SUBDISKS_IN_PLEX); /* allocate sd table */
CHECKALLOC(plex->sdnos, "vinum: Can't allocate plex subdisk table");
bzero(plex->sdnos, (sizeof(int) * INITIAL_SUBDISKS_IN_PLEX)); /* do we need this? */
@@ -1074,7 +1076,9 @@
struct plex *plex; /* for tidying up dangling references */
*sd = SD[namedsdno]; /* copy from the referenced one */
+ sd->sdno = sdno; /* point back to config */
SD[namedsdno].state = sd_unallocated; /* and deallocate the referenced one */
+ SD[namedsdno].name[0] = '\0'; /* make sure no one finds it */
plex = &PLEX[sd->plexno]; /* now take a look at our plex */
for (i = 0; i < plex->subdisks; i++) { /* look for the pointer */
if (plex->sdnos[i] == namedsdno) /* pointing to the old subdisk */
@@ -1251,7 +1255,6 @@
current_plex = -1; /* forget the previous plex */
plexno = get_empty_plex(); /* allocate a plex */
plex = &PLEX[plexno]; /* and point to it */
- plex->plexno = plexno; /* and back to the config */
for (parameter = 1; parameter < tokens; parameter++) { /* look at the other tokens */
switch (get_keyword(token[parameter], &keyword_set)) {
@@ -1273,7 +1276,9 @@
struct volume *vol; /* for tidying up dangling references */
*plex = PLEX[namedplexno]; /* get the info */
+ plex->plexno = plexno; /* point back to config */
PLEX[namedplexno].state = plex_unallocated; /* and deallocate the other one */
+ PLEX[namedplexno].name[0] = '\0'; /* make sure no one finds it */
vol = &VOL[plex->volno]; /* point to the volume */
for (i = 0; i < MAXPLEX; i++) { /* for each plex */
if (vol->plex[i] == namedplexno)
@@ -1468,19 +1473,22 @@
case kw_prefer:
{
- int myplexno; /* index of this plex */
+ int plexno; /* index of this plex */
+ int myplexno; /* and index if it's already ours */
- myplexno = find_plex(token[++parameter], 1); /* find a plex */
- if (myplexno < 0) /* couldn't */
+ plexno = find_plex(token[++parameter], 1); /* find a plex */
+ if (plexno < 0) /* couldn't */
break; /* we've already had an error message */
- myplexno = my_plex(volno, myplexno); /* does it already belong to us? */
+ myplexno = my_plex(volno, plexno); /* does it already belong to us? */
if (myplexno > 0) /* yes */
vol->preferred_plex = myplexno; /* just note the index */
else if (++vol->plexes > 8) /* another entry */
throw_rude_remark(EINVAL, "Too many plexes");
else { /* space for the new plex */
- vol->plex[vol->plexes - 1] = myplexno; /* add it to our list */
+ vol->plex[vol->plexes - 1] = plexno; /* add it to our list */
vol->preferred_plex = vol->plexes - 1; /* and note the index */
+ PLEX[plexno].state = plex_referenced; /* we know something about it */
+ PLEX[plexno].volno = volno; /* and this volume references it */
}
}
break;
--- sys/dev/vinum/vinumio.c.orig Thu May 2 01:43:44 2002
+++ sys/dev/vinum/vinumio.c Sun Sep 29 02:02:29 2002
@@ -490,7 +490,7 @@
snprintf(s,
configend - s,
" readpol prefer %s",
- vinum_conf.plex[vol->preferred_plex].name);
+ vinum_conf.plex[vol->plex[vol->preferred_plex]].name);
while (*s)
s++; /* find the end */
s = sappend("\n", s);
--- sbin/vinum/list.c.orig Sun May 27 22:58:04 2001
+++ sbin/vinum/list.c Sun Sep 29 03:53:46 2002
@@ -1095,13 +1095,14 @@
for (i = 0; i < vinum_conf.volumes_allocated; i++) {
get_volume_info(&vol, i);
if (vol.state != volume_unallocated) {
- if (vol.preferred_plex >= 0) /* preferences, */
+ if (vol.preferred_plex >= 0) { /* preferences, */
+ get_plex_info(&plex, vol.plex[vol.preferred_plex]);
fprintf(of,
"%svolume %s readpol prefer %s\n",
comment,
vol.name,
- vinum_conf.plex[vol.preferred_plex].name);
- else /* default round-robin */
+ plex.name);
+ } else /* default round-robin */
fprintf(of, "%svolume %s\n", comment, vol.name);
}
}
>Release-Note:
>Audit-Trail:
>Unformatted:
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20021003093511.4EAAF15A49>