RFC: adding 'proxy' nodes to provider ports (with patch)

[Pawel Jakub Dawidek]

On Mon, Mar 23, 2009 at 09:07:12PM +0100, Luigi Rizzo wrote:
> On Mon, Mar 23, 2009 at 06:58:16AM +0000, Poul-Henning Kamp wrote:
> > In message <20090323060325.GN3102 at garage.freebsd.pl>, Pawel Jakub Dawidek write
> > s:
> > 
> > >There is still a naming problem. pp and new_pp will end up with the same
> > >name. I'd suggest instructing GEOM to expose only parent in /dev/.
> > 
> > who said the new provider had to have same name ?
> > 
> > >The taste is still going to be send on new class arrival and on the last
> > >pp write close.
> > 
> > We decide that.
> > 
> > Since we are inserting in an already open path, I think it makes very
> > good sense to supress tasting, at least until close.
> 
> To summarize, here is how I have implemented a node that
> supports both regular "create" and the transparent "insert"
> we are discussing.
> Say we want to attach to an existing provider "pp" whose name is "ad0"
> 
>   BEFORE        ---> [ pp    --> old_gp ...]
> 
> Then we can do either "geom xx create ad0" which results in
> 
>   AFTER create  ---> [ newpp --> gp --> cp ] ---> [ pp    --> old_gp ... ]
> 
> or "geom xx insert ad0", which results in
> 
>   AFTER insert  ---> [ pp    --> gp --> cp ] ---> [ newpp --> old_gp ... ]
> 
> The names of the various objects are the same in both cases so
> 
> 	old_gp->name	= "ad0"
> 	pp->name	= "ad0"
> 	gp->name	= "ad0.xx."
> 	newpp->name	= "ad0.xx."
> 
> This lets new clients connect to provider "ad0" without having to
> know about any insertion.
> Also, to remove the newly inserted pieces, in both cases you can
> run the same command "geom xx destroy ad0.xx." (remembering that
> in this case you are naming the geom, not the provider).
> 
> In terms of code, no changes to the infrastructure, and the
> create/insert and destroy functions are the following (error checking
> removed for clarity)
> 
>    g_xx_create(struct g_provider *pp, struct g_class *mp, int insert ...)
>    {
>         snprintf(name, sizeof(name), "%s%s", pp->name, MY_SUFFIX);
>         gp = g_new_geomf(mp, name);
> 	... allocate and fill softc and geom...
>         newpp = g_new_providerf(insert ? pp->geom : gp, gp->name);
> 	... initialize mediasize and sectorsize
>         cp = g_new_consumer(gp);
>         g_attach(cp, insert ? newpp : pp);
>         if (insert) {
>                 g_cancel_event(newpp);          /* no taste() on this*/
>                 /* link pp to old_gp */
>                 LIST_REMOVE(pp, provider);
>                 pp->geom = gp;

		newpp->private = pp->private;
		pp->private = NULL;
		newpp->index = pp->index;
		pp->index = 0;

>                 LIST_INSERT_HEAD(&gp->provider, pp, provider);
>                 g_access(cp, 1, 1, 1);          /* we can move data */
>                 sc->sc_insert = 1;              /* remember for the destroy */
>         }
> 	g_error_provider(newpp, 0);
>     }
> 
> Here it is a bit inefficient to have to call g_cancel_event()
> but short of changing g_new_providerf() there is no way to
> avoid the g_new_provider event.
> 
>     g_xx_destroy(struct g_geom *gp)
>     {
>         ...
>         if (sc->sc_insert) {
>                 pp = LIST_FIRST(&gp->provider);
>                 cp = LIST_FIRST(&gp->consumer);
>                 newpp = cp->provider;
>                 /* Link provider to the original geom. */
>                 LIST_REMOVE(pp, provider);
>                 pp->geom = newpp->geom;

		pp->private = newpp->private;
		newpp->private = NULL;
		pp->index = newpp->index;
		newpp->index = 0;

>                 LIST_INSERT_HEAD(&pp->geom->provider, pp, provider);
>                 g_access(cp, -1, -1, -1);
> 		/* I am not sure if we need the following 3 */
>                 g_detach(cp);
>                 LIST_REMOVE(newpp, provider);
>                 g_destroy_provider(newpp);
>         }
>         ...
>         /* regular destroy path */
>     }
> 
> Above, I am not totally sure if we need to explicitly call g_detach()
> and destroy the provider, or if it will come for free as a result of
> the regular destoy code.
> 
> The block "if (sc->sc_insert) {..}" is reasonably generic
> (and large, when you put in the error checking) to possibly deserve
> a function in geom_subr.c -- but until there are no other clients,
> it makes no sense.
> 
> As usual, feedback welcome.

I don't think this is good idea to try to squeeze creation and insertion
in one function. IMHO it would be better to have generic functions for
insert/remove functionality:

	int g_insert(struct g_class *class, struct g_provider *oldpp);
	int g_remove(struct g_provider *oldpp);

(In g_insert() class name can be attached to new provider's name for
example.)

-- 
Pawel Jakub Dawidek                       http://www.wheel.pl
pjd at FreeBSD.org                           http://www.FreeBSD.org
FreeBSD committer                         Am I Evil? Yes, I Am!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-geom/attachments/20090323/83a8da72/attachment.pgp