Re: [PATCH v6 02/18] xl / libxl: push parsing of SSID and CPU pool ID down to libxl

[Wei Liu <wei.liu2@citrix.com>]

On Tue, Jun 10, 2014 at 01:57:55PM +0100, Ian Campbell wrote:
> On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> > This patch pushes parsing of "init_seclabel", "seclabel",
> > "device_model_stubdomain_seclabel" and "pool" down to libxl level.
> > 
> > Originally the parsing is done in xl level, which is not ideal because
> > libxl won't have the truely relevant information. With this patch libxl
> > holds important information by itself.
> > 
> > The libxl IDL is extended to hold the string of labels and pool name.
> > And if there those strings are present they take precedence over the
> > numeric representations.
> > 
> > As all relevant structures have a field called X_name / X_label now, a
> > string is also copied there so that we can use it directly.
> 
> I'm not sure what you mean by this. Do you mean that if the caller uses
> the numeric and not the string version we will populate the string side?
> 

I mean I added string field in libxl_dominfo etc, so that caller won't
have to do the translation.

I shall make this clearer in commit message.

> >   In order to
> > be compatible with users of older versions of libxl, this patch also
> > defines LIBXL_HAVE_SSID_LABEL and LIBXL_HAVE_CPUPOOL_NAME. If they are
> > defined, the respective strings are available. And if those strings are
> > not NULL, libxl will do the parsing and ignore the numeric values.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > Cc: Dario Faggioli <dario.faggioli@citrix.com>
> > Cc: Juergen Gross <juergen.gross@ts.fujitsu.com>
> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> 
> (I switched to Juergen's new address in the CC)
> 
> > ---
> >  tools/libxl/libxl.c         |   19 ++++++--
> >  tools/libxl/libxl.h         |   20 ++++++++
> >  tools/libxl/libxl_create.c  |   57 +++++++++++++++++++++++
> >  tools/libxl/libxl_dm.c      |    4 ++
> >  tools/libxl/libxl_types.idl |    6 +++
> >  tools/libxl/xl_cmdimpl.c    |  107 ++++++++++++-------------------------------
> >  tools/libxl/xl_sxp.c        |    7 +--
> >  7 files changed, 134 insertions(+), 86 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index 900b8d4..5f320ca 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -516,12 +516,18 @@ int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid,
> >      return 0;
> >  }
> >  
> > -static void xcinfo2xlinfo(const xc_domaininfo_t *xcinfo,
> > +static void xcinfo2xlinfo(libxl_ctx *ctx,
> > +                          const xc_domaininfo_t *xcinfo,
> >                            libxl_dominfo *xlinfo)
> >  {
> > +    size_t size;
> > +
> >      memcpy(&(xlinfo->uuid), xcinfo->handle, sizeof(xen_domain_handle_t));
> >      xlinfo->domid = xcinfo->domain;
> >      xlinfo->ssidref = xcinfo->ssidref;
> > +    if (libxl_flask_sid_to_context(ctx, xlinfo->ssidref,
> > +                                   &xlinfo->ssid_label, &size) < 0)
> > +        xlinfo->ssid_label = NULL;
> 
> libxl_set_memory_target uses xcinfo2xlinfo but incorrectly fails to call
> dispose on the result, so it will leak any ssid_label.
> 

I will fix this.

Wei.

> The other callers of this function all seem to return the result to the
> application, which can be expected to be correct here.
> 
> Other than that:
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Ian.