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

[Li, Liang Z]

> 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 
> 
> 
> 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

That's better. I will change in next version.
And there is another issue similar as we solved to speed up the 
inflating/deflating process.
Should we use a large page bitmap or a small one ? I used a big one in the 
patch.

If we chose to use a small page bitmap, then we have to traverse the free page 
list for many times,
and the meaning of free page will be more fuzzy.

But if we use a big map bitmap, people may ask, why a small one here and a big 
one there?

Thanks!

Liang 

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

[Michael S. Tsirkin]

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 


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,
> +   , sizeof(uint32_t));
> +page_shift = virtio_ldl_p(vdev, );
> +offset += sizeof(uint32_t);
> +s->page_shift = page_shift;
> +
> +iov_to_buf(elem->out_sg, elem->out_num, offset,
> +   , sizeof(uint64_t));
> +base_pfn = virtio_ldq_p(vdev, );
> +offset += sizeof(uint64_t);
> +s->base_pfn = base_pfn;
> +
> +iov_to_buf(elem->out_sg, elem->out_num, offset,
> +   , sizeof(uint64_t));
> +bmap_len = virtio_ldq_p(vdev, );
> +offset += sizeof(uint64_t);
> +if (s->bmap_len < bmap_len) {
> + 

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

[Liang Li]

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 
---
 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,
+   , sizeof(uint32_t));
+page_shift = virtio_ldl_p(vdev, );
+offset += sizeof(uint32_t);
+s->page_shift = page_shift;
+
+iov_to_buf(elem->out_sg, elem->out_num, offset,
+   , sizeof(uint64_t));
+base_pfn = virtio_ldq_p(vdev, );
+offset += sizeof(uint64_t);
+s->base_pfn = base_pfn;
+
+iov_to_buf(elem->out_sg, elem->out_num, offset,
+   , sizeof(uint64_t));
+bmap_len = virtio_ldq_p(vdev, );
+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 =