Date: Tue, 7 Aug 2007 15:18:31 GMT From: Ulf Lilleengen <lulf@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 124825 for review Message-ID: <200708071518.l77FIV71081742@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=124825 Change 124825 by lulf@lulf_carrot on 2007/08/07 15:18:24 - Added more on TODO. - Add more to manpage. - Use return values in gv_create_ functions! Gvinum would crash if the gv_create_ functions did not create their objects, which lead to dereferencing free()'d memory. Check for this error in gv_parse_config. Affected files ... .. //depot/projects/soc2007/lulf/TODO#7 edit .. //depot/projects/soc2007/lulf/gvinum_fixup/sbin/gvinum/gvinum.8#4 edit .. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum.h#25 edit .. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_create.c#7 edit .. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_subr.c#26 edit Differences ... ==== //depot/projects/soc2007/lulf/TODO#7 (text+ko) ==== @@ -190,4 +190,18 @@ flag which tells the system to expand the array when 'init' is run. Should there be a grow command perhaps? +Fixed... + + + + + + +25. Would it be nice with a way to block gvinum userland if we needed to? A way +to sleep on events... + +26. Documentation: + 1) Update handbook, add practical examples. + 2) Update manpage with more examples and perhaps better explanation of + commands. ==== //depot/projects/soc2007/lulf/gvinum_fixup/sbin/gvinum/gvinum.8#4 (text+ko) ==== @@ -171,7 +171,9 @@ Read configuration from all vinum drives. .It Ic start Oo Fl S Ar size Oc Ar volume | plex | subdisk Allow the system to access the objects. If necessary, plexes will be synced and -rebuilt. +rebuilt. If a subdisk was added to a running RAID-5 or striped plex, gvinum will +expand into this subdisk and grow the whole RAID-5 array. This can be done +without unmounting your filesystem. The .Fl S flag is currently ignored. ==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum.h#25 (text+ko) ==== @@ -120,10 +120,10 @@ void gv_bio_done(struct gv_softc *, struct bio *); void gv_cleanup(struct gv_softc *); -void gv_create_drive(struct gv_softc *, struct gv_drive *); -void gv_create_volume(struct gv_softc *, struct gv_volume *); -void gv_create_plex(struct gv_softc *, struct gv_plex *); -void gv_create_sd(struct gv_softc *, struct gv_sd *); +int gv_create_drive(struct gv_softc *, struct gv_drive *); +int gv_create_volume(struct gv_softc *, struct gv_volume *); +int gv_create_plex(struct gv_softc *, struct gv_plex *); +int gv_create_sd(struct gv_softc *, struct gv_sd *); int gv_stripe_active(struct gv_plex *, struct bio *); ==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_create.c#7 (text+ko) ==== @@ -41,7 +41,7 @@ * Create a new drive object, either by user request, during taste of the drive * itself, or because it was referenced by a subdisk during taste. */ -void +int gv_create_drive(struct gv_softc *sc, struct gv_drive *d) { struct g_geom *gp; @@ -67,7 +67,7 @@ if (gv_find_drive(sc, d->name) != NULL) { printf("VINUM: drive '%s' already exists\n", d->name); g_free(d); - return; + return (GV_ERR_CREATE); } pp = g_provider_by_name(d->device); @@ -75,7 +75,7 @@ printf("VINUM: create '%s': device '%s' disappeared?\n", d->name, d->device); g_free(d); - return; + return (GV_ERR_CREATE); } g_topology_lock(); @@ -86,7 +86,7 @@ printf("VINUM: create drive '%s': couldn't attach\n", d->name); g_free(d); - return; + return (GV_ERR_CREATE); } g_topology_unlock(); @@ -106,7 +106,7 @@ LIST_INSERT_HEAD(&sc->drives, d, drive); else LIST_INSERT_AFTER(d2, d, drive); - return; + return (0); } /* @@ -129,7 +129,7 @@ if (d->hdr != NULL) g_free(d->hdr); g_free(d); - return; + return (GV_ERR_CREATE); } g_topology_unlock(); break; @@ -163,9 +163,10 @@ LIST_INSERT_HEAD(&sc->drives, d, drive); gv_set_drive_state(d, GV_DRIVE_UP, 0); + return (0); } -void +int gv_create_volume(struct gv_softc *sc, struct gv_volume *v) { KASSERT(v != NULL, ("gv_create_volume: NULL v")); @@ -176,9 +177,10 @@ LIST_INSERT_HEAD(&sc->volumes, v, volume); v->wqueue = g_malloc(sizeof(struct bio_queue_head), M_WAITOK | M_ZERO); bioq_init(v->wqueue); + return (0); } -void +int gv_create_plex(struct gv_softc *sc, struct gv_plex *p) { struct gv_volume *v; @@ -191,7 +193,7 @@ printf("VINUM: create plex '%s': volume '%s' not found\n", p->name, p->volume); g_free(p); - return; + return (GV_ERR_CREATE); } if (!(v->flags & GV_VOL_NEWBORN)) p->flags |= GV_PLEX_ADDED; @@ -209,9 +211,10 @@ bioq_init(p->wqueue); p->rqueue = g_malloc(sizeof(struct bio_queue_head), M_WAITOK | M_ZERO); bioq_init(p->rqueue); + return (0); } -void +int gv_create_sd(struct gv_softc *sc, struct gv_sd *s) { struct gv_plex *p; @@ -237,7 +240,7 @@ printf("VINUM: create sd '%s': drive '%s' not found\n", s->name, s->drive); g_free(s); - return; + return (GV_ERR_CREATE); } } @@ -247,7 +250,7 @@ printf("VINUM: create sd '%s': plex '%s' not found\n", s->name, s->plex); g_free(s); - return; + return (GV_ERR_CREATE); } /* @@ -256,7 +259,7 @@ */ if (gv_sd_to_drive(s, d) != 0) { g_free(s); - return; + return (GV_ERR_CREATE); } /* @@ -275,14 +278,16 @@ * the attached plex either, if it is also a new one. */ if (!(p->flags & GV_PLEX_NEWBORN)) - return; + return (GV_ERR_CREATE); gv_rm_plex(sc, p); - return; + return (GV_ERR_CREATE); } s->flags |= GV_SD_NEWBORN; s->vinumconf = sc; LIST_INSERT_HEAD(&sc->subdisks, s, sd); + + return (0); } /* ==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_subr.c#26 (text+ko) ==== @@ -61,7 +61,7 @@ struct gv_volume *v, *v2; struct gv_plex *p, *p2; struct gv_sd *s, *s2; - int is_newer, tokens; + int error, is_newer, tokens; char *token[GV_MAXARGS]; is_newer = gv_drive_is_newer(sc); @@ -116,8 +116,9 @@ continue; } - gv_create_plex(sc, p); - + error = gv_create_plex(sc, p); + if (error) + continue; /* * These flags were set in gv_create_plex() and are not * needed here (on-disk config parsing). @@ -152,7 +153,9 @@ if (s->state == GV_SD_UP) s->flags |= GV_SD_CANGOUP; - gv_create_sd(sc, s); + error = gv_create_sd(sc, s); + if (error) + continue; /* * This flag was set in gv_create_sd() and is not @@ -160,7 +163,6 @@ */ s->flags &= ~GV_SD_NEWBORN; s->flags &= ~GV_SD_GROW; - printf("S-flags is now: %d\n", s->flags); } } }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200708071518.l77FIV71081742>