Re: [Xen-devel] [PATCH V5 27/32] libxl: libxl-json format and load/store configuration functions

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Sun, 2014-06-01 at 19:41 +0100, Wei Liu wrote:
> On Tue, May 20, 2014 at 03:03:56PM +0100, Ian Campbell wrote:
> > On Tue, 2014-05-13 at 22:54 +0100, Wei Liu wrote:
> > > Introduce a new format in libxl userdata store called "libxl-json". This
> > > format contains the deserialized version of domain configuration of a
> > > domain.
> > > 
> > > Two libxl functions to load and store domain configuration are also
> > > introduced. They use the new "libxl-json" format.
> > 
> > Is the intention that application should use these to read/modify/write
> > the domain config on changes, as opposed to it being internal
> > functionality which libxl should use to keep things up to date?
> > 
> 
> I think it is a valid usecase that a user of libxl wants to know
> libxl's view of a guest's configuration, and I think user is free to
> manipulate that view if it wants to.

I think the get method is fine. The set is more debatable though.

> Xl does this in later patch, when it migrate a domain the name is
> modified to "XX--incoming" then changed back to "XX".

I think Ian J commented on that too, perhaps for other reasons.

Anyway, this should happen in libxl_domain_rename, shouldn't it?

> > > +     */
> > > +    if (data[len-1] != '\0') {
> > > +        data = libxl__realloc(gc, data, len + 1);
> > > +        data[len] = '\0';
> > > +    }
> > > +
> > > +    rc = libxl_domain_config_from_json(ctx, d_config, (const char 
> > > *)data);
> > > +
> > > +out:
> > > +    free(data);
> > 
> > data might have been libxl__realloc'd above, so isn't it gc'd?
> > 
> 
> No, it's malloc'ed in libxl_read_file_contents.

But then you call libxl__realloc(gc,...). Won't that add the new pointer
to the gc (and fail to remove the old one from it because it wasn't
there)? Then you free it with free() and then GC_FREE will double free
it.

Ian.


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