Re: [Qemu-devel] [PATCH v1 1/4] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ

[Juan Quintela <quintela@redhat.com>]

Wei Wang <wei.w.wang@intel.com> wrote:
> The new feature enables the virtio-balloon device to receive the hint of
> guest free pages from the free page vq, and clears the corresponding bits
> of the free page from the dirty bitmap, so that those free pages are not
> transferred by the migration thread.
>
> Without this feature, to local live migrate an 8G idle guest takes
> ~2286 ms. With this featrue, it takes ~260 ms, which redues the
> migration time to ~11%.
>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> Signed-off-by: Liang Li <liang.z.li@intel.com>
> CC: Michael S. Tsirkin <mst@redhat.com>

I hate to answer twice,but ...


> +static bool virtio_balloon_free_page_support(void *opaque)
> +{
> +    VirtIOBalloon *s = opaque;
> +
> +    if (!balloon_free_page_supported(s)) {
> +        return false;
> +    }
> +
> +    return true;

return balloon_free_page_supported(s); ???


> @@ -312,6 +399,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
>  
>      config.num_pages = cpu_to_le32(dev->num_pages);
>      config.actual = cpu_to_le32(dev->actual);
> +    config.free_page_report_cmd_id = cpu_to_le32(dev->free_page_report_cmd_id);
>  
>      trace_virtio_balloon_get_config(config.num_pages, config.actual);
>      memcpy(config_data, &config, sizeof(struct virtio_balloon_config));
> @@ -418,6 +506,7 @@ static const VMStateDescription vmstate_virtio_balloon_device = {
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32(num_pages, VirtIOBalloon),
>          VMSTATE_UINT32(actual, VirtIOBalloon),
> +        VMSTATE_UINT32(free_page_report_cmd_id, VirtIOBalloon),
>          VMSTATE_END_OF_LIST()

No new version, no subsection, and we add a new field?
I think that is wrong.  We need to send that only for old versions.
Look at how was handled for ....

[PATCH v2] hpet: recover timer offset correctly

v2 discussion explains why we want to handle that way for different
machine types.

v3 does it correctly.


And I think that with this I have reviewed all the migration/vmstate
bits, no?

If something is missing, please ask.

Later, Juan.