Re: [PATCH 2/6] libxl: introduce libxl__device_generic_add_t

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Tue, 27 Mar 2012, Ian Campbell wrote:
> On Tue, 2012-03-27 at 14:59 +0100, Stefano Stabellini wrote:
> > Introduce libxl__device_generic_add_t that takes an xs_transaction_t as
> > parameter.
> > Use libxl__device_generic_add_t to implement libxl__device_generic_add.
> 
> I think it would be better to just change the existing API to add a
> transaction.

OK

> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > ---
> >  tools/libxl/libxl_device.c   |   36 ++++++++++++++++++++++++++----------
> >  tools/libxl/libxl_internal.h |    2 ++
> >  2 files changed, 28 insertions(+), 10 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> > index c2880e0..c6f9044 100644
> > --- a/tools/libxl/libxl_device.c
> > +++ b/tools/libxl/libxl_device.c
> > @@ -61,12 +61,37 @@ int libxl__parse_backend_path(libxl__gc *gc,
> >  int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
> >                               char **bents, char **fents)
> >  {
> > +    int rc = 0;
> > +    xs_transaction_t t;
> > +    libxl_ctx *ctx = libxl__gc_owner(gc);
> > +
> > +retry_transaction:
> > +    t = xs_transaction_start(ctx->xsh);
> > +
> > +    rc = libxl__device_generic_add_t(gc, t, device, bents, fents);
> > +
> > +    if (!xs_transaction_end(ctx->xsh, t, 0)) {
> > +        if (errno == EAGAIN)
> > +            goto retry_transaction;
> > +        else
> > +            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xs transaction failed");
> > +    }
> > +    return rc;
> > +}
> > +
> > +int libxl__device_generic_add_t(libxl__gc *gc, xs_transaction_t t,
> > +        libxl__device *device, char **bents, char **fents)
> > +{
> >      libxl_ctx *ctx = libxl__gc_owner(gc);
> >      char *frontend_path, *backend_path;
> > -    xs_transaction_t t;
> >      struct xs_permissions frontend_perms[2];
> >      struct xs_permissions backend_perms[2];
> >  
> > +    if (t == XBT_NULL)
> > +        /* we need a valid transaction: call libxl__device_generic_add
> > +         * to create one for us */
> > +        return libxl__device_generic_add(gc, device, bents, fents);
> 
> This function will in turn start a new transaction and then call us
> straight back again, which is all a bit of a roundabout way to do
> things. Can't this just be a t = xs_transaction_start?

this goes away if we just have libxl__device_generic_add with the
additional paramter