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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-05-13 at 11:17 +0100, Ian Campbell wrote:
> 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?

Given that you now need to resend anyway (see the build issue  sent
separately), you may as well fold this aspect in too...

Ian.


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