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

[Anthony Liguori]

address@hidden 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 <address@hidden>\
>   

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