Re: [Xen-devel] [PATCH v10 6/6] tools: enable Cache QoS Monitoring feature for libxl/libxc

["Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx>]

> -----Original Message-----
> From: Ian Campbell [mailto:Ian.Campbell@xxxxxxxxxx]
> Sent: Thursday, March 27, 2014 11:14 PM
> To: Xu, Dongxiao
> Cc: xen-devel@xxxxxxxxxxxxx; keir@xxxxxxx; JBeulich@xxxxxxxx;
> Ian.Jackson@xxxxxxxxxxxxx; stefano.stabellini@xxxxxxxxxxxxx;
> andrew.cooper3@xxxxxxxxxx; konrad.wilk@xxxxxxxxxx; dgdegra@xxxxxxxxxxxxx
> Subject: Re: [PATCH v10 6/6] tools: enable Cache QoS Monitoring feature for
> libxl/libxc
> 
> On Wed, 2014-03-26 at 14:35 +0800, Dongxiao Xu wrote:
> [...]
> > +int libxl_pqos_attach(libxl_ctx *ctx, uint32_t domid, libxl_pqos_type
> qos_type);
> > +int libxl_pqos_detach(libxl_ctx *ctx, uint32_t domid, libxl_pqos_type
> qos_type);
> > +libxl_cqminfo * libxl_getcqminfo(libxl_ctx *ctx, unsigned int *l3c_total,
> > +    unsigned int *nr_domains, unsigned int *nr_rmids, unsigned int
> *nr_sockets);
> [...]
> > +libxl_cqminfo = Struct("cqminfo", [
> > +    ("valid",         uint32),
> > +    ("l3c",           uint64),
> > +    ])
> 
> libxl_getcqminfo seems to have a strange mixture of returning an info
> struct and returning things via pointers passed as integers. why is
> that?

The returned info is a unit data structure containing the L3 cache usage data 
for certain RMID on certain socket.
These pointers passed integers tell caller the number of such units are 
returned.

This follows one existing function:
libxl_dominfo * libxl_list_domain(libxl_ctx *ctx, int *nb_domain_out);
where the return value is the unit dominfo data structure, and *nb_domain_out 
tells caller how many data structure units are returned.

> 
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index 8389468..664a34d 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -7381,6 +7381,115 @@ out:
> >      return ret;
> >  }
> >
> > +int main_pqosattach(int argc, char **argv)
> > +{
> > +    uint32_t domid;
> > +    int opt, rc;
> > +    libxl_pqos_type qos_type;
> > +
> > +    SWITCH_FOREACH_OPT(opt, "", NULL, "pqos-attach", 2) {
> > +        /* No options */
> > +    }
> > +
> > +    /* libxl_pqos_attach will handle the parameter check. */
> > +    libxl_pqos_type_from_string(argv[optind], &qos_type);
> 
> This can fail, I think you need to check the return value.

Yes, I am aware of that.
The following libxl_pqos_attach() function will check the parameter correctness.
Actually I put a comment in above (/* libxl_pqos_attach will handle the 
parameter check. */).

Thanks,
Dongxiao

> 
> > +    domid = find_domain(argv[optind + 1]);
> > +
> > +    rc = libxl_pqos_attach(ctx, domid, qos_type);
> > +
> > +    return rc;
> > +}
> > +
> > +int main_pqosdetach(int argc, char **argv)
> > +{
> > +    uint32_t domid;
> > +    int opt, rc;
> > +    libxl_pqos_type qos_type;
> > +
> > +    SWITCH_FOREACH_OPT(opt, "", NULL, "pqos-detach", 2) {
> > +        /* No options */
> > +    }
> > +
> > +    /* libxl_pqos_detach will handle the parameter check. */
> > +    libxl_pqos_type_from_string(argv[optind], &qos_type);
> 
> Same again.
> 
> Ian.
> 

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