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