[PATCH] xl: Suppress spurious warning message for cpupool-list

[PATCH] xl: Suppress spurious warning message for cpupool-list

[George Dunlap]

# 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) {

Re: [PATCH] xl: Suppress spurious warning message for cpupool-list

[Ian Campbell]

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.

Re: [PATCH] xl: Suppress spurious warning message for cpupool-list

[Ian Campbell]

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.

Re: [PATCH] xl: Suppress spurious warning message for cpupool-list

[Ian Campbell]

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.

Re: [PATCH] xl: Suppress spurious warning message for cpupool-list

[George Dunlap]

[-- Attachment #1.1: Type: text/plain, Size: 881 bytes --]

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

[-- Attachment #1.2: Type: text/html, Size: 1311 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH] xl: Suppress spurious warning message for cpupool-list

[Ian Campbell]

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
> 

[PATCH] xl: Suppress spurious warning message for cpupool-list

[George Dunlap]

# HG changeset patch
# User George Dunlap <george.dunlap@eu.citrix.com>
# Date 1353427397 0
# Node ID ab313e9521f44bdd91a20a1b3eb499b2ee321b5e
# Parent  ae6fb202b233af815466055d9f1a635802a50855
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.

Since at the moment the times we want to print the message correspond
with the use of the existing "exact" parameter, use it to decide
whether to print the message or not.

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
@@ -593,7 +593,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 (exact || errno != ENOENT)
+            LOGE(ERROR, "failed to get info for cpupool%d\n", poolid);
         return ERROR_FAIL;
     }

Re: [PATCH] xl: Suppress spurious warning message for cpupool-list

[George Dunlap]

[-- Attachment #1.1: Type: text/plain, Size: 1643 bytes --]

I suppose I should have labelled this one "v2" -- oops. :-)
 -George


On Tue, Nov 20, 2012 at 4:09 PM, George Dunlap
<george.dunlap@eu.citrix.com>wrote:

> # HG changeset patch
> # User George Dunlap <george.dunlap@eu.citrix.com>
> # Date 1353427397 0
> # Node ID ab313e9521f44bdd91a20a1b3eb499b2ee321b5e
> # Parent  ae6fb202b233af815466055d9f1a635802a50855
> 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.
>
> Since at the moment the times we want to print the message correspond
> with the use of the existing "exact" parameter, use it to decide
> whether to print the message or not.
>
> 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
> @@ -593,7 +593,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 (exact || errno != ENOENT)
> +            LOGE(ERROR, "failed to get info for cpupool%d\n", poolid);
>          return ERROR_FAIL;
>      }
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>

[-- Attachment #1.2: Type: text/html, Size: 2319 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH] xl: Suppress spurious warning message for cpupool-list

[Ian Campbell]

On Tue, 2012-11-20 at 16:09 +0000, George Dunlap wrote:
> # HG changeset patch
> # User George Dunlap <george.dunlap@eu.citrix.com>
> # Date 1353427397 0
> # Node ID ab313e9521f44bdd91a20a1b3eb499b2ee321b5e
> # Parent  ae6fb202b233af815466055d9f1a635802a50855
> 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.
> 
> Since at the moment the times we want to print the message correspond
> with the use of the existing "exact" parameter, use it to decide
> whether to print the message or not.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com> + applied, thanks.

Ian J -- a 4.2 backport candidate.
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -593,7 +593,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 (exact || errno != ENOENT)
> +            LOGE(ERROR, "failed to get info for cpupool%d\n", poolid);
>          return ERROR_FAIL;
>      }
>  
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] xl: Suppress spurious warning message for cpupool-list

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [PATCH] xl: Suppress spurious warning message for cpupool-list"):
> > xl: Suppress spurious warning message for cpupool-list
...
> Acked-by: Ian Campbell <ian.campbell@citrix.com> + applied, thanks.
> 
> Ian J -- a 4.2 backport candidate.

Done, thanks.

Ian.