[Qemu-devel] Re: [RFC] virtio: Report new guest memory statistics pertinent to memory ballooning

[Anthony Liguori <aliguori@us.ibm.com>]

agl@linux.vnet.ibm.com wrote:
>     [RFC] virtio: Report new guest memory statistics pertinent to memory ballooning
>     
>     When using ballooning to manage overcommitted memory on a host, a system for
>     guests to communicate their memory usage to the host can provide information
>     that will minimize the impact of ballooning on the guests.  The current method
>     employs a daemon running in each guest that communicates memory statistics to a
>     host daemon at a specified time interval.  The host daemon aggregates this
>     information and inflates and/or deflates balloons according to the level of
>     host memory pressure.  This approach is effective but overly complex since a
>     daemon must be installed inside each guest and coordinated to communicate with
>     the host.  A simpler approach is to collect memory statistics in the virtio
>     balloon driver and communicate them to the host via the device config space.
>     
>     This patch implements the qemu side of the communication channel.  I will post
>     the kernel driver modifications in-reply to this message.
>     
>     I'd like to pose a few questions concerning my implementation:
>     
>      * Is there a better way to use feature codes than using one per variable?
>   

I think it's the right granularity personally.

>      * This patch causes the 'info balloon' monitor command to generate output like
>        the following:
>     
>     	(qemu) info balloon
>     	balloon: actual=1024 MB
>     	balloon: pswapin=0 pages
>     	balloon: pswapout=0 pages
>     	balloon: panon=3928 KB
>     	balloon: pgmajfault=0
>     	balloon: pgminfault=247914
>     	balloon: memfree=987032 KB
>     	balloon: memtot=1020812 KB
>   

We could probably use less obscure names for these stats like 
total_memory.  I know we're facing a global shortage of bits but I think 
this is excessive conservation :-)

We typically print these out as:

 balloon: actual=1024,pswapin=0,pswapout=0,panon=3928...

It's a bit less readable admittedly, but it's how things tend to work.

>    
>        Is this agreeable?
>     
>     Thank you for your comments...
>     
>     Signed-off-by: Adam Litke <agl@us.ibm.com>\
>   

The indent is pretty strange here.  git-send-email should help.

> diff --git a/balloon.h b/balloon.h
> index 60b4a5d..3f67021 100644
> --- a/balloon.h
> +++ b/balloon.h
> @@ -14,14 +14,21 @@
>  #ifndef _QEMU_BALLOON_H
>  #define _QEMU_BALLOON_H
>
> +#include "hw/virtio-balloon.h"
>  #include "cpu-defs.h"
>
> -typedef ram_addr_t (QEMUBalloonEvent)(void *opaque, ram_addr_t target);
> +struct QEMUBalloonState {
> +    ram_addr_t actual;
> +    struct virtio_balloon_stats stats;
> +};
>   

We like to typedef the struct away from structures in qemu.  See 
CodingStyle.

The rest looks good.

-- 
Regards,

Anthony Liguori