From owner-freebsd-bugs Thu Oct 3 8:30:17 2002 Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 42E1337B404 for ; Thu, 3 Oct 2002 08:30:12 -0700 (PDT) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 778C043E42 for ; Thu, 3 Oct 2002 08:30:11 -0700 (PDT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.12.6/8.12.6) with ESMTP id g93FUBCo087201 for ; Thu, 3 Oct 2002 08:30:11 -0700 (PDT) (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.12.6/8.12.6/Submit) id g93FUBGI087200; Thu, 3 Oct 2002 08:30:11 -0700 (PDT) Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id D1FE337B404 for ; Thu, 3 Oct 2002 08:23:27 -0700 (PDT) Received: from tranquility.caelum.net (adsl-66-124-157-122.dsl.sntc01.pacbell.net [66.124.157.122]) by mx1.FreeBSD.org (Postfix) with ESMTP id 1D87743E65 for ; Thu, 3 Oct 2002 08:23:27 -0700 (PDT) (envelope-from asaddi@tranquility.caelum.net) Received: by tranquility.caelum.net (Postfix, from userid 1000) id 8FA1635F; Thu, 3 Oct 2002 02:07:39 -0700 (PDT) Message-Id: <20021003090739.8FA1635F@tranquility.caelum.net> Date: Thu, 3 Oct 2002 02:07:39 -0700 (PDT) From: Allan Saddi Reply-To: Allan Saddi To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Subject: kern/43631: vinum: 'readpol prefer' option broken [patch included] Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org >Number: 43631 >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 08:30:10 PDT 2002 >Closed-Date: >Last-Modified: >Originator: Allan Saddi >Release: FreeBSD 4.6.2-RELEASE-p2 i386 >Organization: Philosophy SoftWorks >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 >Description: Volumes consisting of multiple plexes can be configured to prefer a specific plex for reading using the 'readpol prefer ' 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 in 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 ' 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