PERFORCE change 120427 for review

Ulf Lilleengen lulf at FreeBSD.org
Sat May 26 13:26:40 UTC 2007


http://perforce.freebsd.org/chv.cgi?CH=120427

Change 120427 by lulf at lulf_carrot on 2007/05/26 13:26:35

	- Add support for 'start <plex>' command again with the new event
	  system. However, only rebuilding degraded plexes is possible at the
	  time. To test this i need attach/detach routines, so I'm going to work
	  on this next.
	- Remove weird comments and put comments where they should be.
	- Rework the gv_issue_next* to be called gv_send_parity_bio, and change
	  it parameters so that it can be used for both user initiated rebuild
	  and on rebuilding a degraded plex.

Affected files ...

.. //depot/projects/soc2007/lulf/gvinum_fixup/sbin/gvinum/gvinum.c#5 edit
.. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum.c#12 edit
.. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum.h#10 edit
.. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_init.c#4 edit
.. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_plex.c#10 edit
.. //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_var.h#10 edit

Differences ...

==== //depot/projects/soc2007/lulf/gvinum_fixup/sbin/gvinum/gvinum.c#5 (text+ko) ====


==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum.c#12 (text+ko) ====

@@ -348,11 +348,9 @@
 	} else if (!strcmp(verb, "resetconfig")) {
 		gv_post_event(sc, GV_EVENT_RESET_CONFIG, sc, NULL, NULL);
 
-#if 0
 	} else if (!strcmp(verb, "start")) {
 		gv_start_obj(gp, req);
 
-#endif
 	} else if (!strcmp(verb, "setstate")) {
 		gv_setstate(gp, req);
 	} else
@@ -570,23 +568,46 @@
 				break;
 
 			case GV_EVENT_PARITY_REBUILD:
-				/* Start parity check. */
+				/* 
+				 * Start the rebuild. The gv_plex_done will
+				 * handle issuing of the remaining rebuild bio's
+				 * until it's finished. 
+				 */
+				printf("VINUM: event 'rebuild'\n");
 				p = ev->arg1;
+				if (p->state != GV_PLEX_UP) {
+					printf("VINUM: plex %s is not "
+					    "completely accessible", p->name);
+					break;
+				}
 				p->synced = 0;
-				gv_issue_next_parity_bio(p, 0);
+				gv_send_parity_bio(p, GV_BIO_CHECK |
+				    GV_BIO_PARITY, 0);
 				break;
 
 			case GV_EVENT_PARITY_CHECK:
-				printf("VINUM: event 'rebuild'\n");
-				/* 
-				 * Start the rebuild. The gv_plex_done will
-				 * handle issuing of the remaining rebuild bio's
-				 * until it's finished. 
-				 */
-				/* XXX: Should check plex state here. */
+				/* Start parity check. */
+				printf("VINUM: event 'check'\n");
 				p = ev->arg1;
+				if (p->state != GV_PLEX_UP) {
+					printf("VINUM: plex %s is not "
+					    "completely accessible", p->name);
+					break;
+				}
 				p->synced = 0;
-				gv_issue_next_parity_bio(p, 1);
+				gv_send_parity_bio(p, GV_BIO_CHECK, 0);
+				break;
+
+			case GV_EVENT_START_PLEX:
+				printf("VINUM: event 'start'\n");
+				p = ev->arg1;
+				gv_start_plex(p);
+				break;
+
+			case GV_EVENT_START_VOLUME:
+				printf("VINUM: event 'start'\n");
+				v = ev->arg1;
+				/*gv_start_volume(v);*/
 				break;
 
 			case GV_EVENT_THREAD_EXIT:

==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum.h#10 (text+ko) ====

@@ -35,6 +35,7 @@
 /* geom_vinum_init.c */
 /*void	gv_parityop(struct g_geom *, struct gctl_req *);*/
 void	gv_start_obj(struct g_geom *, struct gctl_req *);
+int	gv_start_plex(struct gv_plex *);
 
 /* geom_vinum_list.c */
 void	gv_ld(struct g_geom *, struct gctl_req *, struct sbuf *);
@@ -112,7 +113,7 @@
 
 int	gv_stripe_active(struct gv_plex *, struct bio *);
 
-void	gv_issue_next_parity_bio(struct gv_plex *, int);
+void	gv_send_parity_bio(struct gv_plex *, int, off_t);
 void	gv_parityop(struct gv_softc *, struct gctl_req *);
 
 #endif /* !_GEOM_VINUM_H_ */

==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_init.c#4 (text+ko) ====

@@ -40,15 +40,9 @@
 #include <geom/vinum/geom_vinum.h>
 #include <geom/vinum/geom_vinum_share.h>
 
-#if 0
-static int	gv_init_plex(struct gv_plex *);
-void	gv_init_td(void *);
-static int	gv_rebuild_plex(struct gv_plex *);
-void	gv_rebuild_td(void *);
-static int	gv_start_plex(struct gv_plex *);
 static int	gv_start_vol(struct gv_volume *);
 static int	gv_sync(struct gv_volume *);
-void	gv_sync_td(void *);
+static int	gv_rebuild_plex(struct gv_plex *);
 
 struct gv_sync_args {
 	struct gv_volume *v;
@@ -58,131 +52,6 @@
 };
 
 void
-gv_parityop(struct g_geom *gp, struct gctl_req *req)
-{
-	struct gv_softc *sc;
-	struct gv_plex *p;
-	struct bio *bp;
-	struct g_consumer *cp;
-	int error, *flags, type, *rebuild, rv;
-	char *plex;
-
-	rv = -1;
-
-	plex = gctl_get_param(req, "plex", NULL);
-	if (plex == NULL) {
-		gctl_error(req, "no plex given");
-		goto out;
-	}
-
-	flags = gctl_get_paraml(req, "flags", sizeof(*flags));
-	if (flags == NULL) {
-		gctl_error(req, "no flags given");
-		goto out;
-	}
-
-	rebuild = gctl_get_paraml(req, "rebuild", sizeof(*rebuild));
-	if (rebuild == NULL) {
-		gctl_error(req, "no rebuild op given");
-		goto out;
-	}
-
-	sc = gp->softc;
-	type = gv_object_type(sc, plex);
-	switch (type) {
-	case GV_TYPE_PLEX:
-		break;
-	case GV_TYPE_VOL:
-	case GV_TYPE_SD:
-	case GV_TYPE_DRIVE:
-	default:
-		gctl_error(req, "'%s' is not a plex", plex);
-		goto out;
-	}
-
-	p = gv_find_plex(sc, plex);
-	if (p->state != GV_PLEX_UP) {
-		gctl_error(req, "plex %s is not completely accessible",
-		    p->name);
-		goto out;
-	}
-	if (p->org != GV_PLEX_RAID5) {
-		gctl_error(req, "plex %s is not a RAID5 plex", p->name);
-		goto out;
-	}
-
-	cp = p->consumer;
-	error = g_access(cp, 1, 1, 0);
-	if (error) {
-		gctl_error(req, "cannot access consumer");
-		goto out;
-	}
-	g_topology_unlock();
-
-	/* Reset the check pointer when using -f. */
-	if (*flags & GV_FLAG_F)
-		p->synced = 0;
-
-	bp = g_new_bio();
-	if (bp == NULL) {
-		gctl_error(req, "cannot create BIO - out of memory");
-		g_topology_lock();
-		error = g_access(cp, -1, -1, 0);
-		goto out;
-	}
-	bp->bio_cmd = BIO_WRITE;
-	bp->bio_done = NULL;
-	bp->bio_data = g_malloc(p->stripesize, M_WAITOK | M_ZERO);
-	bp->bio_cflags |= GV_BIO_CHECK;
-	if (*rebuild)
-		bp->bio_cflags |= GV_BIO_PARITY;
-	bp->bio_offset = p->synced;
-	bp->bio_length = p->stripesize;
-
-	/* Schedule it down ... */
-	g_io_request(bp, cp);
-
-	/* ... and wait for the result. */
-	error = biowait(bp, "gwrite");
-	g_free(bp->bio_data);
-	g_destroy_bio(bp);
-
-	if (error) {
-		/* Incorrect parity. */
-		if (error == EAGAIN)
-			rv = 1;
-
-		/* Some other error happened. */
-		else
-			gctl_error(req, "Parity check failed at offset 0x%jx, "
-			    "errno %d", (intmax_t)p->synced, error);
-
-	/* Correct parity. */
-	} else
-		rv = 0;
-
-	gctl_set_param(req, "offset", &p->synced, sizeof(p->synced));
-
-	/* Advance the checkpointer if there was no error. */
-	if (rv == 0)
-		p->synced += p->stripesize;
-
-	/* End of plex; reset the check pointer and signal it to the caller. */
-	if (p->synced >= p->size) {
-		p->synced = 0;
-		rv = -2;
-	}
-
-	g_topology_lock();
-	error = g_access(cp, -1, -1, 0);
-
-	/* XXX: Unlock? */
-
-out:
-	gctl_set_param(req, "rv", &rv, sizeof(rv));
-}
-
-void
 gv_start_obj(struct g_geom *gp, struct gctl_req *req)
 {
 	struct gv_softc *sc;
@@ -190,7 +59,7 @@
 	struct gv_plex *p;
 	int *argc, *initsize;
 	char *argv, buf[20];
-	int err, i, type;
+	int i, type;
 
 	argc = gctl_get_paraml(req, "argc", sizeof(*argc));
 	initsize = gctl_get_paraml(req, "initsize", sizeof(*initsize));
@@ -211,32 +80,16 @@
 		switch (type) {
 		case GV_TYPE_VOL:
 			v = gv_find_vol(sc, argv);
-			err = gv_start_vol(v);
-			if (err) {
-				if (err == EINPROGRESS) {
-					gctl_error(req, "cannot start volume "
-					    "'%s': already in progress", argv);
-				} else {
-					gctl_error(req, "cannot start volume "
-					    "'%s'; errno: %d", argv, err);
-				}
-				return;
-			}
+			if (v != NULL)
+				gv_post_event(sc, GV_EVENT_START_VOLUME, v,
+				    NULL, NULL);
 			break;
 
 		case GV_TYPE_PLEX:
 			p = gv_find_plex(sc, argv);
-			err = gv_start_plex(p);
-			if (err) {
-				if (err == EINPROGRESS) {
-					gctl_error(req, "cannot start plex "
-					    "'%s': already in progress", argv);
-				} else {
-					gctl_error(req, "cannot start plex "
-					    "'%s'; errno: %d", argv, err);
-				}
-				return;
-			}
+			if (p != NULL)
+				gv_post_event(sc, GV_EVENT_START_PLEX, p, NULL,
+				    NULL);
 			break;
 
 		case GV_TYPE_SD:
@@ -252,7 +105,7 @@
 	}
 }
 
-static int
+int
 gv_start_plex(struct gv_plex *p)
 {
 	struct gv_volume *v;
@@ -265,19 +118,20 @@
 
 	error = 0;
 	v = p->vol_sc;
-	if ((v != NULL) && (v->plexcount > 1))
-		error = gv_sync(v);
+	if ((v != NULL) && (v->plexcount > 1));
+/*		error = gv_sync(v);*/
 	else if (p->org == GV_PLEX_RAID5) {
 		if (p->state == GV_PLEX_DEGRADED)
 			error = gv_rebuild_plex(p);
-		else
-			error = gv_init_plex(p);
+/*		else
+			error = gv_init_plex(p);*/
 	}
 
 	return (error);
 }
 
-static int
+#if 0
+int
 gv_start_vol(struct gv_volume *v)
 {
 	struct gv_plex *p;
@@ -357,91 +211,25 @@
 
 	return (0);
 }
+#endif
 
-struct gv_bio *
-gv_new_rebuild_bio(struct gv_plex *p, off_t offset)
+static int
+gv_rebuild_plex(struct gv_plex *p)
 {
-	struct gv_sync_args *sync;
-	struct bio *bp;
-	struct gv_volume *v;
-	struct gv_plex *p;
-	struct g_consumer *cp;
-	u_char *buf;
-	off_t i;
-	int error;
 
-	KASSERT(p != NULL, ("gv_rebuild_plex: NULL p"));
+	if (gv_provider_is_open(p->vol_sc->provider))
+		return (EBUSY);
 
-	buf = NULL;
-	bp = NULL;
-	v = p->vol_sc;
-
-	if (!= NULL)
-	/* Check if any of our subdisks drives have consumers open. */
-	/* XXX: Not provider since our mirror might block us. */
-/*	if (gv_provider_is_open(v->provider))
-		return (EBUSY);*/
-
-/*	if (p->flags & GV_PLEX_SYNCING)
+	if (p->flags & GV_PLEX_SYNCING)
 		return (EINPROGRESS);
-	p->flags |= GV_PLEX_SYNCING;*/
-
-	sync = g_malloc(sizeof(*sync), M_WAITOK | M_ZERO);
-	sync->to = p;
-	sync->syncsize = GV_DFLT_SYNCSIZE;
-
-/*	p->synced = 0;*/
-/*	cp = p->consumer;
-
-	g_topology_lock();
-	error = g_access(cp, 1, 1, 0);
-	if (error) {
-		g_topology_unlock();
-		printf("GEOM_VINUM: rebuild of %s failed to access consumer: "
-		    "%d\n", p->name, error);
-		return (GV_ERR_REBUILD);
-	}
-	g_topology_unlock();
-*/
-	buf = g_malloc(sync->syncsize, M_WAITOK);
-
-	printf("GEOM_VINUM: rebuild of %s started\n", p->name);
-	i = 0;
-
-/*	for (i = 0; i < p->size; i += (p->stripesize * (p->sdcount - 1))) {*/
-/*
-		if (i + sync->syncsize > p->size)
-			sync->syncsize = p->size - i;
-*/
-	bp = g_new_bio();
-	if (bp == NULL) {
-		printf("GEOM_VINUM: rebuild of %s failed creating bio: "
-		    "out of memory\n", p->name);
-		return (NULL);
-	}
-	bp->bio_cmd = BIO_WRITE;
-	bp->bio_done = NULL;
-	bp->bio_data = buf;
-	bp->bio_cflags |= GV_BIO_REBUILD;
-	bp->bio_offset = offset;
-	bp->bio_length = p->stripesize;
-
-/*	g_topology_lock();
-	g_access(cp, -1, -1, 0);
-	gv_save_config_all(p->vinumconf);
-	g_topology_unlock();
-*/
-/*	p->flags &= ~GV_PLEX_SYNCING;
+	p->flags |= GV_PLEX_SYNCING;
 	p->synced = 0;
-*/
-	/* Successful initialization. */
-/*	if (!error)
-		printf("GEOM_VINUM: rebuild of %s finished\n", p->name);*/
 
-
-	return (bp);
+	gv_send_parity_bio(p, GV_BIO_REBUILD, 0);
+	return (0);
 }
 
+#if 0
 static int
 gv_init_plex(struct gv_plex *p)
 {

==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_plex.c#10 (text+ko) ====

@@ -46,6 +46,7 @@
 static int	gv_normal_parity(struct gv_plex *, struct bio *,
 		    struct gv_raid5_packet *);
 static void	gv_parity_completed(struct gv_plex *, struct bio *);
+static void	gv_rebuild_completed(struct gv_plex *, struct bio *);
 static struct bio * gv_plexbuffer(struct gv_plex *, struct bio *, caddr_t,
 			off_t, off_t, int *);
 
@@ -364,12 +365,13 @@
 	/* When the original request is finished, we deliver it. */
 	pbp->bio_inbed++;
 	if (pbp->bio_inbed == pbp->bio_children) {
-		/* XXX: Should this be done here? Probably to be done later, but
-		 * just for testing. */
+		/* Hand it over for checking or delivery. */
 		if (pbp->bio_cmd == BIO_WRITE &&
-		   (pbp->bio_cflags & GV_BIO_CHECK)) {
-			/* Schedule a new bio down to rebuild parity. */
+		    (pbp->bio_cflags & GV_BIO_CHECK)) {
 			gv_parity_completed(p, pbp);
+		} else if (pbp->bio_cmd == BIO_WRITE &&
+		    (pbp->bio_cflags & GV_BIO_REBUILD)) {
+			gv_rebuild_completed(p, pbp);
 		} else
 			g_io_deliver(pbp, pbp->bio_error);
 	}
@@ -466,17 +468,16 @@
 }
 
 /*
- * Create a new bio struct for the next parity rebuild. It is assumed that a bio
- * from the last rebuild-parity is given as argument.
- * XXX: Should perhaps be another place.
+ * Create a new bio struct for the next parity rebuild. Used both by internal
+ * rebuild of degraded plexes as well as user initiated rebuilds/checks.
  */
 void
-gv_issue_next_parity_bio(struct gv_plex *p, int rebuild)
+gv_send_parity_bio(struct gv_plex *p, int flags, off_t offset)
 {
 	struct bio *bp;
 	int error;
 
-	KASSERT(p != NULL, ("gv_issue_next_parity_bio: NULL p"));
+	KASSERT(p != NULL, ("gv_send_parity_bio: NULL p"));
 
 	/* Make sure we don't have the lock. */
 	g_topology_assert_not();
@@ -498,18 +499,27 @@
 
 	bp->bio_cmd = BIO_WRITE;
 	bp->bio_done = NULL;
-	bp->bio_data = g_malloc(p->stripesize, M_WAITOK | M_ZERO);
 	bp->bio_error = 0;
-/*	bp->bio_cflags |= GV_BIO_REBUILD;*/
-	bp->bio_cflags |= GV_BIO_CHECK;
-	if (rebuild)
-		bp->bio_cflags |= GV_BIO_PARITY;
+	bp->bio_length = p->stripesize;
+
+	/*
+	 * Check if it's a rebuild of a degraded plex or a user request of
+	 * parity rebuild.
+	 */
+	if (flags & GV_BIO_REBUILD)
+		bp->bio_data = g_malloc(GV_DFLT_SYNCSIZE, M_WAITOK);
+	else if (flags & GV_BIO_CHECK)
+		bp->bio_data = g_malloc(p->stripesize, M_WAITOK | M_ZERO);
+	else {
+		printf("VINUM: invalid flags given\n");
+		return;
+	}
+
+	bp->bio_cflags = flags;
 	bp->bio_cflags |= GV_BIO_MALLOC;
-	bp->bio_length = p->stripesize;
 
 	/* We still have more parity to build. */
-	bp->bio_offset = p->synced;
-	/* Now we need to find the correct subdisk to send it to. */
+	bp->bio_offset = offset;
 
 	printf("VINUM: Issuing bio at offset 0x%jx\n",
 	    (intmax_t)bp->bio_offset);
@@ -525,10 +535,13 @@
 static void
 gv_parity_completed(struct gv_plex *p, struct bio *bp)
 {
-	int error, rebuild;
+	int error, flags;
 
 	error = bp->bio_error;
-	rebuild = bp->bio_cflags & GV_BIO_PARITY;
+	flags = bp->bio_cflags;
+	flags &= ~GV_BIO_MALLOC;
+
+	/* Clean up what we allocated. */
 	if (bp->bio_cflags & GV_BIO_MALLOC)
 		g_free(bp->bio_data);
 	g_destroy_bio(bp);
@@ -538,13 +551,11 @@
 	g_topology_lock();
 	gv_access(p->vol_sc->provider, -1, -1, 0);
 	g_topology_unlock();
-	/* Clean up what we allocated. */
-	/* XXX: Check for error type. */
 	if (error) {
 		if (error == EAGAIN) {
 			printf("VINUM: Parity incorrect at offset 0x%jx\n",
 			    (intmax_t)p->synced);
-			if (!rebuild)
+			if (!(flags & GV_BIO_PARITY))
 				return;
 		}
 		printf("VINUM: Parity check on %s failed at 0x%jx errno %d\n",
@@ -564,7 +575,52 @@
 	}
 
 	/* Send down next. It will determine if we need to itself. */
-	gv_issue_next_parity_bio(p, rebuild);
+	gv_send_parity_bio(p, flags, p->synced);
+}
+
+/*
+ * Handle a finished plex rebuild bio.
+ */
+static void
+gv_rebuild_completed(struct gv_plex *p, struct bio *bp)
+{
+	int error, flags;
+	off_t offset;
+
+	error = bp->bio_error;
+	flags = bp->bio_cflags;
+	offset = bp->bio_offset;
+	flags &= ~GV_BIO_MALLOC;
+
+	/* Clean up what we allocated. */
+	if (bp->bio_cflags & GV_BIO_MALLOC)
+		g_free(bp->bio_data);
+	g_destroy_bio(bp);
+
+	/* Make sure we don't have the lock. */
+	g_topology_assert_not();
+	g_topology_lock();
+	gv_access(p->vol_sc->provider, -1, -1, 0);
+	g_topology_unlock();
+	if (error) {
+		printf("VINUM: rebuild of %s failed at offset %jd errno: %d\n",
+		    p->name, (intmax_t)offset, error);
+		return;
+	}
+
+	offset += (p->stripesize * (p->sdcount - 1));
+	printf("VINUM: rebuild at 0x%jx finished\n", (intmax_t)offset);
+	if (offset >= p->size) {
+		/* We're finished. */
+		printf("VINUM: rebuild of %s finished\n", p->name);
+		gv_save_config(p->vinumconf);
+		p->flags &= ~GV_PLEX_SYNCING;
+		p->synced = 0;
+		return;
+	}
+
+	/* Send down next. It will determine if we need to itself. */
+	gv_send_parity_bio(p, flags, offset);
 }
 
 void

==== //depot/projects/soc2007/lulf/gvinum_fixup/sys/geom/vinum/geom_vinum_var.h#10 (text+ko) ====

@@ -192,6 +192,8 @@
 #define GV_EVENT_RESET_CONFIG		17
 #define GV_EVENT_PARITY_REBUILD		18
 #define GV_EVENT_PARITY_CHECK		19
+#define GV_EVENT_START_PLEX		20
+#define GV_EVENT_START_VOLUME		21
 
 struct gv_event {
 	int	type;


More information about the p4-projects mailing list