Re: [PATCH v3 7/8] virtio-mem: Migrate immutable properties early

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* David Hildenbrand (david@redhat.com) wrote:
> The bitmap and the size are immutable while migration is active: see
> virtio_mem_is_busy(). We can migrate this information early, before
> migrating any actual RAM content. Further, all information we need for
> sanity checks is immutable as well.
> 
> Having this information in place early will, for example, allow for
> properly preallocating memory before touching these memory locations
> during RAM migration: this way, we can make sure that all memory was
> actually preallocated and that any user errors (e.g., insufficient
> hugetlb pages) can be handled gracefully.
> 
> In contrast, usable_region_size and requested_size can theoretically
> still be modified on the source while the VM is running. Keep migrating
> these properties the usual, late, way.
> 
> Use a new device property to keep behavior of compat machines
> unmodified.

Can you get me a migration file from this? I want to try and understand
what happens when you have the vmstate_register together with the ->vmsd -
I'm not quite sure what ends up in the output.  Preferably for a VM with
two virtio-mem's.

Dave


> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  hw/core/machine.c              |  4 ++-
>  hw/virtio/virtio-mem.c         | 51 ++++++++++++++++++++++++++++++++--
>  include/hw/virtio/virtio-mem.h |  8 ++++++
>  3 files changed, 60 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 616f3a207c..29b57f6448 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -41,7 +41,9 @@
>  #include "hw/virtio/virtio-pci.h"
>  #include "qom/object_interfaces.h"
>  
> -GlobalProperty hw_compat_7_2[] = {};
> +GlobalProperty hw_compat_7_2[] = {
> +    { "virtio-mem", "x-early-migration", "false" },
> +};
>  const size_t hw_compat_7_2_len = G_N_ELEMENTS(hw_compat_7_2);
>  
>  GlobalProperty hw_compat_7_1[] = {
> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
> index 02f7b5469a..51666baa01 100644
> --- a/hw/virtio/virtio-mem.c
> +++ b/hw/virtio/virtio-mem.c
> @@ -31,6 +31,8 @@
>  #include CONFIG_DEVICES
>  #include "trace.h"
>  
> +static const VMStateDescription vmstate_virtio_mem_device_early;
> +
>  /*
>   * We only had legacy x86 guests that did not support
>   * VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE. Other targets don't have legacy guests.
> @@ -878,6 +880,10 @@ static void virtio_mem_device_realize(DeviceState *dev, Error **errp)
>  
>      host_memory_backend_set_mapped(vmem->memdev, true);
>      vmstate_register_ram(&vmem->memdev->mr, DEVICE(vmem));
> +    if (vmem->early_migration) {
> +        vmstate_register(VMSTATE_IF(vmem), VMSTATE_INSTANCE_ID_ANY,
> +                         &vmstate_virtio_mem_device_early, vmem);
> +    }
>      qemu_register_reset(virtio_mem_system_reset, vmem);
>  
>      /*
> @@ -899,6 +905,10 @@ static void virtio_mem_device_unrealize(DeviceState *dev)
>       */
>      memory_region_set_ram_discard_manager(&vmem->memdev->mr, NULL);
>      qemu_unregister_reset(virtio_mem_system_reset, vmem);
> +    if (vmem->early_migration) {
> +        vmstate_unregister(VMSTATE_IF(vmem), &vmstate_virtio_mem_device_early,
> +                           vmem);
> +    }
>      vmstate_unregister_ram(&vmem->memdev->mr, DEVICE(vmem));
>      host_memory_backend_set_mapped(vmem->memdev, false);
>      virtio_del_queue(vdev, 0);
> @@ -1015,18 +1025,53 @@ static const VMStateDescription vmstate_virtio_mem_sanity_checks = {
>      },
>  };
>  
> +static bool virtio_mem_vmstate_field_exists(void *opaque, int version_id)
> +{
> +    const VirtIOMEM *vmem = VIRTIO_MEM(opaque);
> +
> +    /* With early migration, these fields were already migrated. */
> +    return !vmem->early_migration;
> +}
> +
>  static const VMStateDescription vmstate_virtio_mem_device = {
>      .name = "virtio-mem-device",
>      .minimum_version_id = 1,
>      .version_id = 1,
>      .priority = MIG_PRI_VIRTIO_MEM,
>      .post_load = virtio_mem_post_load,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_WITH_TMP_TEST(VirtIOMEM, virtio_mem_vmstate_field_exists,
> +                              VirtIOMEMMigSanityChecks,
> +                              vmstate_virtio_mem_sanity_checks),
> +        VMSTATE_UINT64(usable_region_size, VirtIOMEM),
> +        VMSTATE_UINT64_TEST(size, VirtIOMEM, virtio_mem_vmstate_field_exists),
> +        VMSTATE_UINT64(requested_size, VirtIOMEM),
> +        VMSTATE_BITMAP_TEST(bitmap, VirtIOMEM, virtio_mem_vmstate_field_exists,
> +                            0, bitmap_size),
> +        VMSTATE_END_OF_LIST()
> +    },
> +};
> +
> +/*
> + * Transfer properties that are immutable while migration is active early,
> + * such that we have have this information around before migrating any RAM
> + * content.
> + *
> + * Note that virtio_mem_is_busy() makes sure these properties can no longer
> + * change on the migration source until migration completed.
> + *
> + * With QEMU compat machines, we transmit these properties later, via
> + * vmstate_virtio_mem_device instead -- see virtio_mem_vmstate_field_exists().
> + */
> +static const VMStateDescription vmstate_virtio_mem_device_early = {
> +    .name = "virtio-mem-device-early",
> +    .minimum_version_id = 1,
> +    .version_id = 1,
> +    .immutable = 1,
>      .fields = (VMStateField[]) {
>          VMSTATE_WITH_TMP(VirtIOMEM, VirtIOMEMMigSanityChecks,
>                           vmstate_virtio_mem_sanity_checks),
> -        VMSTATE_UINT64(usable_region_size, VirtIOMEM),
>          VMSTATE_UINT64(size, VirtIOMEM),
> -        VMSTATE_UINT64(requested_size, VirtIOMEM),
>          VMSTATE_BITMAP(bitmap, VirtIOMEM, 0, bitmap_size),
>          VMSTATE_END_OF_LIST()
>      },
> @@ -1211,6 +1256,8 @@ static Property virtio_mem_properties[] = {
>      DEFINE_PROP_ON_OFF_AUTO(VIRTIO_MEM_UNPLUGGED_INACCESSIBLE_PROP, VirtIOMEM,
>                              unplugged_inaccessible, ON_OFF_AUTO_AUTO),
>  #endif
> +    DEFINE_PROP_BOOL(VIRTIO_MEM_EARLY_MIGRATION_PROP, VirtIOMEM,
> +                     early_migration, true),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/include/hw/virtio/virtio-mem.h b/include/hw/virtio/virtio-mem.h
> index 7745cfc1a3..f15e561785 100644
> --- a/include/hw/virtio/virtio-mem.h
> +++ b/include/hw/virtio/virtio-mem.h
> @@ -31,6 +31,7 @@ OBJECT_DECLARE_TYPE(VirtIOMEM, VirtIOMEMClass,
>  #define VIRTIO_MEM_BLOCK_SIZE_PROP "block-size"
>  #define VIRTIO_MEM_ADDR_PROP "memaddr"
>  #define VIRTIO_MEM_UNPLUGGED_INACCESSIBLE_PROP "unplugged-inaccessible"
> +#define VIRTIO_MEM_EARLY_MIGRATION_PROP "x-early-migration"
>  #define VIRTIO_MEM_PREALLOC_PROP "prealloc"
>  
>  struct VirtIOMEM {
> @@ -74,6 +75,13 @@ struct VirtIOMEM {
>      /* whether to prealloc memory when plugging new blocks */
>      bool prealloc;
>  
> +    /*
> +     * Whether we migrate properties that are immutable while migration is
> +     * active early, before state of other devices and especially, before
> +     * migrating any RAM content.
> +     */
> +    bool early_migration;
> +
>      /* notifiers to notify when "size" changes */
>      NotifierList size_change_notifiers;
>  
> -- 
> 2.39.0
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK