Re: [Xen-devel] [PATCH V5 02/32] xl / libxl: push parsing of SSID and CPU pool ID down to libxl

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, May 15, 2014 at 05:38:05PM +0100, Ian Campbell wrote:
> On Tue, 2014-05-13 at 22:53 +0100, Wei Liu wrote:
> > @@ -619,6 +625,11 @@ static int cpupool_info(libxl__gc *gc,
> >      }
> >  
> >      info->poolid = xcinfo->cpupool_id;
> > +    info->pool_name = libxl_cpupoolid_to_name(CTX, info->poolid);
> > +    if (!info->pool_name) {
> > +        rc = ERROR_NOMEM;
> 
> libxl_cpupooid_to_name also fails if the pool doesn't exist. ERROR_FAIL
> might be more appropriate?

OK.

> > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> > index 84f9c0e..2b06094 100644
> > --- a/tools/libxl/libxl.h
> > +++ b/tools/libxl/libxl.h
> > @@ -469,6 +469,30 @@
> >  #define LIBXL_EXTERNAL_CALLERS_ONLY /* disappears for callers outside 
> > libxl */
> >  #endif
> >  
> > +/*
> > + * LIBXL_HAVE_SSID_LABEL
> > + *
> > + * If this is defined, then libxl IDL contains string of XSM security
> > + * label in all XSM related structures.
> > + *
> > + * If this string is not NULL, libxl will overwrite the numeric
> > + * representation of this configuration regardless if it has been
> > + * set by the caller, because libxl will do the parsing by itself.
> 
> A more natural to say this would be:
> 
> "If set this string takes precedence over the numeric field"
> 
> I think...
> 

OK.

> > + */
> > +#define LIBXL_HAVE_SSID_LABEL 1
> > +
> > +/*
> > + * LIBXL_HAVE_CPUPOOL_NAME
> > + *
> > + * If this is defined, then libxl IDL contains string of CPU pool
> > + * name in all CPU pool related structures.
> > + *
> > + * If this string is not NULL, libxl will overwrite the numeric
> > + * representation of this configuration regardless if it has been
> > + * set by the caller, because libxl will do the parsing by itself.
> 
> Similarly.
> 
> > + */
> > +#define LIBXL_HAVE_CPUPOOL_NAME 1
> > +
> >  typedef uint8_t libxl_mac[6];
> >  #define LIBXL_MAC_FMT "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx"
> >  #define LIBXL_MAC_FMTLEN ((2*6)+5) /* 6 hex bytes plus 5 colons */
> > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> > index d015cf4..fe3bdd2 100644
> > --- a/tools/libxl/libxl_create.c
> > +++ b/tools/libxl/libxl_create.c
> > @@ -724,6 +724,63 @@ static void initiate_domain_create(libxl__egc *egc,
> >  
> >      domid = 0;
> >  
> > +    if (d_config->c_info.ssid_label) {
> > +        char *s = d_config->c_info.ssid_label;
> > +        ret = libxl_flask_context_to_sid(ctx, s, strlen(s),
> > +                                         &d_config->c_info.ssidref);
> > +        if (ret) {
> > +            if (errno == ENOSYS) {
> > +                LOG(WARN, "XSM Disabled: init_seclabel not supported");
> 
> Should you set ssidref to the explicit invalid value here? I suppose it
> isn't going to matter much?
> 

I don't think so. Nobody should try to make sense out of ssidref if XSM
is disabled anyway. And I really have no idea what is the "invalid
value" for ssidref.

Wei.

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