Date: Mon, 23 Mar 2009 21:19:48 +0100 From: Pawel Jakub Dawidek <pjd@FreeBSD.org> To: Luigi Rizzo <rizzo@iet.unipi.it> Cc: Ivan Voras <ivoras@FreeBSD.org>, Poul-Henning Kamp <phk@phk.freebsd.dk>, freebsd-geom@FreeBSD.org, luigi@FreeBSD.org, fabio@gandalf.sssup.it Subject: Re: RFC: adding 'proxy' nodes to provider ports (with patch) Message-ID: <20090323201948.GA1723@garage.freebsd.pl> In-Reply-To: <20090323200712.GA28660@onelab2.iet.unipi.it> References: <20090323060325.GN3102@garage.freebsd.pl> <42618.1237791496@critter.freebsd.dk> <20090323200712.GA28660@onelab2.iet.unipi.it>
next in thread | previous in thread | raw e-mail | index | archive | help
--LZvS9be/3tNcYl/X
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
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@garage.freebsd.pl>, Pawel Jakub Dawid=
ek write
> > s:
> >=20
> > >There is still a naming problem. pp and new_pp will end up with the sa=
me
> > >name. I'd suggest instructing GEOM to expose only parent in /dev/.
> >=20
> > who said the new provider had to have same name ?
> >=20
> > >The taste is still going to be send on new class arrival and on the la=
st
> > >pp write close.
> >=20
> > We decide that.
> >=20
> > Since we are inserting in an already open path, I think it makes very
> > good sense to supress tasting, at least until close.
>=20
> 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"
>=20
> BEFORE ---> [ pp --> old_gp ...]
>=20
> Then we can do either "geom xx create ad0" which results in
>=20
> AFTER create ---> [ newpp --> gp --> cp ] ---> [ pp --> old_gp ... ]
>=20
> or "geom xx insert ad0", which results in
>=20
> AFTER insert ---> [ pp --> gp --> cp ] ---> [ newpp --> old_gp ... ]
>=20
> The names of the various objects are the same in both cases so
>=20
> old_gp->name =3D "ad0"
> pp->name =3D "ad0"
> gp->name =3D "ad0.xx."
> newpp->name =3D "ad0.xx."
>=20
> 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).
>=20
> In terms of code, no changes to the infrastructure, and the
> create/insert and destroy functions are the following (error checking
> removed for clarity)
>=20
> g_xx_create(struct g_provider *pp, struct g_class *mp, int insert ...)
> {
> snprintf(name, sizeof(name), "%s%s", pp->name, MY_SUFFIX);
> gp =3D g_new_geomf(mp, name);
> ... allocate and fill softc and geom...
> newpp =3D g_new_providerf(insert ? pp->geom : gp, gp->name);
> ... initialize mediasize and sectorsize
> cp =3D 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 =3D gp;
newpp->private =3D pp->private;
pp->private =3D NULL;
newpp->index =3D pp->index;
pp->index =3D 0;
> LIST_INSERT_HEAD(&gp->provider, pp, provider);
> g_access(cp, 1, 1, 1); /* we can move data */
> sc->sc_insert =3D 1; /* remember for the des=
troy */
> }
> g_error_provider(newpp, 0);
> }
>=20
> 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.
>=20
> g_xx_destroy(struct g_geom *gp)
> {
> ...
> if (sc->sc_insert) {
> pp =3D LIST_FIRST(&gp->provider);
> cp =3D LIST_FIRST(&gp->consumer);
> newpp =3D cp->provider;
> /* Link provider to the original geom. */
> LIST_REMOVE(pp, provider);
> pp->geom =3D newpp->geom;
pp->private =3D newpp->private;
newpp->private =3D NULL;
pp->index =3D newpp->index;
newpp->index =3D 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 */
> }
>=20
> 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.
>=20
> 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.
>=20
> 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.)
--=20
Pawel Jakub Dawidek http://www.wheel.pl
pjd@FreeBSD.org http://www.FreeBSD.org
FreeBSD committer Am I Evil? Yes, I Am!
--LZvS9be/3tNcYl/X
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.4 (FreeBSD)
iD8DBQFJx+7kForvXbEpPzQRAluwAJ9K7kpyfB58to9cTtBgLXPXUEkDQwCdFNmR
vKbosqvo+QbJ+mcddARSZRg=
=uK+L
-----END PGP SIGNATURE-----
--LZvS9be/3tNcYl/X--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090323201948.GA1723>