Re: [Qemu-devel] [PATCH v11 7/7] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT

[Wei Wang <wei.w.wang@intel.com>]

On 03/05/2019 10:50 PM, Dr. David Alan Gilbert wrote:
> * Wei Wang (wei.w.wang@intel.com) wrote:
> > The new feature enables the virtio-balloon device to receive hints of
> > guest free pages from the free page vq.
> >
> > A notifier is registered to the migration precopy notifier chain. The
> > notifier calls free_page_start after the migration thread syncs the dirty
> > bitmap, so that the free page optimization starts to clear bits of free
> > pages from the bitmap. It calls the free_page_stop before the migration
> > thread syncs the bitmap, which is the end of the current round of ram
> > save. The free_page_stop is also called to stop the optimization in the
> > case when there is an error occurred in the process of ram saving.
> >
> > 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 free_page_start call is made. Guest reporting must be
> > disabled before the migration dirty bitmap is synchronized.
> >
> > Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> > CC: Michael S. Tsirkin <mst@redhat.com>
> > CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > CC: Juan Quintela <quintela@redhat.com>
> > CC: Peter Xu <peterx@redhat.com>
> > ---
> >   hw/virtio/virtio-balloon.c                      | 263 ++++++++++++++++++++++++
> >   include/hw/virtio/virtio-balloon.h              |  28 ++-
> >   include/standard-headers/linux/virtio_balloon.h |   5 +
> >   3 files changed, 295 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> > +    if (dev->free_page_report_status == FREE_PAGE_REPORT_S_REQUESTED) {
> > +        config.free_page_report_cmd_id =
> > +                       cpu_to_le32(dev->free_page_report_cmd_id);
> > +    } else if (dev->free_page_report_status == FREE_PAGE_REPORT_S_STOP) {
> > +        config.free_page_report_cmd_id =
> > +                       cpu_to_le32(VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID);
> > +    } else if (dev->free_page_report_status == FREE_PAGE_REPORT_S_DONE) {
> > +        config.free_page_report_cmd_id =
> > +                       cpu_to_le32(VIRTIO_BALLOON_FREE_PAGE_REPORT_DONE_ID);
> > +    }
> It looks like somewhere in the last 3 months the name in the kernel
> changed; so I think I've fixed this correctly but please shout if it's
> wrong:
>
>      if (dev->free_page_report_status == FREE_PAGE_REPORT_S_REQUESTED) {
>          config.free_page_report_cmd_id =
>                         cpu_to_le32(dev->free_page_report_cmd_id);
>      } else if (dev->free_page_report_status == FREE_PAGE_REPORT_S_STOP) {
>          config.free_page_report_cmd_id =
>                         cpu_to_le32(VIRTIO_BALLOON_CMD_ID_STOP);
>      } else if (dev->free_page_report_status == FREE_PAGE_REPORT_S_DONE) {
>          config.free_page_report_cmd_id =
>                         cpu_to_le32(VIRTIO_BALLOON_CMD_ID_DONE);
>      }
>
> and I've dropped the kernel header update since it's already there.

Looks good. Thanks for the update.

Best,
Wei