Re: [Xen-devel] [PATCH 4/5] libxl: claim: Print the values in 'xl info' unconditionally

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2013-05-10 at 22:00 +0100, Konrad Rzeszutek Wilk wrote:
> During the review of "libxl: Change claim_mode from bool to int."
> Ian Campbell suggested that the xl info should print the
> claim information irregardless of the global claim_mode value.
> 
> Suggested-by: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> ---
>  tools/libxl/xl_cmdimpl.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index c3e1183..bb7a7af 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -4604,11 +4604,7 @@ static void output_physinfo(void)
>          printf("sharing_freed_memory   : %"PRIu64"\n", 
> info.sharing_freed_pages / i);
>          printf("sharing_used_memory    : %"PRIu64"\n", 
> info.sharing_used_frames / i);
>      }
> -    /*
> -     * Only if enabled (claim_mode=1) or there are outstanding claims.
> -     */
> -    if (claim_mode || info.outstanding_pages)
> -        printf("outstanding_claims     : %ld\n", info.outstanding_pages / i);
> +    printf("outstanding_claims     : %ld\n", info.outstanding_pages / i);

I here is only initialised within the previous "if (vinfo)" (the tail of
which is right above).

This printf should probably therefore have always been inside that same
block. (the horrible use of the variable i as something other than a
loop iterator is probably at least partly to blame for the confusion)

This patch isn't making this any worse so I'll Ack + apply but perhaps
you could send a follow up to fix this?

>  
>      if (!libxl_get_freecpus(ctx, &cpumap)) {
>          libxl_for_each_bit(i, cpumap)



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