[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v10 6/6] tools: enable Cache QoS Monitoring feature for libxl/libxc
> -----Original Message----- > From: Ian Campbell [mailto:Ian.Campbell@xxxxxxxxxx] > Sent: Friday, March 28, 2014 5:44 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 Fri, 2014-03-28 at 00:53 +0000, Xu, Dongxiao wrote: > > > -----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. > > But libxl_getqminfo has three such nr_* pointers as parameters, which > one is the length of the returned array? What are the other two? How > does one enumerate the things which the other nr_* refer to? The size is calculated by nr_domains*nr_sockets. The nr_rmids is to show how many RMIDs a system has. > > > > > 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. */). > > If libxl_pqos_type_from_string fails then qos_type remains undefined, so > you can't rely on some later function checking for correctness, the > undefined value may happen to appear valid. Okay, I will add a check in the function. Thanks, Dongxiao > > Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |