On Thu, Dec 17, 2009 at 12:39:35AM +0100, Matthias Bolte wrote:
> 2009/12/11 Adam Litke <a...@us.ibm.com>:
> > Set up the types for the domainMemoryStats function and insert it into the
> > virDriver structure definition.  Because of static initializers, update 
> > every
> > driver and set the new field to NULL.
> >
> > Note: The changes in python/* are to fix compiler errors.  The actual python
> > binding is implemented in a later patch.
> >
> > Signed-off-by: Adam Litke <a...@us.ibm.com>
> > To: libvirt list <libvir-list@redhat.com>
> > ---
> >  include/libvirt/libvirt.h.in |   52 
> > ++++++++++++++++++++++++++++++++++++++++++
> >  python/generator.py          |    4 ++-
> >  src/driver.h                 |    7 +++++
> >  src/esx/esx_driver.c         |    1 +
> >  src/lxc/lxc_driver.c         |    1 +
> >  src/opennebula/one_driver.c  |    1 +
> >  src/openvz/openvz_driver.c   |    1 +
> >  src/phyp/phyp_driver.c       |    1 +
> >  src/qemu/qemu_driver.c       |    1 +
> >  src/remote/remote_driver.c   |    1 +
> >  src/test/test_driver.c       |    1 +
> >  src/uml/uml_driver.c         |    1 +
> >  src/vbox/vbox_tmpl.c         |    1 +
> >  src/xen/xen_driver.c         |    1 +
> >  14 files changed, 73 insertions(+), 1 deletions(-)
> >
> > diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> > index f51a565..3416ed4 100644
> > --- a/include/libvirt/libvirt.h.in
> > +++ b/include/libvirt/libvirt.h.in
> > @@ -333,6 +333,55 @@ struct _virDomainInterfaceStats {
> >  */
> >  typedef virDomainInterfaceStatsStruct *virDomainInterfaceStatsPtr;
> >
> > +/**
> > + * Memory Statistics Tags:
> > + */
> > +typedef enum {
> > +    /* The total amount of data read from swap space (in bytes). */
> > +    VIR_MEMSTAT_SWAP_IN         = 0,
> > +    /* The total amount of memory written out to swap space (in bytes). */
> > +    VIR_MEMSTAT_SWAP_OUT        = 1,
> > +
> > +    /*
> > +     * Page faults occur when a process makes a valid access to virtual 
> > memory
> > +     * that is not available.  When servicing the page fault, if disk IO is
> > +     * required, it is considered a major fault.  If not, it is a minor 
> > fault.
> > +     * These are expressed as the number of faults that have occurred.
> > +     */
> > +    VIR_MEMSTAT_MAJOR_FAULT     = 2,
> > +    VIR_MEMSTAT_MINOR_FAULT     = 3,
> > +
> > +    /*
> > +     * The amount of memory left completely unused by the system.  Memory 
> > that
> > +     * is available but used for reclaimable caches should NOT be reported 
> > as
> > +     * free.  This value is expressed in bytes.
> > +     */
> > +    VIR_MEMSTAT_MEM_FREE        = 4,
> > +
> > +    /*
> > +     * The total amount of usable memory as seen by the domain.  This value
> > +     * may be less than the amount of memory assigned to the domain if a
> > +     * balloon driver is in use or if the guest OS does not initialize all
> > +     * assigned pages.  This value is expressed in bytes.
> > +     */
> > +    VIR_MEMSTAT_MEM_TOTAL       = 5,
> > +
> > +    /*
> > +     * The number of statistics supported by this version of the interface.
> > +     * To add new statistics, add them to the enum and increase this value.
> > +     */
> > +    VIR_MEMSTAT_NR_TAGS         = 6,
> > +} virDomainMemoryStatTags;
> 
> IMHO the enum member names should follow the general naming scheme
> (see src/conf/domain_conf.h for example)
> 
> VIR_DOMAIN_MEMORY_STAT_* instead of VIR_MEMSTAT_*
> 
> Maybe I'm nitpicking here, but I would rename
> 
> VIR_MEMSTAT_MEM_FREE to VIR_DOMAIN_MEMORY_STAT_UNUSED as in "unused by
> the guest"
> 
> and
> 
> VIR_MEMSTAT_MEM_TOTAL to VIR_DOMAIN_MEMORY_STAT_AVAILABLE as in
> "available to the guest"
> 
> IMHO 'available' is a better term here, because it reflects the fact
> that it excludes the memory claimed by the balloon driver better.

  Okay, agreed

> > +typedef struct _virDomainMemoryStat virDomainMemoryStatStruct;
> > +
> > +struct _virDomainMemoryStat {
> > +    virDomainMemoryStatTags tag;
> > +    unsigned long long val;
> > +};
> > +
> > +typedef virDomainMemoryStatStruct *virDomainMemoryStatPtr;
> > +
> >
> >  /* Domain migration flags. */
> >  typedef enum {
> > @@ -633,6 +682,9 @@ int                     virDomainInterfaceStats 
> > (virDomainPtr dom,
> >                                                  const char *path,
> >                                                  virDomainInterfaceStatsPtr 
> > stats,
> >                                                  size_t size);
> > +int                     virDomainMemoryStats (virDomainPtr dom,
> > +                                              virDomainMemoryStatPtr stats,
> > +                                              unsigned int nr_stats);
> 
> Maybe an 'unsigned int flags' parameter should be added, even if its
> unused now it may come in handy for future extensions.

  Ah, good point !!!
Adam, you don't need to propagate it yet though the drivers, but at the
public interface in libvirt.h and libvirt.c, let's add it !

 thanks,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to