[libvirt] [PATCH 1/6] domMemStats: Add domainMemoryStats method to struct _virDriver

[Matthias Bolte]

2009/12/11 Adam Litke <agl at 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 <agl at us.ibm.com>
> To: libvirt list <libvir-list at 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.

> +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.

Matthias