George Dunlap
2012-Aug-15 09:28 UTC
[PATCH] xl: Suppress spurious warning message for cpupool-list
# HG changeset patch # User George Dunlap <george.dunlap@eu.citrix.com> # Date 1345022863 -3600 # Node ID 0982bad392e4f96fb39a025d6528c33be32c6c04 # Parent dc56a9defa30312a46cfb6ddb578e64cfbc6bc8b xl: Suppress spurious warning message for cpupool-list libxl_cpupool_list() enumerates the cpupools by "probing": calling cpupool_info, starting at 0 and stopping when it gets an error. However, cpupool_info will print an error when the call to xc_cpupool_getinfo() fails, resulting in every xl command that uses libxl_list_cpupool (such as cpupool-list) printing that error message spuriously. This patch adds a "probe" argument to cpupool_info(). If set, it won''t print a warning if the xc_cpupool_getinfo() fails with ENOENT. Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -583,7 +583,8 @@ int libxl_domain_info(libxl_ctx *ctx, li static int cpupool_info(libxl__gc *gc, libxl_cpupoolinfo *info, uint32_t poolid, - bool exact /* exactly poolid or >= poolid */) + bool exact /* exactly poolid or >= poolid */, + bool probe /* Don''t complain for non-existent pools */) { xc_cpupoolinfo_t *xcinfo; int rc = ERROR_FAIL; @@ -591,7 +592,8 @@ static int cpupool_info(libxl__gc *gc, xcinfo = xc_cpupool_getinfo(CTX->xch, poolid); if (xcinfo == NULL) { - LOGE(ERROR, "failed to get info for cpupool%d\n", poolid); + if (!probe || errno != ENOENT) + LOGE(ERROR, "failed to get info for cpupool%d\n", poolid); return ERROR_FAIL; } @@ -623,7 +625,7 @@ int libxl_cpupool_info(libxl_ctx *ctx, libxl_cpupoolinfo *info, uint32_t poolid) { GC_INIT(ctx); - int rc = cpupool_info(gc, info, poolid, true); + int rc = cpupool_info(gc, info, poolid, true, false); GC_FREE; return rc; } @@ -639,7 +641,7 @@ libxl_cpupoolinfo * libxl_list_cpupool(l poolid = 0; for (i = 0;; i++) { - if (cpupool_info(gc, &info, poolid, false)) + if (cpupool_info(gc, &info, poolid, false, true)) break; tmp = realloc(ptr, (i + 1) * sizeof(libxl_cpupoolinfo)); if (!tmp) {
Ian Campbell
2012-Aug-17 08:21 UTC
Re: [PATCH] xl: Suppress spurious warning message for cpupool-list
On Wed, 2012-08-15 at 10:28 +0100, George Dunlap wrote:> # HG changeset patch > # User George Dunlap <george.dunlap@eu.citrix.com> > # Date 1345022863 -3600 > # Node ID 0982bad392e4f96fb39a025d6528c33be32c6c04 > # Parent dc56a9defa30312a46cfb6ddb578e64cfbc6bc8b > xl: Suppress spurious warning message for cpupool-list > > libxl_cpupool_list() enumerates the cpupools by "probing": calling > cpupool_info, starting at 0 and stopping when it gets an error. However, > cpupool_info will print an error when the call to xc_cpupool_getinfo() fails, > resulting in every xl command that uses libxl_list_cpupool (such as > cpupool-list) printing that error message spuriously.I see: # xl -vvv cpupool-list Name CPUs Sched Active Domain count Pool-0 4 credit y 2 xc: debug: hypercall buffer: total allocations:5 total releases:5 xc: debug: hypercall buffer: current allocations:0 maximum allocations:2 xc: debug: hypercall buffer: cache current size:2 xc: debug: hypercall buffer: cache hits:3 misses:2 toobig:0 which doesn''t seem to include the error message. However by code inspection I''m sure I should be seeing what you describe. WTF?> This patch adds a "probe" argument to cpupool_info(). If set, it won''t print > a warning if the xc_cpupool_getinfo() fails with ENOENT.Looking at the callers I think the existing "exact" parameter could be used instead of a new param -- it would be fine to fail silently on ENOENT iff !exact, I think. Ian.
Ian Campbell
2012-Aug-17 08:23 UTC
Re: [PATCH] xl: Suppress spurious warning message for cpupool-list
On Fri, 2012-08-17 at 09:21 +0100, Ian Campbell wrote:> On Wed, 2012-08-15 at 10:28 +0100, George Dunlap wrote: > > # HG changeset patch > > # User George Dunlap <george.dunlap@eu.citrix.com> > > # Date 1345022863 -3600 > > # Node ID 0982bad392e4f96fb39a025d6528c33be32c6c04 > > # Parent dc56a9defa30312a46cfb6ddb578e64cfbc6bc8b > > xl: Suppress spurious warning message for cpupool-list > > > > libxl_cpupool_list() enumerates the cpupools by "probing": calling > > cpupool_info, starting at 0 and stopping when it gets an error. However, > > cpupool_info will print an error when the call to xc_cpupool_getinfo() fails, > > resulting in every xl command that uses libxl_list_cpupool (such as > > cpupool-list) printing that error message spuriously. > > I see: > # xl -vvv cpupool-list > Name CPUs Sched Active Domain count > Pool-0 4 credit y 2 > xc: debug: hypercall buffer: total allocations:5 total releases:5 > xc: debug: hypercall buffer: current allocations:0 maximum allocations:2 > xc: debug: hypercall buffer: cache current size:2 > xc: debug: hypercall buffer: cache hits:3 misses:2 toobig:0 > > which doesn''t seem to include the error message. However by code > inspection I''m sure I should be seeing what you describe. WTF?Nevermind this bit, I''d forgotten I''d stuck 4.1 on my test box...> > > This patch adds a "probe" argument to cpupool_info(). If set, it won''t print > > a warning if the xc_cpupool_getinfo() fails with ENOENT. > > Looking at the callers I think the existing "exact" parameter could be > used instead of a new param -- it would be fine to fail silently on > ENOENT iff !exact, I think. > > Ian.
Ian Campbell
2012-Aug-31 08:16 UTC
Re: [PATCH] xl: Suppress spurious warning message for cpupool-list
On Fri, 2012-08-17 at 09:21 +0100, Ian Campbell wrote:> On Wed, 2012-08-15 at 10:28 +0100, George Dunlap wrote: > > # HG changeset patch > > # User George Dunlap <george.dunlap@eu.citrix.com> > > # Date 1345022863 -3600 > > # Node ID 0982bad392e4f96fb39a025d6528c33be32c6c04 > > # Parent dc56a9defa30312a46cfb6ddb578e64cfbc6bc8b > > xl: Suppress spurious warning message for cpupool-list > > > > libxl_cpupool_list() enumerates the cpupools by "probing": calling > > cpupool_info, starting at 0 and stopping when it gets an error. However, > > cpupool_info will print an error when the call to xc_cpupool_getinfo() fails, > > resulting in every xl command that uses libxl_list_cpupool (such as > > cpupool-list) printing that error message spuriously. > [...] > > This patch adds a "probe" argument to cpupool_info(). If set, it won''t print > > a warning if the xc_cpupool_getinfo() fails with ENOENT. > > Looking at the callers I think the existing "exact" parameter could be > used instead of a new param -- it would be fine to fail silently on > ENOENT iff !exact, I think.Hi George, were you intending to do this for 4.2? Ian.
George Dunlap
2012-Nov-20 16:03 UTC
Re: [PATCH] xl: Suppress spurious warning message for cpupool-list
On Fri, Aug 17, 2012 at 9:21 AM, Ian Campbell <Ian.Campbell@citrix.com>wrote:> > This patch adds a "probe" argument to cpupool_info(). If set, it won''t > print > > a warning if the xc_cpupool_getinfo() fails with ENOENT. > > Looking at the callers I think the existing "exact" parameter could be > used instead of a new param -- it would be fine to fail silently on > ENOENT iff !exact, I think. >Hmm -- those two don''t seem to me to necessarily mean the same thing; it''s not clear to me that "exact==true" would imply "print an error message" and "exact==false" would imply "don''t print an error message". But at the moment the two callers of the function pass (true,false) and (false,true) for those booleans, and it is an internal function, so if it turns out we need to make a distinction we can always add the extra parameter in later. New patch on the way... -George _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Ian Campbell
2012-Nov-20 16:08 UTC
Re: [PATCH] xl: Suppress spurious warning message for cpupool-list
On Tue, 2012-11-20 at 16:03 +0000, George Dunlap wrote:> On Fri, Aug 17, 2012 at 9:21 AM, Ian Campbell > <Ian.Campbell@citrix.com> wrote: > > This patch adds a "probe" argument to cpupool_info(). If > set, it won''t print > > a warning if the xc_cpupool_getinfo() fails with ENOENT. > > > Looking at the callers I think the existing "exact" parameter > could be > used instead of a new param -- it would be fine to fail > silently on > ENOENT iff !exact, I think. > > Hmm -- those two don''t seem to me to necessarily mean the same thing; > it''s not clear to me that "exact==true" would imply "print an error > message" and "exact==false" would imply "don''t print an error > message".I think my logic was: If I asked for an exact thing and didn''t get it then that is worth logging. But if I didn''t ask for something exact then it doesn''t really matter if it can''t be found so no point in logging. If you see what I mean.> > But at the moment the two callers of the function pass (true,false) > and (false,true) for those booleans, and it is an internal function, > so if it turns out we need to make a distinction we can always add the > extra parameter in later. > > New patch on the way... > > -George >