Re: [PATCH 6/6] xl: 'xl list' supports '-c' for global claim information.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Mon, Mar 18, 2013 at 09:42:14AM +0000, Ian Campbell wrote:
> On Fri, 2013-03-15 at 18:05 +0000, Konrad Rzeszutek Wilk wrote:
> > On Wed, Mar 13, 2013 at 04:03:14PM +0000, Ian Campbell wrote:
> > > On Wed, 2013-03-13 at 15:02 +0000, Konrad Rzeszutek Wilk wrote:
> > > > On Wed, Mar 13, 2013 at 10:51:18AM +0000, Ian Campbell wrote:
> > > > > On Mon, 2013-03-11 at 14:20 +0000, Konrad Rzeszutek Wilk wrote:
> > > > > > When guests have XENMEM_claim_pages called, they influence a global
> > > > > > counter value - which has the cumulative count of the number of
> > > > > > pages requested by all domains. This value is provided via the
> > > > > > XENMEM_get_unclaimed_pages hypercall. The value fluctuates quite
> > > > > > often so the value is stale once it is provided to the user-space.
> > > > > > However it is useful for diagnostic purposes.
> > > > > > 
> > > > > > [v1: s/unclaimed/outstanding/]
> > > > > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > > > > > ---
> > > > > >  tools/libxl/libxl.c         | 12 ++++++++++++
> > > > > >  tools/libxl/libxl.h         |  1 +
> > > > > >  tools/libxl/libxl_types.idl |  4 ++++
> > > > > >  tools/libxl/xl_cmdimpl.c    | 29 +++++++++++++++++++++++++----
> > > > > >  tools/libxl/xl_cmdtable.c   |  4 +++-
> > > > > >  5 files changed, 45 insertions(+), 5 deletions(-)
> > > > > > 
> > > > > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > > > > > index 0745888..fd5d725 100644
> > > > > > --- a/tools/libxl/libxl.c
> > > > > > +++ b/tools/libxl/libxl.c
> > > > > > @@ -4051,6 +4051,18 @@ libxl_numainfo *libxl_get_numainfo(libxl_ctx *ctx, int *nr)
> > > > > >      return ret;
> > > > > >  }
> > > > > >  
> > > > > > +int libxl_get_claiminfo(libxl_ctx *ctx, libxl_claiminfo *claiminfo)
> > > > > > +{
> > > > > > +    long l;
> > > > > > +
> > > > > > +    l = xc_domain_get_outstanding_pages(ctx->xch);
> > > > > > +    if (l == -ENOSYS)
> > > > > 
> > > > > Does this function really return -errno and not -1 + set errno on error?
> > > > 
> > > > It should be -Exxxx. I need to double check with a hypervisor that does not
> > > > have this hypercall implemented to make sure it actually does return
> > > > a proper -E value.
> > > 
> > > You should probably double check the call chain to make sure it
> > > propagates the way you think. Remember that libc (not libxc) translates
> > > the return value of the underlying ioctl used to make the hypercall from
> > > -Efoo into -1 + errno=Efoo. Some libxc call paths then jump through
> > > hoops to turn this back into -Efoo :-( Some libxc calls do either
> > > depending on the error path they take :-((
> > 
> > It looks as it gets the righ value:
> > > xl create vm-hvm.cfg 
> > Parsing config from vm-hvm.cfg
> > WARNING: ignoring "kernel" directive for HVM guest. Use "firmware_override" instead if you really want a non-default firmware
> > xc: info: VIRTUAL MEMORY ARRANGEMENT:
> >   Loader:        0000000000100000->000000000019c844
> >   Modules:       0000000000000000->0000000000000000
> >   TOTAL:         0000000000000000->00000000ff800000
> >   ENTRY ADDRESS: 0000000000100000
> > xc: error: Could not allocate memory for HVM guest as we cannot claim memory! (38 = Function not implemented): Internal error
> 
> I don't think this path uses xc_domain_get_outstanding_pages does it? I
> expect it is calling xc_domain_claim_pages(). This message calls PERROR

Right, xc_domain_claim_pges.
> which uses errno not -err, which in a round about way shows that
> xc_domain_get_outstanding_pages does set errno and return -1 (since both
> use do_memory_op) and doesn't return -errno (i.e. it "proves" the
> opposite to what you were trying to prove...)

Yes. I realized that after I sent the email - and then redid the code to
take care of that.
> 
> > libxl: error: libxl_dom.c:613:libxl__build_hvm: hvm building failed
> > libxl: error: libxl_create.c:907:domcreate_rebuild_done: cannot (re-)build domain: -3
> > libxl: error: libxl_dm.c:1250:libxl__destroy_device_model: could not find device-model's pid for dom 1
> > libxl: error: libxl.c:1415:libxl__destroy_domid: libxl__destroy_device_model failed for 1
> > 
> > 18:02:45 # 22 :/OVS/seed_pool/konrad/ 
> > > echo $?
> > 3
> > 
> > and when running 'xl info' through strace:
> > 
> > ioctl(5, SNDCTL_DSP_RESET, 0x7fffb7a4ebe0) = -1 ENOSYS (Function not implemented)
> > write(2, "libxl_get_claiminfo failed.\n", 28libxl_get_claiminfo failed.
> > ) = 28
> 
> Does this print leak into the xl info output? Would be preferable to
> keep it silent I think, or to integrate cleanly with the output (e..g y
> outputting "0" or "-" or...)


Yup. Cleaned it up.
> 
> Ian.
> 
> 
> > 
> 
>