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

[Daniel Veillard]

On Tue, Dec 08, 2009 at 02:57:15PM -0500, Adam Litke wrote:
> Set up the types for the domainMemStats function and insert it into the
> virDriver structure definition.  Because of static initializers, update every
> driver and set the new field to NULL.
> 

>  include/libvirt/libvirt.h.in |   31 +++++++++++++++++++++++++++++++

> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index f51a565..e430599 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -333,6 +333,34 @@ struct _virDomainInterfaceStats {
>   */
>  typedef virDomainInterfaceStatsStruct *virDomainInterfaceStatsPtr;
>  
> +/**
> + * virDomainMemStats:
> + *
> + * Memory stats for virDomainMemStats.
> + *
> + * Hypervisors may return a field set to ((long long)-1) which indicates
> + * that the hypervisor or guest does not support that statistic.
> + *
> + * NB. Here 'long long' means 64 bit integer.
> + */
> +typedef struct _virDomainMemStats virDomainMemStatsStruct;
> +
> +struct _virDomainMemStats {
> +    unsigned long long swap_in;
> +    unsigned long long swap_out;
> +    unsigned long long major_fault;
> +    unsigned long long minor_fault;
> +    unsigned long long mem_free;
> +    unsigned long long mem_tot;
> +};
> +
> +/**
> + * virDomainMemStatsPtr:
> + *
> + * A pointer to a virDomainMemStats structure
> + */
> +typedef virDomainMemStatsStruct *virDomainMemStatsPtr;
> +
>  
> +int                     virDomainMemStats (virDomainPtr dom,
> +                                           virDomainMemStatsPtr stats,
> +                                           size_t size);
>  int                     virDomainBlockPeek (virDomainPtr dom,
>                                              const char *path,
>                                              unsigned long long offset,

  This is the part where we need discussion, if we introduce an new API
it cannot be tuned for a single hypervisor, and as raised already the
amount of informations exposed there don't match what other are
providing.
  IMHO everything else is rather simple compared to the API definition.

The problem of a struct like this is that it doesn't allow expansion,
which is why I assume you added the size argument, but it's different
from what we did for the other entry points, I'm not sure it will really
be sufficient to handle all cases, for example if you remote call to a
libvirtd on an older version how do you handle the structure size change
and mostly the initialization of the fields unknown on the remote end.

The semantic of all your fields need to be precisely described (there
isn't even a comment for them !) for examples can you formally define
the difference of semantic between a major_fault and a minor_fault ?
Would mem_free include usage say by the buffer cache on linux ? What
mem_tot means the memory used ? the memory available ? etc ...

  You need to define precisely the semantic of all the values you
want to export, then we can compare with the semantic exported by other
hypervisors, and make a decision. Without it, there is now way we can
make use of that patch or API in general.

  thanks !

Daniel

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