Re: [Xen-devel] [PATCH V4 16/24] libxl: copy function for builtin types

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-05-07 at 11:47 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH V4 16/24] libxl: copy function for builtin 
> types"):
> > On Thu, 2014-05-01 at 13:58 +0100, Wei Liu wrote:
> > > +typedef struct libxl__ctx libxl_ctx;
> > > +
> > >  int libxl_uuid_is_nil(libxl_uuid *uuid);
> > >  void libxl_uuid_generate(libxl_uuid *uuid);
> > >  int libxl_uuid_from_string(libxl_uuid *uuid, const char *in);
> > >  void libxl_uuid_copy(libxl_uuid *dst, const libxl_uuid *src);
> > > +void libxl_uuid_copy_ctx(libxl_ctx *ctx, libxl_uuid *dst,
> > > +                         const libxl_uuid *src);
> > 
> > Hrm, this is rather unfortunate.
> > 
> > All of these only take a ctx so they can use NOGC. I wonder if a #define
> > INIT_NOGC which provides a suitable gc (with maxsize == -1) might be
> > nicer than this?
> 
> The problem with this is that the caller's log functions won't be
> respected.

Ah yes, I'd forgotten this.

> I think it would be better to make all of the copy
> functions take a ctx.

Right. I was mostly annoyed by libxl_uuid_copy vs. libxl_uuid_copy_ctx.

I think we can probably use LIBXL_API_VERSION to add this parameter,
similar to how we did for libxl_domain_create_restore. That would mean
libxl_uuid_copy would have to accept ctx==NULL, but it seems unlikely to
need to log anyway.

Something like:

     #define LIBXL_HAVE_UUID_COPY_CTX_PARAM

        void libxl_uuid_copy(libxl_ctx *ctx, libxl_uuid *dst,
                             const libxl_uuid *src);
        
        #if defined(LIBXL_API_VERSION) && LIBXL_API_VERSION < 0x040400
        #define libxl_uuid_copy(dst, src) libxl_uuid_copy(NULL, dst, src)
        #endif

ought to do it I think? Perhaps we should do all of libxl_uuid_* for
consistency?

> Alternatively we should at least document this restriction, but also
> then we shouldn't ever call the copy functions from within libxl which
> (without reading the rest of the series) I think would probably defeat
> the point.

I think they are currently all used from xl, but one of my comments
might change that.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel