Simplify the GEOM OAM api: Drop the request type, and let everything

hinge on the "verb" parameter which the class gets to interpret as
it sees fit.

Move the entire request into the kernel and move changed parameters
back when done.

7 files changed, 289 insertions, 432 deletions

diff --git a/sys/geom/bde/g_bde.c b/sys/geom/bde/g_bde.c
index 5e10ba3..e3e06ec 100644
--- a/sys/geom/bde/g_bde.c
+++ b/sys/geom/bde/g_bde.c
@@ -111,7 +111,7 @@ g_bde_access(struct g_provider *pp, int dr, int dw, int de)
 	return (g_access_rel(cp, dr, dw, de));
 }
 
-static int
+static void
 g_bde_create_geom(struct gctl_req *req, struct g_class *mp, struct g_provider *pp)
 {
 	struct g_geom *gp;
@@ -124,8 +124,6 @@ g_bde_create_geom(struct gctl_req *req, struct g_class *mp, struct g_provider *p
 	void *pass;
 	void *key;
 
-	if (pp == NULL)
-		return (gctl_error(req, "Provider needed"));
 	g_trace(G_T_TOPOLOGY, "g_bde_create_geom(%s, %s)", mp->name, pp->name);
 	g_topology_assert();
 	gp = NULL;
@@ -143,21 +141,19 @@ g_bde_create_geom(struct gctl_req *req, struct g_class *mp, struct g_provider *p
 		g_detach(cp);
 		g_destroy_consumer(cp);
 		g_destroy_geom(gp);
-		return (error);
+		gctl_error(req, "could not access consumer");
 	}
-	g_topology_unlock();
-	g_waitidle();
 	pass = NULL;
 	key = NULL;
 	do {
 		pass = gctl_get_param(req, "pass", &i);
 		if (pass == NULL || i != SHA512_DIGEST_LENGTH) {
-			error = gctl_error(req, "No usable key presented");
+			gctl_error(req, "No usable key presented");
 			break;
 		}
 		key = gctl_get_param(req, "key", &i);
 		if (key != NULL && i != 16) {
-			error = gctl_error(req, "Invalid key presented");
+			gctl_error(req, "Invalid key presented");
 			break;
 		}
 		sectorsize = cp->provider->sectorsize;
@@ -194,7 +190,6 @@ g_bde_create_geom(struct gctl_req *req, struct g_class *mp, struct g_provider *p
 		kthread_create(g_bde_worker, gp, &sc->thread, 0, 0,
 			"g_bde %s", gp->name);
 		mtx_unlock(&Giant);
-		g_topology_lock();
 		pp = g_new_providerf(gp, gp->name);
 #if 0
 		/*
@@ -209,28 +204,21 @@ g_bde_create_geom(struct gctl_req *req, struct g_class *mp, struct g_provider *p
 		pp->mediasize = sc->mediasize;
 		pp->sectorsize = sc->sectorsize;
 		g_error_provider(pp, 0);
-		g_topology_unlock();
 		break;
 	} while (0);
-	if (pass != NULL) {
+	if (pass != NULL)
 		bzero(pass, SHA512_DIGEST_LENGTH);
-		g_free(pass);
-	}
-	if (key != NULL) {
+	if (key != NULL)
 		bzero(key, 16);
-		g_free(key);
-	}
-	g_topology_lock();
-	if (error == 0) {
-		return (0);
-	}
+	if (error == 0)
+		return;
 	g_access_rel(cp, -1, -1, -1);
 	g_detach(cp);
 	g_destroy_consumer(cp);
 	if (gp->softc != NULL)
 		g_free(gp->softc);
 	g_destroy_geom(gp);
-	return (error);
+	return;
 }
 
 
@@ -252,7 +240,6 @@ g_bde_destroy_geom(struct gctl_req *req, struct g_class *mp, struct g_geom *gp)
 	KASSERT(pp != NULL, ("NULL provider"));
 	if (pp->acr > 0 || pp->acw > 0 || pp->ace > 0)
 		return (EBUSY);
-	g_orphan_provider(pp, ENXIO);
 	sc = gp->softc;
 	cp = LIST_FIRST(&gp->consumer);
 	KASSERT(cp != NULL, ("NULL consumer"));
@@ -262,23 +249,38 @@ g_bde_destroy_geom(struct gctl_req *req, struct g_class *mp, struct g_geom *gp)
 	KASSERT(error == 0, ("error on close"));
 	g_detach(cp);
 	g_destroy_consumer(cp);
-	g_topology_unlock();
 	while (sc->dead != 2 && !LIST_EMPTY(&pp->consumers))
 		tsleep(sc, PRIBIO, "g_bdedie", hz);
-	g_waitidle();
-	g_topology_lock();
-	g_destroy_provider(pp);
 	mtx_destroy(&sc->worklist_mutex);
 	bzero(&sc->key, sizeof sc->key);
 	g_free(sc);
-	g_destroy_geom(gp);
+	g_wither_geom(gp, ENXIO);
 	return (0);
 }
 
+static void
+g_bde_ctlreq(struct gctl_req *req, struct g_class *mp, char const *verb)
+{
+	struct g_geom *gp;
+	struct g_provider *pp;
+
+	if (!strcmp(verb, "create geom")) {
+		pp = gctl_get_provider(req, "provider");
+		if (pp != NULL)
+			g_bde_create_geom(req, mp, pp);
+	} else if (!strcmp(verb, "destroy geom")) {
+		gp = gctl_get_geom(req, mp, "geom");
+		if (gp != NULL)
+			g_bde_destroy_geom(req, mp, gp);
+	} else {
+		gctl_error(req, "unknown verb");
+	}
+}
+
 static struct g_class g_bde_class	= {
 	.name = BDE_CLASS_NAME,
-	.create_geom = g_bde_create_geom,
 	.destroy_geom = g_bde_destroy_geom,
+	.ctlreq = g_bde_ctlreq,
 };
 
 DECLARE_GEOM_CLASS(g_bde_class, g_bde);
diff --git a/sys/geom/geom.h b/sys/geom/geom.h
index 7bccecc..5dc6925 100644
--- a/sys/geom/geom.h
+++ b/sys/geom/geom.h
@@ -58,6 +58,7 @@ struct gctl_req;
 struct g_configargs;
 
 typedef int g_config_t (struct g_configargs *ca);
+typedef void g_ctl_req_t (struct gctl_req *, struct g_class *cp, char const *verb);
 typedef int g_ctl_create_geom_t (struct gctl_req *, struct g_class *cp, struct g_provider *pp);
 typedef int g_ctl_destroy_geom_t (struct gctl_req *, struct g_class *cp, struct g_geom *gp);
 typedef int g_ctl_config_geom_t (struct gctl_req *, struct g_geom *gp, const char *verb);
@@ -88,11 +89,10 @@ struct g_class {
 	const char		*name;
 	g_taste_t		*taste;
 	g_config_t		*config;
+	g_ctl_req_t		*ctlreq;
 	g_init_t		*init;
 	g_fini_t		*fini;
-	g_ctl_create_geom_t	*create_geom;
 	g_ctl_destroy_geom_t	*destroy_geom;
-	g_ctl_config_geom_t	*config_geom;
 	/*
 	 * The remaining elements are private
 	 */
@@ -304,9 +304,13 @@ extern struct sx topology_lock;
 #endif /* _KERNEL */
 
 /* geom_ctl.c */
-int gctl_set_param(struct gctl_req *req, const char *param, void *ptr, int len);
+void gctl_set_param(struct gctl_req *req, const char *param, void const *ptr, int len);
 void *gctl_get_param(struct gctl_req *req, const char *param, int *len);
+char const *gctl_get_asciiparam(struct gctl_req *req, const char *param);
 void *gctl_get_paraml(struct gctl_req *req, const char *param, int len);
 int gctl_error(struct gctl_req *req, const char *fmt, ...);
+struct g_class *gctl_get_class(struct gctl_req *req, char const *arg);
+struct g_geom *gctl_get_geom(struct gctl_req *req, struct g_class *mpr, char const *arg);
+struct g_provider *gctl_get_provider(struct gctl_req *req, char const *arg);
 
 #endif /* _GEOM_GEOM_H_ */
diff --git a/sys/geom/geom_bsd.c b/sys/geom/geom_bsd.c
index 41a141c..464bfc1 100644
--- a/sys/geom/geom_bsd.c
+++ b/sys/geom/geom_bsd.c
@@ -670,28 +670,32 @@ g_bsd_callconfig(void *arg, int flag)
 /*
  * NB! curthread is user process which GCTL'ed.
  */
-static int
-g_bsd_config(struct gctl_req *req, struct g_geom *gp, const char *verb)
+static void
+g_bsd_config(struct gctl_req *req, struct g_class *mp, char const *verb)
 {
 	u_char *label;
 	int error;
 	struct h0h0 h0h0;
+	struct g_geom *gp;
 	struct g_slicer *gsp;
 	struct g_consumer *cp;
 	struct g_bsd_softc *ms;
 
 	g_topology_assert();
+	gp = gctl_get_geom(req, mp, "geom");
+	if (gp == NULL)
+		return;
 	cp = LIST_FIRST(&gp->consumer);
 	gsp = gp->softc;
 	ms = gsp->softc;
 	if (!strcmp(verb, "read mbroffset")) {
-		error = gctl_set_param(req, "mbroffset",
+		gctl_set_param(req, "mbroffset",
 		    &ms->mbroffset, sizeof(ms->mbroffset));
-		return (error);
+		return;
 	} else if (!strcmp(verb, "write label")) {
 		label = gctl_get_paraml(req, "label", LABELSIZE);
 		if (label == NULL)
-			return (EINVAL);
+			return;
 		h0h0.gp = gp;
 		h0h0.ms = gsp->softc;
 		h0h0.label = label;
@@ -699,40 +703,36 @@ g_bsd_config(struct gctl_req *req, struct g_geom *gp, const char *verb)
 		/* XXX: Does this reference register with our selfdestruct code ? */
 		error = g_access_rel(cp, 1, 1, 1);
 		if (error) {
-			g_free(label);
-			return (error);
+			gctl_error(req, "could not access consumer");
+			return;
 		}
-		g_topology_unlock();
-		g_waitfor_event(g_bsd_callconfig, &h0h0, M_WAITOK, gp, NULL);
-		g_topology_lock();
+		g_bsd_callconfig(&h0h0, 0);
 		error = h0h0.error;
 		g_access_rel(cp, -1, -1, -1);
-		g_free(label);
 	} else if (!strcmp(verb, "write bootcode")) {
 		label = gctl_get_paraml(req, "bootcode", BBSIZE);
 		if (label == NULL)
-			return (EINVAL);
+			return;
 		/* XXX: Does this reference register with our selfdestruct code ? */
 		error = g_access_rel(cp, 1, 1, 1);
 		if (error) {
-			g_free(label);
-			return (error);
+			gctl_error(req, "could not access consumer");
+			return;
 		}
 		error = g_bsd_writelabel(gp, label);
 		g_access_rel(cp, -1, -1, -1);
-		g_free(label);
 	} else {
-		return (gctl_error(req, "Unknown verb parameter"));
+		gctl_error(req, "Unknown verb parameter");
 	}
 
-	return (error);
+	return;
 }
 
 /* Finally, register with GEOM infrastructure. */
 static struct g_class g_bsd_class = {
 	.name = BSD_CLASS_NAME,
 	.taste = g_bsd_taste,
-	.config_geom = g_bsd_config,
+	.ctlreq = g_bsd_config,
 };
 
 DECLARE_GEOM_CLASS(g_bsd_class, g_bsd);
diff --git a/sys/geom/geom_ctl.c b/sys/geom/geom_ctl.c
index 053f0ee..bb92d41 100644
--- a/sys/geom/geom_ctl.c
+++ b/sys/geom/geom_ctl.c
@@ -58,7 +58,6 @@
 #include <geom/geom_int.h>
 #define GCTL_TABLE 1
 #include <geom/geom_ctl.h>
-#include <geom/geom_ext.h>
 
 #include <machine/stdarg.h>
 
@@ -92,27 +91,25 @@ g_ctl_init(void)
 int
 gctl_error(struct gctl_req *req, const char *fmt, ...)
 {
-	int error;
 	va_list ap;
-	struct sbuf *sb;
-
-	sb = sbuf_new(NULL, NULL, 0, SBUF_AUTOEXTEND);
-	if (sb == NULL) {
-		error = copyout(fmt, req->error,
-			    imin(req->lerror, strlen(fmt) + 1));
-	} else {
-		va_start(ap, fmt);
-		sbuf_vprintf(sb, fmt, ap);
-		sbuf_finish(sb);
-		if (g_debugflags & G_F_CTLDUMP)
-			printf("gctl %p error \"%s\"\n", req, sbuf_data(sb));
-		error = copyout(sbuf_data(sb), req->error,
-		    imin(req->lerror, sbuf_len(sb) + 1));
-	}
-	if (!error)
-		error = EINVAL;
-	sbuf_delete(sb);
-	return (error);
+
+	if (req == NULL)
+		return (EINVAL);
+
+	/* We only record the first error */
+	if (req->nerror)
+		return (req->nerror);
+
+	va_start(ap, fmt);
+	sbuf_vprintf(req->serror, fmt, ap);
+	sbuf_finish(req->serror);
+	if (g_debugflags & G_F_CTLDUMP)
+		printf("gctl %p error \"%s\"\n", req, sbuf_data(req->serror));
+	req->nerror = copyout(sbuf_data(req->serror), req->error,
+	    imin(req->lerror, sbuf_len(req->serror) + 1));
+	if (!req->nerror)
+		req->nerror = EINVAL;
+	return (req->nerror);
 }
 
 /*
@@ -120,60 +117,51 @@ gctl_error(struct gctl_req *req, const char *fmt, ...)
  * XXX: this should really be a standard function in the kernel.
  */
 static void *
-geom_alloc_copyin(struct gctl_req *req, void *uaddr, size_t len, int *errp)
+geom_alloc_copyin(struct gctl_req *req, void *uaddr, size_t len)
 {
-	int error;
 	void *ptr;
 
 	ptr = g_malloc(len, M_WAITOK);
 	if (ptr == NULL)
-		error = ENOMEM;
+		req->nerror = ENOMEM;
 	else
-		error = copyin(uaddr, ptr, len);
-	if (!error)
+		req->nerror = copyin(uaddr, ptr, len);
+	if (!req->nerror)
 		return (ptr);
-	gctl_error(req, "no access to argument");
-	*errp = error;
 	if (ptr != NULL)
 		g_free(ptr);
 	return (NULL);
 }
 
-
-/*
- * XXX: This function is a nightmare.  It walks through the request and
- * XXX: makes sure that the various bits and pieces are there and copies
- * XXX: some of them into kernel memory to make things easier.
- * XXX: I really wish we had a standard marshalling layer somewhere.
- */
-
-static int
+static void
 gctl_copyin(struct gctl_req *req)
 {
-	int error, i, j;
+	int error, i;
 	struct gctl_req_arg *ap;
 	char *p;
 
-	error = 0;
-	ap = geom_alloc_copyin(req, req->arg, req->narg * sizeof(*ap), &error);
+	ap = geom_alloc_copyin(req, req->arg, req->narg * sizeof(*ap));
 	if (ap == NULL) {
-		gctl_error(req, "copyin() of arguments failed");
-		return (error);
+		req->nerror = ENOMEM;
+		req->arg = NULL;
+		return;
 	}
 
-	for (i = 0; !error && i < req->narg; i++) {
-		if (ap[i].len > 0 &&
-		    !useracc(ap[i].value, ap[i].len, 
-		    ap[i].flag & GCTL_PARAM_RW))
-			error = gctl_error(req, "no access to param data");
-		if (error)
-			break;
-		p = NULL;
+	/* Nothing have been copyin()'ed yet */
+	for (i = 0; i < req->narg; i++) {
+		ap[i].flag &= ~(GCTL_PARAM_NAMEKERNEL|GCTL_PARAM_VALUEKERNEL);
+		ap[i].flag &= ~GCTL_PARAM_CHANGED;
+		ap[i].kvalue = NULL;
+	}
+
+	error = 0;
+	for (i = 0; i < req->narg; i++) {
 		if (ap[i].nlen < 1 || ap[i].nlen > SPECNAMELEN) {
-			error = gctl_error(req, "wrong param name length");
+			error = gctl_error(req,
+			    "wrong param name length %d: %d", i, ap[i].nlen);
 			break;
 		}
-		p = geom_alloc_copyin(req, ap[i].name, ap[i].nlen, &error);
+		p = geom_alloc_copyin(req, ap[i].name, ap[i].nlen);
 		if (p == NULL)
 			break;
 		if (p[ap[i].nlen - 1] != '\0') {
@@ -182,17 +170,52 @@ gctl_copyin(struct gctl_req *req)
 			break;
 		}
 		ap[i].name = p;
-		ap[i].nlen = 0;
+		ap[i].flag |= GCTL_PARAM_NAMEKERNEL;
+		if (ap[i].len < 0) {
+			error = gctl_error(req, "negative param length");
+			break;
+		}
+		if (ap[i].len == 0) {
+			ap[i].kvalue = ap[i].value;
+			ap[i].flag |= GCTL_PARAM_VALUEKERNEL;
+			continue;
+		}
+		p = geom_alloc_copyin(req, ap[i].value, ap[i].len);
+		if (p == NULL)
+			break;
+		if ((ap[i].flag & GCTL_PARAM_ASCII) &&
+		    p[ap[i].len - 1] != '\0') {
+			error = gctl_error(req, "unterminated param value");
+			g_free(p);
+			break;
+		}
+		ap[i].kvalue = p;
+		ap[i].flag |= GCTL_PARAM_VALUEKERNEL;
 	}
-	if (!error) {
-		req->arg = ap;
-		return (0);
+	req->arg = ap;
+	return;
+}
+
+static void
+gctl_copyout(struct gctl_req *req)
+{
+	int error, i;
+	struct gctl_req_arg *ap;
+
+	if (req->nerror)
+		return;
+	error = 0;
+	ap = req->arg;
+	for (i = 0; i < req->narg; i++, ap++) {
+		if (!(ap->flag & GCTL_PARAM_CHANGED))
+			continue;
+		error = copyout(ap->kvalue, ap->value, ap->len);
+		if (!error)
+			continue;
+		req->nerror = error;
+		return;
 	}
-	for (j = 0; j < i; j++)
-		if (ap[j].nlen == 0 && ap[j].name != NULL)
-			g_free(ap[j].name);
-	g_free(ap);
-	return (error);
+	return;
 }
 
 static void
@@ -200,61 +223,60 @@ gctl_free(struct gctl_req *req)
 {
 	int i;
 
+	if (req->arg == NULL)
+		return;
 	for (i = 0; i < req->narg; i++) {
-		if (req->arg[i].nlen == 0 && req->arg[i].name != NULL)
+		if (req->arg[i].flag & GCTL_PARAM_NAMEKERNEL)
 			g_free(req->arg[i].name);
+		if ((req->arg[i].flag & GCTL_PARAM_VALUEKERNEL) &&
+		    req->arg[i].len > 0)
+			g_free(req->arg[i].kvalue);
 	}
 	g_free(req->arg);
+	sbuf_delete(req->serror);
 }
 
 static void
 gctl_dump(struct gctl_req *req)
 {
 	u_int i;
-	int j, error;
+	int j;
 	struct gctl_req_arg *ap;
-	void *p;
-	
 
-	printf("Dump of gctl %s request at %p:\n", req->reqt->name, req);
-	if (req->lerror > 0) {
-		p = geom_alloc_copyin(req, req->error, req->lerror, &error);
-		if (p != NULL) {
-			((char *)p)[req->lerror - 1] = '\0';
-			printf("  error:\t\"%s\"\n", (char *)p);
-			g_free(p);
-		}
+	printf("Dump of gctl request at %p:\n", req);
+	if (req->nerror > 0) {
+		printf("  nerror:\t%d\n", req->nerror);
+		if (sbuf_len(req->serror) > 0)
+			printf("  error:\t\"%s\"\n", sbuf_data(req->serror));
 	}
 	for (i = 0; i < req->narg; i++) {
 		ap = &req->arg[i];
-		printf("  param:\t\"%s\"", ap->name);
+		if (!(ap->flag & GCTL_PARAM_NAMEKERNEL))
+			printf("  param:\t%d@%p", ap->nlen, ap->name);
+		else
+			printf("  param:\t\"%s\"", ap->name);
 		printf(" [%s%s%d] = ",
 		    ap->flag & GCTL_PARAM_RD ? "R" : "",
 		    ap->flag & GCTL_PARAM_WR ? "W" : "",
 		    ap->len);
-		if (ap->flag & GCTL_PARAM_ASCII) {
-			p = geom_alloc_copyin(req, ap->value, ap->len, &error);
-			if (p != NULL) {
-				((char *)p)[ap->len - 1] = '\0';
-				printf("\"%s\"", (char *)p);
-			}
-			g_free(p);
+		if (!(ap->flag & GCTL_PARAM_VALUEKERNEL)) {
+			printf(" =@ %p", ap->value);
+		} else if (ap->flag & GCTL_PARAM_ASCII) {
+			printf("\"%s\"", (char *)ap->kvalue);
 		} else if (ap->len > 0) {
-			p = geom_alloc_copyin(req, ap->value, ap->len, &error);
 			for (j = 0; j < ap->len; j++)
-				printf(" %02x", ((u_char *)p)[j]);
-			g_free(p);
+				printf(" %02x", ((u_char *)ap->kvalue)[j]);
 		} else {
-			printf(" = %p", ap->value);
+			printf(" = %p", ap->kvalue);
 		}
 		printf("\n");
 	}
 }
 
-int
-gctl_set_param(struct gctl_req *req, const char *param, void *ptr, int len)
+void
+gctl_set_param(struct gctl_req *req, const char *param, void const *ptr, int len)
 {
-	int i, error;
+	int i;
 	struct gctl_req_arg *ap;
 
 	for (i = 0; i < req->narg; i++) {
@@ -263,23 +285,24 @@ gctl_set_param(struct gctl_req *req, const char *param, void *ptr, int len)
 			continue;
 		if (!(ap->flag & GCTL_PARAM_WR)) {
 			gctl_error(req, "No write access %s argument", param);
-			return (EINVAL);
+			return;
 		}
-		if (ap->len != len) {
+		if (ap->len < len) {
 			gctl_error(req, "Wrong length %s argument", param);
-			return (EINVAL);
+			return;
 		}
-		error = copyout(ptr, ap->value, len);
-		return (error);
+		bcopy(ptr, ap->kvalue, len);
+		ap->flag |= GCTL_PARAM_CHANGED;
+		return;
 	}
 	gctl_error(req, "Missing %s argument", param);
-	return (EINVAL);
+	return;
 }
 
 void *
 gctl_get_param(struct gctl_req *req, const char *param, int *len)
 {
-	int i, error, j;
+	int i;
 	void *p;
 	struct gctl_req_arg *ap;
 
@@ -289,17 +312,36 @@ gctl_get_param(struct gctl_req *req, const char *param, int *len)
 			continue;
 		if (!(ap->flag & GCTL_PARAM_RD))
 			continue;
-		if (ap->len > 0)
-			j = ap->len;
-		else
-			j = 0;
-		if (j != 0)
-			p = geom_alloc_copyin(req, ap->value, j, &error);
-			/* XXX: should not fail, tested prviously */
-		else
-			p = ap->value;
+		p = ap->kvalue;
 		if (len != NULL)
-			*len = j;
+			*len = ap->len;
+		return (p);
+	}
+	return (NULL);
+}
+
+char const *
+gctl_get_asciiparam(struct gctl_req *req, const char *param)
+{
+	int i;
+	char const *p;
+	struct gctl_req_arg *ap;
+
+	for (i = 0; i < req->narg; i++) {
+		ap = &req->arg[i];
+		if (strcmp(param, ap->name))
+			continue;
+		if (!(ap->flag & GCTL_PARAM_RD))
+			continue;
+		p = ap->kvalue;
+		if (ap->len < 1) {
+			gctl_error(req, "No length argument (%s)", param);
+			return (NULL);
+		}
+		if (p[ap->len - 1] != '\0') {
+			gctl_error(req, "Unterminated argument (%s)", param);
+			return (NULL);
+		}
 		return (p);
 	}
 	return (NULL);
@@ -315,193 +357,105 @@ gctl_get_paraml(struct gctl_req *req, const char *param, int len)
 	if (p == NULL)
 		gctl_error(req, "Missing %s argument", param);
 	else if (i != len) {
-		g_free(p);
 		p = NULL;
 		gctl_error(req, "Wrong length %s argument", param);
 	}
 	return (p);
 }
 
-static struct g_class*
-gctl_get_class(struct gctl_req *req)
+struct g_class *
+gctl_get_class(struct gctl_req *req, char const *arg)
 {
-	char *p;
-	int len;
+	char const *p;
 	struct g_class *cp;
 
-	p = gctl_get_param(req, "class", &len);
+	p = gctl_get_asciiparam(req, arg);
 	if (p == NULL)
 		return (NULL);
-	if (p[len - 1] != '\0') {
-		gctl_error(req, "Unterminated class name");
-		g_free(p);
-		return (NULL);
-	}
 	LIST_FOREACH(cp, &g_classes, class) {
-		if (!strcmp(p, cp->name)) {
-			g_free(p);
+		if (!strcmp(p, cp->name))
 			return (cp);
-		}
 	}
-	g_free(p);
 	gctl_error(req, "Class not found");
 	return (NULL);
 }
 
-static struct g_geom*
-gctl_get_geom(struct gctl_req *req, struct g_class *mpr)
+struct g_geom *
+gctl_get_geom(struct gctl_req *req, struct g_class *mpr, char const *arg)
 {
-	char *p;
-	int len;
+	char const *p;
 	struct g_class *mp;
 	struct g_geom *gp;
 
-	p = gctl_get_param(req, "geom", &len);
+	p = gctl_get_asciiparam(req, arg);
 	if (p == NULL)
 		return (NULL);
-	if (p[len - 1] != '\0') {
-		gctl_error(req, "Unterminated provider name");
-		g_free(p);
-		return (NULL);
-	}
 	LIST_FOREACH(mp, &g_classes, class) {
 		if (mpr != NULL && mpr != mp)
 			continue;
 		LIST_FOREACH(gp, &mp->geom, geom) {
-			if (!strcmp(p, gp->name)) {
-				g_free(p);
+			if (!strcmp(p, gp->name))
 				return (gp);
-			}
 		}
 	}
 	gctl_error(req, "Geom not found");
-	g_free(p);
 	return (NULL);
 }
 
-static struct g_provider*
-gctl_get_provider(struct gctl_req *req)
+struct g_provider *
+gctl_get_provider(struct gctl_req *req, char const *arg)
 {
-	char *p;
-	int len;
+	char const *p;
 	struct g_class *cp;
 	struct g_geom *gp;
 	struct g_provider *pp;
 
-	p = gctl_get_param(req, "provider", &len);
+	p = gctl_get_asciiparam(req, arg);
 	if (p == NULL)
 		return (NULL);
-	if (p[len - 1] != '\0') {
-		gctl_error(req, "Unterminated provider name");
-		g_free(p);
-		return (NULL);
-	}
 	LIST_FOREACH(cp, &g_classes, class) {
 		LIST_FOREACH(gp, &cp->geom, geom) {
 			LIST_FOREACH(pp, &gp->provider, provider) {
-				if (!strcmp(p, pp->name)) {
-					g_free(p);
+				if (!strcmp(p, pp->name))
 					return (pp);
-				}
 			}
 		}
 	}
 	gctl_error(req, "Provider not found");
-	g_free(p);
 	return (NULL);
 }
 
-static int
-gctl_create_geom(struct gctl_req *req)
-{
-	struct g_class *mp;
-	struct g_provider *pp;
-	int error;
-
-	g_topology_assert();
-	mp = gctl_get_class(req);
-	if (mp == NULL)
-		return (gctl_error(req, "Class not found"));
-	if (mp->create_geom == NULL)
-		return (gctl_error(req, "Class has no create_geom method"));
-	pp = gctl_get_provider(req);
-	error = mp->create_geom(req, mp, pp);
-	g_topology_assert();
-	return (error);
-}
-
-static int
-gctl_destroy_geom(struct gctl_req *req)
+static void
+g_ctl_req(void *arg, int flag __unused)
 {
 	struct g_class *mp;
-	struct g_geom *gp;
-	int error;
+	struct gctl_req *req;
+	char const *verb;
 
 	g_topology_assert();
-	mp = gctl_get_class(req);
+	req = arg;
+	mp = gctl_get_class(req, "class");
 	if (mp == NULL)
-		return (gctl_error(req, "Class not found"));
-	if (mp->destroy_geom == NULL)
-		return (gctl_error(req, "Class has no destroy_geom method"));
-	gp = gctl_get_geom(req, mp);
-	if (gp == NULL)
-		return (gctl_error(req, "Geom not specified"));
-	if (gp->class != mp)
-		return (gctl_error(req, "Geom not of specificed class"));
-	error = mp->destroy_geom(req, mp, gp);
+		return;
+	verb = gctl_get_param(req, "verb", NULL);
+	if (mp->ctlreq == NULL)
+		gctl_error(req, "Class takes no requests");
+	else
+		mp->ctlreq(req, mp, verb);
 	g_topology_assert();
-	return (error);
 }
 
-static int
-gctl_config_geom(struct gctl_req *req)
-{
-	struct g_class *mp;
-	struct g_geom *gp;
-	char *verb;
-	int error, vlen;
-
-	g_topology_assert();
-	mp = gctl_get_class(req);
-	if (mp == NULL)
-		return (gctl_error(req, "Class not found"));
-	if (mp->config_geom == NULL)
-		return (gctl_error(req, "Class has no config_geom method"));
-	gp = gctl_get_geom(req, mp);
-	if (gp == NULL)
-		return (gctl_error(req, "Geom not specified"));
-	if (gp->class != mp)
-		return (gctl_error(req, "Geom not of specificed class"));
-	verb = gctl_get_param(req, "verb", &vlen);
-	if (verb == NULL)
-		return (gctl_error(req, "Missing verb parameter"));
-	if (vlen < 2) {
-		g_free(verb);
-		return (gctl_error(req, "Too short verb parameter"));
-	}
-	if (verb[vlen - 1] != '\0') {
-		g_free(verb);
-		return (gctl_error(req, "Unterminated verb parameter"));
-	}
-	error = mp->config_geom(req, gp, verb);
-	g_free(verb);
-	g_topology_assert();
-	return (error);
-}
 
-/*
- * Handle ioctl from libgeom::geom_ctl.c
- */
 static int
 g_ctl_ioctl_ctl(dev_t dev, u_long cmd, caddr_t data, int fflag, struct thread *td)
 {
-	int error;
-	int i;
 	struct gctl_req *req;
 
 	req = (void *)data;
+	req->nerror = 0;
+	req->serror = sbuf_new(NULL, NULL, 0, SBUF_AUTOEXTEND);
 	/* It is an error if we cannot return an error text */
-	if (req->lerror < 1)
+	if (req->lerror < 2)
 		return (EINVAL);
 	if (!useracc(req->error, req->lerror, VM_PROT_WRITE))
 		return (EINVAL);
@@ -511,39 +465,19 @@ g_ctl_ioctl_ctl(dev_t dev, u_long cmd, caddr_t data, int fflag, struct thread *t
 		return (gctl_error(req,
 		    "kernel and libgeom version mismatch."));
 	
-	/* Check the request type */
-	for (i = 0; gcrt[i].request != GCTL_INVALID_REQUEST; i++)
-		if (gcrt[i].request == req->request)
-			break;
-	if (gcrt[i].request == GCTL_INVALID_REQUEST)
-		return (gctl_error(req, "invalid request"));
-	req->reqt = &gcrt[i];
-
 	/* Get things on board */
-	error = gctl_copyin(req);
-	if (error)
-		return (error);
+	gctl_copyin(req);
 
 	if (g_debugflags & G_F_CTLDUMP)
 		gctl_dump(req);
-	g_topology_lock();
-	switch (req->request) {
-	case GCTL_CREATE_GEOM:
-		error = gctl_create_geom(req);
-		break;
-	case GCTL_DESTROY_GEOM:
-		error = gctl_destroy_geom(req);
-		break;
-	case GCTL_CONFIG_GEOM:
-		error = gctl_config_geom(req);
-		break;
-	default:
-		error = gctl_error(req, "XXX: TBD");
-		break;
+
+	if (!req->nerror) {
+		g_waitfor_event(g_ctl_req, req, M_WAITOK, NULL);
+		gctl_copyout(req);
 	}
+
 	gctl_free(req);
-	g_topology_unlock();
-	return (error);
+	return (req->nerror);
 }
 
 static int
@@ -553,12 +487,10 @@ g_ctl_ioctl(dev_t dev, u_long cmd, caddr_t data, int fflag, struct thread *td)
 
 	switch(cmd) {
 	case GEOM_CTL:
-		DROP_GIANT();
 		error = g_ctl_ioctl_ctl(dev, cmd, data, fflag, td);
-		PICKUP_GIANT();
 		break;
 	default:
-		error = ENOTTY;
+		error = ENOIOCTL;
 		break;
 	}
 	return (error);
diff --git a/sys/geom/geom_ctl.h b/sys/geom/geom_ctl.h
index 3f7b3d5..fd68bda 100644
--- a/sys/geom/geom_ctl.h
+++ b/sys/geom/geom_ctl.h
@@ -32,57 +32,51 @@
 #ifndef _GEOM_GEOM_CTL_H_
 #define _GEOM_GEOM_CTL_H_
 
-/*
- * Version number.  Used to check consistency between kernel and libgeom.
- */
-#define GCTL_VERSION	1
+#include <sys/ioccom.h>
 
 /*
- * Primitives.
+ * Version number.  Used to check consistency between kernel and libgeom.
  */
-enum gctl_request {
-	GCTL_INVALID_REQUEST = 0,
+#define GCTL_VERSION	2
 
-	GCTL_CREATE_GEOM,
-	GCTL_DESTROY_GEOM,
+struct gctl_req_arg {
+	u_int				nlen;
+	char				*name;
+	off_t				offset;
+	int				flag;
+	int				len;
+	void				*value;
+	/* kernel only fields */
+	void				*kvalue;
+};
 
-	GCTL_ATTACH,
-	GCTL_DETACH,
+#define GCTL_PARAM_RD		1	/* Must match VM_PROT_READ */
+#define GCTL_PARAM_WR		2	/* Must match VM_PROT_WRITE */
+#define GCTL_PARAM_RW		(GCTL_PARAM_RD | GCTL_PARAM_WR)
+#define GCTL_PARAM_ASCII	4
 
-	GCTL_CREATE_PROVIDER,
-	GCTL_DESTROY_PROVIDER,
+/* These are used in the kernel only */
+#define GCTL_PARAM_NAMEKERNEL	8
+#define GCTL_PARAM_VALUEKERNEL	16
+#define GCTL_PARAM_CHANGED	32
 
-	GCTL_INSERT_GEOM,
-	GCTL_ELIMINATE_GEOM,
+struct gctl_req {
+	u_int				version;
+	u_int				serial;
+	u_int				narg;
+	struct gctl_req_arg		*arg;
+	u_int				lerror;
+	char				*error;
+	struct gctl_req_table		*reqt;
 
-	GCTL_CONFIG_GEOM,
+	/* kernel only fields */
+	int				nerror;
+	struct sbuf			*serror;
 };
 
-#ifdef GCTL_TABLE
-struct gctl_req_table {
-	int             	class;
-	int             	geom;
-	int             	provider;
-	int             	consumer;
-	int             	params;
-	char			*name;
-	enum gctl_request	request;
-} gcrt[] = {
-/*        Cl Ge Pr Co Pa Name                Request			*/
-	{ 1, 0, 1, 0, 1, "create geom",		GCTL_CREATE_GEOM            },
-	{ 0, 1, 0, 0, 1, "destroy geom",	GCTL_DESTROY_GEOM           },
-	{ 0, 1, 1, 0, 1, "attach",		GCTL_ATTACH                 },
-	{ 0, 1, 1, 0, 1, "detach",		GCTL_DETACH                 },
-	{ 0, 1, 0, 0, 1, "create provider",	GCTL_CREATE_PROVIDER        },
-	{ 0, 1, 1, 0, 1, "destroy provider",	GCTL_DESTROY_PROVIDER       },
-	{ 1, 1, 1, 0, 1, "insert geom",		GCTL_INSERT_GEOM            },
-	{ 0, 1, 0, 0, 1, "eliminate geom",	GCTL_ELIMINATE_GEOM         },
-	{ 0, 1, 0, 0, 1, "config geom",		GCTL_CONFIG_GEOM            },
+#define GEOM_CTL	_IOW('G', GCTL_VERSION, struct gctl_req)
 
-	/* Terminator entry */
-	{ 1, 1, 1, 1, 1, "*INVALID*",		GCTL_INVALID_REQUEST        }
-};
+#define PATH_GEOM_CTL	"geom.ctl"
 
-#endif /* GCTL_TABLE */
 
 #endif /* _GEOM_GEOM_CTL_H_ */
diff --git a/sys/geom/geom_ext.h b/sys/geom/geom_ext.h
deleted file mode 100644
index a48fc0b..0000000
--- a/sys/geom/geom_ext.h
+++ /dev/null
@@ -1,72 +0,0 @@
-/*-
- * Copyright (c) 2003 Poul-Henning Kamp
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- * 3. The names of the authors may not be used to endorse or promote
- *    products derived from this software without specific prior written
- *    permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- *
- * This file defines the interface between libgeom and the geom control device
- * in the kernel, the interfaces defined herein are to be considered private
- * and may only be used by libgeom.  Applications wishing to interact with
- * the geom subsystem must use libgeoms published APIs.
- *
- * $FreeBSD$
- */
-
-#ifndef _GEOM_GEOM_EXT_H_
-#define _GEOM_GEOM_EXT_H_
-
-#include <sys/ioccom.h>
-#include <geom/geom_ctl.h>
-
-struct gctl_req_arg {
-	u_int				nlen;
-	char				*name;
-	off_t				offset;
-	int				flag;
-	int				len;
-	void				*value;
-};
-
-#define GCTL_PARAM_RD		1	/* Must match VM_PROT_READ */
-#define GCTL_PARAM_WR		2	/* Must match VM_PROT_WRITE */
-#define GCTL_PARAM_RW		(GCTL_PARAM_RD | GCTL_PARAM_WR)
-#define GCTL_PARAM_ASCII	4
-
-struct gctl_req {
-	u_int				version;
-	u_int				serial;
-	enum gctl_request		request;
-	u_int				narg;
-	struct gctl_req_arg		*arg;
-	u_int				lerror;
-	char				*error;
-	struct gctl_req_table		*reqt;
-};
-
-#define GEOM_CTL	_IOW('G', GCTL_VERSION, struct gctl_req)
-
-#define PATH_GEOM_CTL	"geom.ctl"
-
-#endif /* _GEOM_GEOM_EXT_H_ */
diff --git a/sys/geom/geom_sunlabel.c b/sys/geom/geom_sunlabel.c
index f37c0dc..cfade35 100644
--- a/sys/geom/geom_sunlabel.c
+++ b/sys/geom/geom_sunlabel.c
@@ -169,22 +169,26 @@ g_sunlabel_callconfig(void *arg, int flag)
 /*
  * NB! curthread is user process which GCTL'ed.
  */
-static int
-g_sunlabel_config(struct gctl_req *req, struct g_geom *gp, const char *verb)
+static void
+g_sunlabel_config(struct gctl_req *req, struct g_class *mp, const char *verb)
 {
 	u_char *label;
 	int error, i;
 	struct g_hh01 h0h0;
 	struct g_slicer *gsp;
+	struct g_geom *gp;
 	struct g_consumer *cp;
 
 	g_topology_assert();
+	gp = gctl_get_geom(req, mp, "geom");
+	if (gp == NULL)
+		return;
 	cp = LIST_FIRST(&gp->consumer);
 	gsp = gp->softc;
 	if (!strcmp(verb, "write label")) {
 		label = gctl_get_paraml(req, "label", SUN_SIZE);
 		if (label == NULL)
-			return (EINVAL);
+			return;
 		h0h0.gp = gp;
 		h0h0.ms = gsp->softc;
 		h0h0.label = label;
@@ -192,24 +196,20 @@ g_sunlabel_config(struct gctl_req *req, struct g_geom *gp, const char *verb)
 		/* XXX: Does this reference register with our selfdestruct code ? */
 		error = g_access_rel(cp, 1, 1, 1);
 		if (error) {
-			g_free(label);
-			return (error);
+			gctl_error(req, "could not access consumer");
+			return;
 		}
-		g_topology_unlock();
-		g_waitfor_event(g_sunlabel_callconfig, &h0h0, M_WAITOK, gp, NULL);
-		g_topology_lock();
-		error = h0h0.error;
+		g_sunlabel_callconfig(&h0h0, 0);
 		g_access_rel(cp, -1, -1, -1);
-		g_free(label);
 	} else if (!strcmp(verb, "write bootcode")) {
 		label = gctl_get_paraml(req, "bootcode", SUN_BOOTSIZE);
 		if (label == NULL)
-			return (EINVAL);
+			return;
 		/* XXX: Does this reference register with our selfdestruct code ? */
 		error = g_access_rel(cp, 1, 1, 1);
 		if (error) {
-			g_free(label);
-			return (error);
+			gctl_error(req, "could not access consumer");
+			return;
 		}
 		for (i = 0; i < SUN_NPART; i++) {
 			if (gsp->slices[i].length <= SUN_BOOTSIZE)
@@ -219,12 +219,9 @@ g_sunlabel_config(struct gctl_req *req, struct g_geom *gp, const char *verb)
 			    SUN_BOOTSIZE - SUN_SIZE);
 		}
 		g_access_rel(cp, -1, -1, -1);
-		g_free(label);
 	} else {
-		return (gctl_error(req, "Unknown verb parameter"));
+		gctl_error(req, "Unknown verb parameter");
 	}
-
-	return (error);
 }
 
 static struct g_geom *
@@ -278,7 +275,7 @@ g_sunlabel_taste(struct g_class *mp, struct g_provider *pp, int flags)
 static struct g_class g_sunlabel_class = {
 	.name = SUNLABEL_CLASS_NAME,
 	.taste = g_sunlabel_taste,
-	.config_geom = g_sunlabel_config,
+	.ctlreq = g_sunlabel_config,
 };
 
 DECLARE_GEOM_CLASS(g_sunlabel_class, g_sunlabel);