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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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.

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

> > 
> > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > ---
> >  tools/libxl/libxl.c |   68 
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tools/libxl/libxl.h |   10 ++++++++
> >  2 files changed, 78 insertions(+)
> > 
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index c9475d7..b578a5c 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -5751,6 +5751,74 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, 
> > libxl_mac *src)
> >      for (i = 0; i < 6; i++)
> >          (*dst)[i] = (*src)[i];
> >  }
> > +
> > +int libxl_load_domain_configuration(libxl_ctx *ctx, uint32_t domid,
> > +                                    libxl_domain_config *d_config)
> > +{
> > +    GC_INIT(ctx);
> > +    uint8_t *data;
> > +    int rc, len;
> > +
> > +    rc = libxl_userdata_retrieve(ctx, domid, "libxl-json", &data, &len);
> > +    if (rc) {
> > +        LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
> 
> You can use LOGEV here to shorten lines and loge below etc.
> 

OK.

> > +                            "Failed to retrieve domain configuration for 
> > domain %d",
> > +                            domid);
> > +        rc = ERROR_FAIL;
> > +        goto out;
> > +    }
> > +
> > +    if (len == 0) {
> > +        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> 
> > +                         "Configuration data stream empty for domain %d",
> > +                         domid);
> > +        rc = ERROR_FAIL;
> > +        goto out;
> > +    }
> > +
> > +    /* Make sure this string ends with \0 -- the parser expects a NULL
> > +     * terminated string.
> 
> Could this be achieved by strlen(d_config_json)+1 when saving? Assuming
> that string is NULL terminated.
> 

I need to try. If it works then I will switch to that.

> > +     */
> > +    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.

> > +    GC_FREE;
> > +    return rc;
> > +}
> > +
[...]
> >   * Local variables:
> >   * mode: C
> > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> > index d5dfe2e..be722b6 100644
> > --- a/tools/libxl/libxl.h
> > +++ b/tools/libxl/libxl.h
> > @@ -1140,6 +1140,8 @@ void libxl_cpuid_set(libxl_ctx *ctx, uint32_t domid,
> >   *  "xl"          domain config file in xl format, Unix line endings
> >   *  "libvirt-xml" domain config file in libvirt XML format.  See
> >   *                http://libvirt.org/formatdomain.html
> > + *  "libxl-json"  domain config file in JSON format, generated by
> > + *                libxl
> 
> I think I would mention libxl_domain_config. Something like
> "libxl_domain_config object in JSON format, generated by libxl."
> perhaps.
> 

OK.

Wei.

> 
> Ian.

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