On Thu, 2009-12-17 at 10:43 +0000, Daniel P. Berrange wrote:
> On Fri, Dec 11, 2009 at 03:26:12PM -0500, Adam Litke wrote:
> > Set up the types for the domainMemoryStats function and insert it into the
> >
> > +/**
> > + * 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;
>
> I'm a little wary of this last element 'VIR_MEMSTAT_NR_TAGS', since
> it is something that will obviously change as we add more stats. I
> see your intent is that the caller simply statically declare an array
> that is VIR_MEMSTAT_NR_TAGS in size. The only other alternative I see
> is to have an API for querying the number of supported statistics, but
> then that has the downside of requiring an extra API call (network
> round trip) for something we expect to be called quite frequently.
Yep, this dilemma is apparent to me as well. I think the only two
alternatives are: a query API (as you mention above), or going back to a
static stats structure. I agree that forcing two separate calls for
each operation would be unfortunate. And if we revert to using a static
stats structure, we just end up creating churn in that definition (while
introducing a new set of problems). So, it seems that
VIR_MEMSTAT_NR_TAGS is the least offensive way to solve the problem.
Would it help to rename it to something else (eg. VIR_MEMSTAT_NR_STATS)?
> > +
> > +typedef struct _virDomainMemoryStat virDomainMemoryStatStruct;
> > +
> > +struct _virDomainMemoryStat {
> > + virDomainMemoryStatTags tag;
>
> This should be an 'int', since enums have an ill-defined size for ABI
Ok.
--
Thanks,
Adam
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list