[libvirt] [PATCH 2/6] domMemoryStats: Add public symbol to libvirt API

[Adam Litke]

On Thu, 2009-12-17 at 10:37 +0000, Daniel P. Berrange wrote:
> On Fri, Dec 11, 2009 at 03:26:13PM -0500, Adam Litke wrote:
> > Add a new function 'virDomainMemoryStats' to the libvirt API.  Export it via
> > libvirt_public.syms
> > 
> > Signed-off-by: Adam Litke <agl at us.ibm.com>
> > To: libvirt list <libvir-list at redhat.com>
> > ---
> >  src/libvirt.c           |   64 +++++++++++++++++++++++++++++++++++++++++++++++
> >  src/libvirt_public.syms |    2 +
> >  2 files changed, 66 insertions(+), 0 deletions(-)
> > 
> > diff --git a/src/libvirt.c b/src/libvirt.c
> > index b14942d..0af1ee3 100644
> > --- a/src/libvirt.c
> > +++ b/src/libvirt.c
> > @@ -4045,6 +4045,70 @@ error:
> >  }
> >  
> >  /**
> > + * virDomainMemStats:
> > + * @dom: pointer to the domain object
> > + * @stats: nr_stats-sized array of stat structures (returned)
> > + * @nr_stats: number of memory statistics requested
> > + * 
> > + * This function provides memory statistics for the domain.
> > + *
> > + * Up to 'nr_stats' elements of 'stats' will be populated with memory statistics
> > + * from the domain.  Only statistics supported by the domain, the driver, and
> > + * this version of libvirt will be returned.
> 
> What is the intended return value if a driver does not support a 
> particular field. Should it be initialized to '0' or -1.

As the API is currently designed, it is perfectly normal for a driver to
return fewer stats than were asked for by the caller.  In this case, the
caller knows how many elements of the stats array can be accessed based
on the return value.  So, if a driver does not support a statistic, that
tag and value should simply be omitted from the returned result.

> 
> Also, I'm wondering does the calling application need to initialize
> the '@stats' paramter at all, eg filling in the 'tag' with the 
> VIR_MEMSTAT_XXX constants for which they want data. Or must the
> application always supply the 'stats' array large enough to hold
> all types of stats ?

No, this is not necessary.  A non-negative return value from
virDomainMemoryStats() indicates the number of elements in the stats
array that have been initialized by the driver.  If the stats array
happens to be larger than the return value, the extra elements are
considered invalid.

Most callers of virDomainMemoryStats() will want all available stats and
thus should pass VIR_MEMSTAT_NR_TAGS as nr_stats.  This will ensure that
there is enough room to accommodate the superset of memory statistics
supported by the version of libvirt being used.

> > + * 
> > + * Memory Statistics:
> > + *
> > + * VIR_MEMSTAT_SWAP_IN:
> > + *     The total amount of data read from swap space (in bytes).
> > + * VIR_MEMSTAT_SWAP_OUT:
> > + *     The total amount of memory written out to swap space (in bytes).
> > + * VIR_MEMSTAT_MAJOR_FAULT:
> > + *     The number of page faults that required disk IO to service.
> > + * VIR_MEMSTAT_MINOR_FAULT:
> > + *     The number of page faults serviced without disk IO.
> > + * VIR_MEMSTAT_MEM_FREE:
> > + *     The amount of memory which is not being used for any purpose (in bytes).
> > + * VIR_MEMSTAT_MEM_TOTAL:
> > + *     The total amount of memory recognized by the domain's OS (in bytes).
> > + *
> > + * Returns: The number of stats provided or -1 in case of failure.
> > + */
> > +int virDomainMemoryStats (virDomainPtr dom, virDomainMemoryStatPtr stats,
> > +                          unsigned int nr_stats)
> > +{
> 
> > diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> > index 8921c1a..5761a7e 100644
> > --- a/src/libvirt_public.syms
> > +++ b/src/libvirt_public.syms
> > @@ -327,6 +327,8 @@ LIBVIRT_0.7.2 {
> >  	virStreamAbort;
> >  	virStreamFree;
> >  	virDomainMigrateToURI;
> > +        # XXX: For testing only -- Move to 0.7.3 section
> > +        virDomainMemoryStats;
> >  } LIBVIRT_0.7.1;
> 
> You should create the new section in your patch if it doesn't exist yet,
> using the next logical version number. Otherwise this is the kind of
> thing that can get missed if we pull your patchset straight into GIT

Ok.


-- 
Thanks,
Adam