Re: [virtio-dev] Re: [PATCH v4 3/4] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT

2018-03-15 Thread Michael S. Tsirkin 
On Thu, Mar 15, 2018 at 06:24:16PM +0800, Wei Wang wrote:
> On 03/15/2018 10:47 AM, Michael S. Tsirkin wrote:
> > On Thu, Mar 15, 2018 at 09:15:48AM +0800, Wei Wang wrote:
> > > On 03/14/2018 10:12 PM, Michael S. Tsirkin wrote:
> > > > On Wed, Mar 14, 2018 at 02:03:19PM +0800, Wei Wang wrote:
> > > > > On 03/14/2018 10:53 AM, Michael S. Tsirkin wrote:
> > > > > > On Wed, Mar 14, 2018 at 10:43:01AM +0800, Wei Wang wrote:
> > > > > > > On 03/14/2018 12:49 AM, Michael S. Tsirkin wrote:
> > > > > > > > On Wed, Mar 07, 2018 at 08:34:24PM +0800, Wei Wang wrote:
> > > > > > > > 
> > > > > > > > > Signed-off-by: Wei Wang 
> > > > > > > > > Signed-off-by: Liang Li 
> > > > > > > > > CC: Michael S. Tsirkin 
> > > > > > > > > CC: Dr. David Alan Gilbert 
> > > > > > > > > CC: Juan Quintela 
> > > > > > > > I find it suspicious that neither unrealize nor reset
> > > > > > > > functions have been touched at all.
> > > > > > > > Are you sure you have thought through scenarious like
> > > > > > > > hot-unplug or disabling the device by guest?
> > > > > > > OK. I think we can call balloon_free_page_stop in unrealize and 
> > > > > > > reset.
> > > > > > > 
> > > > > > > 
> > > > > > > > +static void *virtio_balloon_poll_free_page_hints(void *opaque)
> > > > > > > > +{
> > > > > > > > +VirtQueueElement *elem;
> > > > > > > > +VirtIOBalloon *dev = opaque;
> > > > > > > > +VirtQueue *vq = dev->free_page_vq;
> > > > > > > > +uint32_t id;
> > > > > > > > +size_t size;
> > > > > > > > What makes it safe to poke at this device from multiple threads?
> > > > > > > > I think that it would be safer to do it from e.g. BH.
> > > > > > > > 
> > > > > > > Actually the free_page_optimization thread is the only user of 
> > > > > > > free_page_vq,
> > > > > > > and there is only one optimization thread each time. Would this 
> > > > > > > be safe
> > > > > > > enough?
> > > > > > > 
> > > > > > > Best,
> > > > > > > Wei
> > > > > > Aren't there other fields there? Also things like reset affect all 
> > > > > > VQs.
> > > > > > 
> > > > > Yes. But I think BHs are used to avoid re-entrancy, which isn't the 
> > > > > issue
> > > > > here.
> > > > Since you are adding locks to address the issue - doesn't this imply
> > > > reentrancy is exactly the issue?
> > > Not really. The lock isn't intended for any reentrancy issues, since there
> > > will be only one run of the virtio_balloon_poll_free_page_hints function 
> > > at
> > > any given time. Instead, the lock is used to synchronize
> > > virtio_balloon_poll_free_page_hints and virtio_balloon_free_page_stop to
> > > access dev->free_page_report_status.
> > I wonder whether that's enough. E.g. is there a race with guest
> > trying to reset the device? That resets all VQs you know.
> 
> I think that's OK - we will call virtio_balloon_free_page_stop in the device
> reset function, and qemu_thread_join() in virtio_balloon_free_page_stop will
> wait till the optimization thread exits. That is, the reset will proceed
> after the optimization thread exits.
> 
> 
> > 
> > 
> > > Please see the whole picture below:
> > > 
> > > virtio_balloon_poll_free_page_hints()
> > > {
> > > 
> > >  while (1) {
> > >  qemu_spin_lock();
> > >  if (dev->free_page_report_status >= FREE_PAGE_REPORT_S_STOP ||
> > >  !runstate_is_running()) {
> > >  qemu_spin_unlock();
> > >  break;
> > >  }
> > >  ...
> > >  if (id == dev->free_page_report_cmd_id) {
> > > ==>dev->free_page_report_status = FREE_PAGE_REPORT_S_START;
> > >  ...
> > >  qemu_spin_unlock();
> > >  }
> > > }
> > > 
> > > 
> > > static void virtio_balloon_free_page_stop(void *opaque)
> > > {
> > >  VirtIOBalloon *s = opaque;
> > >  VirtIODevice *vdev = VIRTIO_DEVICE(s);
> > > 
> > >  qemu_spin_lock();
> > > ...
> > > ==>   s->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
> > >  ...
> > >  qemu_spin_unlock();
> > > }
> > > 
> > > 
> > > Without the lock, there are theoretical possibilities that assigning STOP
> > > below is overridden by START above. In that
> > > case,virtio_balloon_free_page_stop does not effectively stop
> > > virtio_balloon_poll_free_page_hints.
> > > I think this issue couldn't be solved by BHs.
> > > 
> > > Best,
> > > Wei
> > Don't all BHs run under the BQL?
> 
> Actually the virtio_balloon_free_page_stop is called by the migration thread
> (instead of a BH). Even we guarantee the migration thread calls
> virtio_balloon_free_page_stop under BQL, the BQL is still too big for our
> case. Imagine this case: when the migration thread calls
> virtio_balloon_free_page_stop to stop the reporting, it blocks by BQL as
> virtio_balloon_poll_free_page_hints is in progress with BQL held, and the
> migration thread won't proceed untill virtio_balloon_poll_free_page_hints
> exits (i.e. getting

Re: [virtio-dev] Re: [PATCH v4 3/4] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT

2018-03-15 Thread Wei Wang 

On 03/15/2018 10:47 AM, Michael S. Tsirkin wrote:

On Thu, Mar 15, 2018 at 09:15:48AM +0800, Wei Wang wrote:

On 03/14/2018 10:12 PM, Michael S. Tsirkin wrote:

On Wed, Mar 14, 2018 at 02:03:19PM +0800, Wei Wang wrote:

On 03/14/2018 10:53 AM, Michael S. Tsirkin wrote:

On Wed, Mar 14, 2018 at 10:43:01AM +0800, Wei Wang wrote:

On 03/14/2018 12:49 AM, Michael S. Tsirkin wrote:

On Wed, Mar 07, 2018 at 08:34:24PM +0800, Wei Wang wrote:


Signed-off-by: Wei Wang 
Signed-off-by: Liang Li 
CC: Michael S. Tsirkin 
CC: Dr. David Alan Gilbert 
CC: Juan Quintela 

I find it suspicious that neither unrealize nor reset
functions have been touched at all.
Are you sure you have thought through scenarious like
hot-unplug or disabling the device by guest?

OK. I think we can call balloon_free_page_stop in unrealize and reset.



+static void *virtio_balloon_poll_free_page_hints(void *opaque)
+{
+VirtQueueElement *elem;
+VirtIOBalloon *dev = opaque;
+VirtQueue *vq = dev->free_page_vq;
+uint32_t id;
+size_t size;
What makes it safe to poke at this device from multiple threads?
I think that it would be safer to do it from e.g. BH.


Actually the free_page_optimization thread is the only user of free_page_vq,
and there is only one optimization thread each time. Would this be safe
enough?

Best,
Wei

Aren't there other fields there? Also things like reset affect all VQs.


Yes. But I think BHs are used to avoid re-entrancy, which isn't the issue
here.

Since you are adding locks to address the issue - doesn't this imply
reentrancy is exactly the issue?

Not really. The lock isn't intended for any reentrancy issues, since there
will be only one run of the virtio_balloon_poll_free_page_hints function at
any given time. Instead, the lock is used to synchronize
virtio_balloon_poll_free_page_hints and virtio_balloon_free_page_stop to
access dev->free_page_report_status.

I wonder whether that's enough. E.g. is there a race with guest
trying to reset the device? That resets all VQs you know.


I think that's OK - we will call virtio_balloon_free_page_stop in the 
device reset function, and qemu_thread_join() in 
virtio_balloon_free_page_stop will wait till the optimization thread 
exits. That is, the reset will proceed after the optimization thread exits.







Please see the whole picture below:

virtio_balloon_poll_free_page_hints()
{

 while (1) {
 qemu_spin_lock();
 if (dev->free_page_report_status >= FREE_PAGE_REPORT_S_STOP ||
 !runstate_is_running()) {
 qemu_spin_unlock();
 break;
 }
 ...
 if (id == dev->free_page_report_cmd_id) {
==>dev->free_page_report_status = FREE_PAGE_REPORT_S_START;
 ...
 qemu_spin_unlock();
 }
}


static void virtio_balloon_free_page_stop(void *opaque)
{
 VirtIOBalloon *s = opaque;
 VirtIODevice *vdev = VIRTIO_DEVICE(s);

 qemu_spin_lock();
...
==>   s->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
 ...
 qemu_spin_unlock();
}


Without the lock, there are theoretical possibilities that assigning STOP
below is overridden by START above. In that
case,virtio_balloon_free_page_stop does not effectively stop
virtio_balloon_poll_free_page_hints.
I think this issue couldn't be solved by BHs.

Best,
Wei

Don't all BHs run under the BQL?


Actually the virtio_balloon_free_page_stop is called by the migration 
thread (instead of a BH). Even we guarantee the migration thread calls 
virtio_balloon_free_page_stop under BQL, the BQL is still too big for 
our case. Imagine this case: when the migration thread calls 
virtio_balloon_free_page_stop to stop the reporting, it blocks by BQL as 
virtio_balloon_poll_free_page_hints is in progress with BQL held, and 
the migration thread won't proceed untill 
virtio_balloon_poll_free_page_hints exits (i.e. getting all the hints). 
I think this isn't our intention - we basically want the migration 
thread to stop the guest reporting immediately.

So I think the small lock above is better (it locks for only one hint).

Best,
Wei

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] Re: [PATCH v4 3/4] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT

2018-03-14 Thread Michael S. Tsirkin 
On Thu, Mar 15, 2018 at 09:15:48AM +0800, Wei Wang wrote:
> On 03/14/2018 10:12 PM, Michael S. Tsirkin wrote:
> > On Wed, Mar 14, 2018 at 02:03:19PM +0800, Wei Wang wrote:
> > > On 03/14/2018 10:53 AM, Michael S. Tsirkin wrote:
> > > > On Wed, Mar 14, 2018 at 10:43:01AM +0800, Wei Wang wrote:
> > > > > On 03/14/2018 12:49 AM, Michael S. Tsirkin wrote:
> > > > > > On Wed, Mar 07, 2018 at 08:34:24PM +0800, Wei Wang wrote:
> > > > > > 
> > > > > > > Signed-off-by: Wei Wang 
> > > > > > > Signed-off-by: Liang Li 
> > > > > > > CC: Michael S. Tsirkin 
> > > > > > > CC: Dr. David Alan Gilbert 
> > > > > > > CC: Juan Quintela 
> > > > > > I find it suspicious that neither unrealize nor reset
> > > > > > functions have been touched at all.
> > > > > > Are you sure you have thought through scenarious like
> > > > > > hot-unplug or disabling the device by guest?
> > > > > OK. I think we can call balloon_free_page_stop in unrealize and reset.
> > > > > 
> > > > > 
> > > > > > +static void *virtio_balloon_poll_free_page_hints(void *opaque)
> > > > > > +{
> > > > > > +VirtQueueElement *elem;
> > > > > > +VirtIOBalloon *dev = opaque;
> > > > > > +VirtQueue *vq = dev->free_page_vq;
> > > > > > +uint32_t id;
> > > > > > +size_t size;
> > > > > > What makes it safe to poke at this device from multiple threads?
> > > > > > I think that it would be safer to do it from e.g. BH.
> > > > > > 
> > > > > Actually the free_page_optimization thread is the only user of 
> > > > > free_page_vq,
> > > > > and there is only one optimization thread each time. Would this be 
> > > > > safe
> > > > > enough?
> > > > > 
> > > > > Best,
> > > > > Wei
> > > > Aren't there other fields there? Also things like reset affect all VQs.
> > > > 
> > > Yes. But I think BHs are used to avoid re-entrancy, which isn't the issue
> > > here.
> > Since you are adding locks to address the issue - doesn't this imply
> > reentrancy is exactly the issue?
> 
> Not really. The lock isn't intended for any reentrancy issues, since there
> will be only one run of the virtio_balloon_poll_free_page_hints function at
> any given time. Instead, the lock is used to synchronize
> virtio_balloon_poll_free_page_hints and virtio_balloon_free_page_stop to
> access dev->free_page_report_status.

I wonder whether that's enough. E.g. is there a race with guest
trying to reset the device? That resets all VQs you know.


> Please see the whole picture below:
> 
> virtio_balloon_poll_free_page_hints()
> {
> 
> while (1) {
> qemu_spin_lock();
> if (dev->free_page_report_status >= FREE_PAGE_REPORT_S_STOP ||
> !runstate_is_running()) {
> qemu_spin_unlock();
> break;
> }
> ...
> if (id == dev->free_page_report_cmd_id) {
> ==>dev->free_page_report_status = FREE_PAGE_REPORT_S_START;
> ...
> qemu_spin_unlock();
> }
> }
> 
> 
> static void virtio_balloon_free_page_stop(void *opaque)
> {
> VirtIOBalloon *s = opaque;
> VirtIODevice *vdev = VIRTIO_DEVICE(s);
> 
> qemu_spin_lock();
> ...
> ==>   s->free_page_report_status = FREE_PAGE_REPORT_S_STOP;
> ...
> qemu_spin_unlock();
> }
> 
> 
> Without the lock, there are theoretical possibilities that assigning STOP
> below is overridden by START above. In that
> case,virtio_balloon_free_page_stop does not effectively stop
> virtio_balloon_poll_free_page_hints.
> I think this issue couldn't be solved by BHs.
> 
> Best,
> Wei

Don't all BHs run under the BQL?

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] Re: [PATCH v4 3/4] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT

2018-03-14 Thread Michael S. Tsirkin 
On Wed, Mar 14, 2018 at 02:03:19PM +0800, Wei Wang wrote:
> On 03/14/2018 10:53 AM, Michael S. Tsirkin wrote:
> > On Wed, Mar 14, 2018 at 10:43:01AM +0800, Wei Wang wrote:
> > > On 03/14/2018 12:49 AM, Michael S. Tsirkin wrote:
> > > > On Wed, Mar 07, 2018 at 08:34:24PM +0800, Wei Wang wrote:
> > > > 
> > > > > Signed-off-by: Wei Wang 
> > > > > Signed-off-by: Liang Li 
> > > > > CC: Michael S. Tsirkin 
> > > > > CC: Dr. David Alan Gilbert 
> > > > > CC: Juan Quintela 
> > > > I find it suspicious that neither unrealize nor reset
> > > > functions have been touched at all.
> > > > Are you sure you have thought through scenarious like
> > > > hot-unplug or disabling the device by guest?
> > > OK. I think we can call balloon_free_page_stop in unrealize and reset.
> > > 
> > > 
> > > > +static void *virtio_balloon_poll_free_page_hints(void *opaque)
> > > > +{
> > > > +VirtQueueElement *elem;
> > > > +VirtIOBalloon *dev = opaque;
> > > > +VirtQueue *vq = dev->free_page_vq;
> > > > +uint32_t id;
> > > > +size_t size;
> > > > What makes it safe to poke at this device from multiple threads?
> > > > I think that it would be safer to do it from e.g. BH.
> > > > 
> > > Actually the free_page_optimization thread is the only user of 
> > > free_page_vq,
> > > and there is only one optimization thread each time. Would this be safe
> > > enough?
> > > 
> > > Best,
> > > Wei
> > Aren't there other fields there? Also things like reset affect all VQs.
> > 
> 
> Yes. But I think BHs are used to avoid re-entrancy, which isn't the issue
> here.

Since you are adding locks to address the issue - doesn't this imply
reentrancy is exactly the issue?

> The potential issue I could observe here is that
> "dev->free_page_report_status" is read and written by the optimization
> thread, and it is also modified by the migration thread and reset via
> virtio_balloon_free_page_stop.
> 
> How about adding a QEMU SpinLock, like this:
> 
> virtio_balloon_poll_free_page_hints()
> {
> 
> while (1) {
> qemu_spin_lock();
> /* If the status has been changed to STOP or EXIT, or the VM is
> stopped, just exit */
> if (dev->free_page_report_status >= FREE_PAGE_REPORT_S_STOP ||
> !runstate_is_running()) {
> qemu_spin_unlock();
> break;
> }
> 
> qemu_spin_unlock();
> }
> }
> 
> 
> Best,
> Wei

That will address the issue but it does look weird.

-- 
MST

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] Re: [PATCH v4 3/4] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT

2018-03-14 Thread Wei Wang 

On 03/14/2018 10:53 AM, Michael S. Tsirkin wrote:

On Wed, Mar 14, 2018 at 10:43:01AM +0800, Wei Wang wrote:

On 03/14/2018 12:49 AM, Michael S. Tsirkin wrote:

On Wed, Mar 07, 2018 at 08:34:24PM +0800, Wei Wang wrote:


Signed-off-by: Wei Wang 
Signed-off-by: Liang Li 
CC: Michael S. Tsirkin 
CC: Dr. David Alan Gilbert 
CC: Juan Quintela 

I find it suspicious that neither unrealize nor reset
functions have been touched at all.
Are you sure you have thought through scenarious like
hot-unplug or disabling the device by guest?

OK. I think we can call balloon_free_page_stop in unrealize and reset.



+static void *virtio_balloon_poll_free_page_hints(void *opaque)
+{
+VirtQueueElement *elem;
+VirtIOBalloon *dev = opaque;
+VirtQueue *vq = dev->free_page_vq;
+uint32_t id;
+size_t size;
What makes it safe to poke at this device from multiple threads?
I think that it would be safer to do it from e.g. BH.


Actually the free_page_optimization thread is the only user of free_page_vq,
and there is only one optimization thread each time. Would this be safe
enough?

Best,
Wei

Aren't there other fields there? Also things like reset affect all VQs.



Yes. But I think BHs are used to avoid re-entrancy, which isn't the 
issue here.


The potential issue I could observe here is that 
"dev->free_page_report_status" is read and written by the optimization 
thread, and it is also modified by the migration thread and reset via 
virtio_balloon_free_page_stop.


How about adding a QEMU SpinLock, like this:

virtio_balloon_poll_free_page_hints()
{

while (1) {
qemu_spin_lock();
/* If the status has been changed to STOP or EXIT, or the VM is 
stopped, just exit */
if (dev->free_page_report_status >= FREE_PAGE_REPORT_S_STOP || 
!runstate_is_running()) {

qemu_spin_unlock();
break;
}

qemu_spin_unlock();
}
}


Best,
Wei

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] Re: [PATCH v4 3/4] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT

2018-03-13 Thread Michael S. Tsirkin 
On Wed, Mar 14, 2018 at 10:43:01AM +0800, Wei Wang wrote:
> On 03/14/2018 12:49 AM, Michael S. Tsirkin wrote:
> > On Wed, Mar 07, 2018 at 08:34:24PM +0800, Wei Wang wrote:
> > 
> > > Signed-off-by: Wei Wang 
> > > Signed-off-by: Liang Li 
> > > CC: Michael S. Tsirkin 
> > > CC: Dr. David Alan Gilbert 
> > > CC: Juan Quintela 
> > I find it suspicious that neither unrealize nor reset
> > functions have been touched at all.
> > Are you sure you have thought through scenarious like
> > hot-unplug or disabling the device by guest?
> 
> OK. I think we can call balloon_free_page_stop in unrealize and reset.
> 
> 
> > +static void *virtio_balloon_poll_free_page_hints(void *opaque)
> > +{
> > +VirtQueueElement *elem;
> > +VirtIOBalloon *dev = opaque;
> > +VirtQueue *vq = dev->free_page_vq;
> > +uint32_t id;
> > +size_t size;
> > What makes it safe to poke at this device from multiple threads?
> > I think that it would be safer to do it from e.g. BH.
> > 
> 
> Actually the free_page_optimization thread is the only user of free_page_vq,
> and there is only one optimization thread each time. Would this be safe
> enough?
> 
> Best,
> Wei

Aren't there other fields there? Also things like reset affect all VQs.

-- 
MST

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] Re: [PATCH v4 3/4] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT

2018-03-13 Thread Wei Wang 

On 03/14/2018 12:49 AM, Michael S. Tsirkin wrote:

On Wed, Mar 07, 2018 at 08:34:24PM +0800, Wei Wang wrote:


Signed-off-by: Wei Wang 
Signed-off-by: Liang Li 
CC: Michael S. Tsirkin 
CC: Dr. David Alan Gilbert 
CC: Juan Quintela 

I find it suspicious that neither unrealize nor reset
functions have been touched at all.
Are you sure you have thought through scenarious like
hot-unplug or disabling the device by guest?
   


OK. I think we can call balloon_free_page_stop in unrealize and reset.


  
+static void *virtio_balloon_poll_free_page_hints(void *opaque)

+{
+VirtQueueElement *elem;
+VirtIOBalloon *dev = opaque;
+VirtQueue *vq = dev->free_page_vq;
+uint32_t id;
+size_t size;
What makes it safe to poke at this device from multiple threads?
I think that it would be safer to do it from e.g. BH.



Actually the free_page_optimization thread is the only user of 
free_page_vq, and there is only one optimization thread each time. Would 
this be safe enough?


Best,
Wei

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH v4 3/4] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT

2018-03-13 Thread Michael S. Tsirkin 
On Wed, Mar 07, 2018 at 08:34:24PM +0800, Wei Wang wrote:
> The new feature enables the virtio-balloon device to receive hints of
> guest free pages from the free page vq.
> 
> balloon_free_page_start - start guest free page hint reporting.
> balloon_free_page_stop - stop guest free page hint reporting.
> 
> Note: balloon will report pages which were free at the time
> of this call. As the reporting happens asynchronously, dirty bit logging
> must be enabled before this call is made. Guest reporting must be
> disabled before the migration dirty bitmap is synchronized.

Add this usage documentation in code as well, not just
in the commit log.

Another limitation seems to be that machine needs
to be running. I think ideally you should not teach callers
about this limitation though.

> Signed-off-by: Wei Wang 
> Signed-off-by: Liang Li 
> CC: Michael S. Tsirkin 
> CC: Dr. David Alan Gilbert 
> CC: Juan Quintela 

I find it suspicious that neither unrealize nor reset
functions have been touched at all.
Are you sure you have thought through scenarious like
hot-unplug or disabling the device by guest?
  
> ---
>  balloon.c   |  49 +--
>  hw/virtio/virtio-balloon.c  | 183 
> +---
>  include/hw/virtio/virtio-balloon.h  |  15 +-
>  include/standard-headers/linux/virtio_balloon.h |   7 +
>  include/sysemu/balloon.h|  15 +-
>  5 files changed, 240 insertions(+), 29 deletions(-)
> 
> diff --git a/balloon.c b/balloon.c
> index d8dd6fe..b0b0749 100644
> --- a/balloon.c
> +++ b/balloon.c
> @@ -36,6 +36,9 @@
>  
>  static QEMUBalloonEvent *balloon_event_fn;
>  static QEMUBalloonStatus *balloon_stat_fn;
> +static QEMUBalloonFreePageSupport *balloon_free_page_support_fn;
> +static QEMUBalloonFreePageStart *balloon_free_page_start_fn;
> +static QEMUBalloonFreePageStop *balloon_free_page_stop_fn;
>  static void *balloon_opaque;
>  static bool balloon_inhibited;
>  
> @@ -64,19 +67,42 @@ static bool have_balloon(Error **errp)
>  return true;
>  }
>  
> -int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
> - QEMUBalloonStatus *stat_func, void *opaque)
> +bool balloon_free_page_support(void)
>  {
> -if (balloon_event_fn || balloon_stat_fn || balloon_opaque) {
> -/* We're already registered one balloon handler.  How many can
> - * a guest really have?
> - */
> -return -1;
> +return balloon_free_page_support_fn &&
> +   balloon_free_page_support_fn(balloon_opaque);
> +}
> +
> +void balloon_free_page_start(void)
> +{
> +balloon_free_page_start_fn(balloon_opaque);
> +}
> +
> +void balloon_free_page_stop(void)
> +{
> +balloon_free_page_stop_fn(balloon_opaque);
> +}
> +
> +void qemu_add_balloon_handler(QEMUBalloonEvent *event_fn,
> +  QEMUBalloonStatus *stat_fn,
> +  QEMUBalloonFreePageSupport 
> *free_page_support_fn,
> +  QEMUBalloonFreePageStart *free_page_start_fn,
> +  QEMUBalloonFreePageStop *free_page_stop_fn,
> +  void *opaque)
> +{
> +if (balloon_event_fn || balloon_stat_fn || balloon_free_page_support_fn 
> ||
> +balloon_free_page_start_fn || balloon_free_page_stop_fn ||
> +balloon_opaque) {
> +/* We already registered one balloon handler. */
> +return;
>  }
> -balloon_event_fn = event_func;
> -balloon_stat_fn = stat_func;
> +
> +balloon_event_fn = event_fn;
> +balloon_stat_fn = stat_fn;
> +balloon_free_page_support_fn = free_page_support_fn;
> +balloon_free_page_start_fn = free_page_start_fn;
> +balloon_free_page_stop_fn = free_page_stop_fn;
>  balloon_opaque = opaque;
> -return 0;
>  }
>  
>  void qemu_remove_balloon_handler(void *opaque)
> @@ -86,6 +112,9 @@ void qemu_remove_balloon_handler(void *opaque)
>  }
>  balloon_event_fn = NULL;
>  balloon_stat_fn = NULL;
> +balloon_free_page_support_fn = NULL;
> +balloon_free_page_start_fn = NULL;
> +balloon_free_page_stop_fn = NULL;
>  balloon_opaque = NULL;
>  }
>  
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 4822449..48ed2ec 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -31,6 +31,7 @@
>  
>  #include "hw/virtio/virtio-bus.h"
>  #include "hw/virtio/virtio-access.h"
> +#include "migration/misc.h"
>  
>  #define BALLOON_PAGE_SIZE  (1 << VIRTIO_BALLOON_PFN_SHIFT)
>  
> @@ -308,6 +309,111 @@ out:
>  }
>  }
>  
> +static void *virtio_balloon_poll_free_page_hints(void *opaque)
> +{
> +VirtQueueElement *elem;
> +VirtIOBalloon *dev = opaque;
> +VirtQueue *vq = dev->free_page_vq;
> +uint32_t id;
> +size_t size;

What makes it safe to poke at