Re: [Qemu-devel] [QEMU 4/7] balloon: get free page info from guest

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Jun 13, 2016 at 06:16:46PM +0800, Liang Li wrote:
> Add a new feature to get the free page information from guest,
> the free page information is saved in a bitmap. Please note that
> 'free page' only means these pages are free before the request,
> some of the pages will become no free during the process of
> sending the free page bitmap to QEMU.
> 
> Signed-off-by: Liang Li <liang.z.li@intel.com>


I don't much like this request interface.
The meaning of free page is rather fuzzy too - so at what
point are they free?


My suggestion would be:
	report free page request ID to guest
	include request ID when guest sends free page list

the definition is then:
	page was free sometime after host set this value of request
	ID and before it received response with the same ID





> ---
>  balloon.c                          | 24 +++++++++++-
>  hw/virtio/virtio-balloon.c         | 75 +++++++++++++++++++++++++++++++++++++-
>  include/hw/virtio/virtio-balloon.h |  4 ++
>  include/sysemu/balloon.h           |  8 ++++
>  4 files changed, 108 insertions(+), 3 deletions(-)
> 
> diff --git a/balloon.c b/balloon.c
> index 3d96111..c74c472 100644
> --- a/balloon.c
> +++ b/balloon.c
> @@ -37,6 +37,7 @@
>  static QEMUBalloonEvent *balloon_event_fn;
>  static QEMUBalloonStatus *balloon_stat_fn;
>  static QEMUBalloonDropCache *balloon_drop_cache_fn;
> +static QEMUBalloonGetFreePage *balloon_get_free_page_fn;
>  static void *balloon_opaque;
>  static bool balloon_inhibited;
>  
> @@ -68,10 +69,11 @@ static bool have_balloon(Error **errp)
>  int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
>                               QEMUBalloonStatus *stat_func,
>                               QEMUBalloonDropCache *drop_cache_func,
> +                             QEMUBalloonGetFreePage *get_free_page_func,
>                               void *opaque)
>  {
>      if (balloon_event_fn || balloon_stat_fn || balloon_drop_cache_fn
> -        || balloon_opaque) {
> +        || balloon_get_free_page_fn || balloon_opaque) {
>          /* We're already registered one balloon handler.  How many can
>           * a guest really have?
>           */
> @@ -80,6 +82,7 @@ int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
>      balloon_event_fn = event_func;
>      balloon_stat_fn = stat_func;
>      balloon_drop_cache_fn = drop_cache_func;
> +    balloon_get_free_page_fn = get_free_page_func;
>      balloon_opaque = opaque;
>      return 0;
>  }
> @@ -92,6 +95,7 @@ void qemu_remove_balloon_handler(void *opaque)
>      balloon_event_fn = NULL;
>      balloon_stat_fn = NULL;
>      balloon_drop_cache_fn = NULL;
> +    balloon_get_free_page_fn = NULL;
>      balloon_opaque = NULL;
>  }
>  
> @@ -141,3 +145,21 @@ void qmp_balloon_drop_cache(DropCacheType type, Error **errp)
>  
>      balloon_drop_cache_fn(balloon_opaque, type);
>  }
> +
> +bool balloon_free_pages_support(void)
> +{
> +    return balloon_get_free_page_fn ? true : false;
> +}
> +
> +BalloonReqStatus balloon_get_free_pages(unsigned long *bitmap, unsigned long len)
> +{
> +    if (!balloon_get_free_page_fn) {
> +        return REQ_UNSUPPORT;
> +    }
> +
> +    if (!bitmap) {
> +        return REQ_INVALID_PARAM;
> +    }
> +
> +    return balloon_get_free_page_fn(balloon_opaque, bitmap, len);
> +}
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 4757ba5..30ba074 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -38,6 +38,7 @@
>  
>  enum balloon_req_id {
>         BALLOON_DROP_CACHE,
> +       BALLOON_GET_FREE_PAGES,
>  };
>  
>  static void balloon_page(void *addr, int deflate)
> @@ -435,7 +436,8 @@ static void virtio_balloon_handle_resp(VirtIODevice *vdev, VirtQueue *vq)
>      VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
>      VirtQueueElement *elem;
>      size_t offset = 0;
> -    uint32_t tmp32, id = 0;
> +    uint32_t tmp32, id = 0, page_shift;
> +    uint64_t base_pfn, tmp64, bmap_len;
>  
>      elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
>      if (!elem) {
> @@ -457,6 +459,32 @@ static void virtio_balloon_handle_resp(VirtIODevice *vdev, VirtQueue *vq)
>      case BALLOON_DROP_CACHE:
>          s->req_status = REQ_DONE;
>          break;
> +    case BALLOON_GET_FREE_PAGES:
> +        iov_to_buf(elem->out_sg, elem->out_num, offset,
> +                   &tmp32, sizeof(uint32_t));
> +        page_shift = virtio_ldl_p(vdev, &tmp32);
> +        offset += sizeof(uint32_t);
> +        s->page_shift = page_shift;
> +
> +        iov_to_buf(elem->out_sg, elem->out_num, offset,
> +                   &tmp64, sizeof(uint64_t));
> +        base_pfn = virtio_ldq_p(vdev, &tmp64);
> +        offset += sizeof(uint64_t);
> +        s->base_pfn = base_pfn;
> +
> +        iov_to_buf(elem->out_sg, elem->out_num, offset,
> +                   &tmp64, sizeof(uint64_t));
> +        bmap_len = virtio_ldq_p(vdev, &tmp64);
> +        offset += sizeof(uint64_t);
> +        if (s->bmap_len < bmap_len) {
> +             s->req_status = REQ_INVALID_PARAM;
> +             return;
> +        }
> +
> +        iov_to_buf(elem->out_sg, elem->out_num, offset,
> +                   s->free_page_bmap, bmap_len);
> +        s->req_status = REQ_DONE;
> +       break;
>      default:
>          break;
>      }
> @@ -574,6 +602,48 @@ static int virtio_balloon_drop_cache(void *opaque, unsigned long type)
>      return REQ_DONE;
>  }
>  
> +static BalloonReqStatus virtio_balloon_free_pages(void *opaque,
> +                                                  unsigned long *bitmap,
> +                                                  unsigned long bmap_len)
> +{
> +    VirtIOBalloon *s = opaque;
> +    VirtIODevice *vdev = VIRTIO_DEVICE(s);
> +    VirtQueueElement *elem = s->misc_vq_elem;
> +    int len;
> +
> +    if (!balloon_misc_supported(s)) {
> +        return REQ_UNSUPPORT;
> +    }
> +
> +    if (s->req_status == REQ_INIT) {
> +        s->free_page_bmap = bitmap;
> +        if (elem == NULL || !elem->in_num) {
> +            elem = virtqueue_pop(s->mvq, sizeof(VirtQueueElement));
> +            if (!elem) {
> +                return REQ_ERROR;
> +            }
> +            s->misc_vq_elem = elem;
> +        }
> +        s->misc_req.id = BALLOON_GET_FREE_PAGES;
> +        s->misc_req.param = 0;
> +        s->bmap_len = bmap_len;
> +        len = iov_from_buf(elem->in_sg, elem->in_num, 0, &s->misc_req,
> +                           sizeof(s->misc_req));
> +        virtqueue_push(s->mvq, elem, len);
> +        virtio_notify(vdev, s->mvq);
> +        g_free(s->misc_vq_elem);
> +        s->misc_vq_elem = NULL;
> +        s->req_status = REQ_ON_GOING;
> +        return REQ_ERROR;
> +    } else if (s->req_status == REQ_ON_GOING) {
> +        return REQ_ON_GOING;
> +    } else if (s->req_status == REQ_DONE) {
> +        s->req_status = REQ_INIT;
> +    }
> +
> +    return REQ_DONE;
> +}
> +
>  static void virtio_balloon_to_target(void *opaque, ram_addr_t target)
>  {
>      VirtIOBalloon *dev = VIRTIO_BALLOON(opaque);
> @@ -637,7 +707,8 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
>  
>      ret = qemu_add_balloon_handler(virtio_balloon_to_target,
>                                     virtio_balloon_stat,
> -                                   virtio_balloon_drop_cache, s);
> +                                   virtio_balloon_drop_cache,
> +                                   virtio_balloon_free_pages, s);
>  
>      if (ret < 0) {
>          error_setg(errp, "Only one balloon device is supported");
> diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
> index a21bb45..6382bcf 100644
> --- a/include/hw/virtio/virtio-balloon.h
> +++ b/include/hw/virtio/virtio-balloon.h
> @@ -60,6 +60,10 @@ typedef struct VirtIOBalloon {
>      uint32_t host_features;
>      MiscReq misc_req;
>      BalloonReqStatus req_status;
> +    uint64_t *free_page_bmap;
> +    uint64_t bmap_len;
> +    uint64_t base_pfn;
> +    uint32_t page_shift;
>  } VirtIOBalloon;
>  
>  #endif
> diff --git a/include/sysemu/balloon.h b/include/sysemu/balloon.h
> index 0e85f2b..6c362e8 100644
> --- a/include/sysemu/balloon.h
> +++ b/include/sysemu/balloon.h
> @@ -15,17 +15,25 @@
>  #define _QEMU_BALLOON_H
>  
>  #include "qapi-types.h"
> +#include "hw/virtio/virtio-balloon.h"
>  
>  typedef void (QEMUBalloonEvent)(void *opaque, ram_addr_t target);
>  typedef void (QEMUBalloonStatus)(void *opaque, BalloonInfo *info);
>  typedef int (QEMUBalloonDropCache)(void *opaque, unsigned long ctrl);
> +typedef BalloonReqStatus (QEMUBalloonGetFreePage)(void *opaque,
> +                                                  unsigned long *bitmap,
> +                                                  unsigned long len);
>  
>  int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
>                               QEMUBalloonStatus *stat_func,
>                               QEMUBalloonDropCache *drop_cache_func,
> +                             QEMUBalloonGetFreePage *get_free_page_func,
>                               void *opaque);
>  void qemu_remove_balloon_handler(void *opaque);
>  bool qemu_balloon_is_inhibited(void);
>  void qemu_balloon_inhibit(bool state);
> +bool balloon_free_pages_support(void);
> +BalloonReqStatus balloon_get_free_pages(unsigned long *bitmap,
> +                                        unsigned long len);
>  
>  #endif
> -- 
> 1.9.1