Re: [RFC v2 02/14] virtio,virtio-net: add initial early VMSD for setup-phase migration

[Eugenio Perez Martin <eperezma@redhat.com>]

On Tue, Mar 24, 2026 at 3:29 PM Jonah Palmer <jonah.palmer@oracle.com> wrote:
>
>
>
> On 3/24/26 5:27 AM, Eugenio Perez Martin wrote:
> > On Fri, Mar 20, 2026 at 3:25 PM Jonah Palmer <jonah.palmer@oracle.com> wrote:
> >>
> >> Adds a separate VMStateDescription for virtio-net that uses the
> >> .early_setup feature. With this feature, we can migrate a virtio-net
> >> device's state earlier, before the stop-and-copy phase.
> >>
> >> Future patches will utilize this to move control plane operations out of
> >> the stop-and-copy phase to reduce the downtime latency caused by
> >> migrating a virtio-net device.
> >>
> >> A VirtIODevMigration migration data structure is also introduced here
> >> for VirtIODevices to help track the current state of a migration.
> >>
> >> The early_load member is used to signal that a VirtIODevice is being
> >> loaded early and to not throw an error regarding vring indices.
> >> Inconsistent indices shouldn't be an issue for a device so long as the
> >> final indices are eventually loaded before the device starts.
> >>
> >> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
> >> ---
> >>   hw/net/virtio-net.c        | 53 ++++++++++++++++++++++++++++++++++++++
> >>   hw/virtio/virtio.c         | 14 +++++++++-
> >>   include/hw/virtio/virtio.h |  9 +++++++
> >>   3 files changed, 75 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >> index 12b3456ca2..ddd6ed6e62 100644
> >> --- a/hw/net/virtio-net.c
> >> +++ b/hw/net/virtio-net.c
> >> @@ -3864,6 +3864,37 @@ static bool failover_hide_primary_device(DeviceListener *listener,
> >>       return qatomic_read(&n->failover_primary_hidden);
> >>   }
> >>
> >> +static int virtio_net_early_pre_load(void *opaque)
> >> +{
> >> +    VirtIONet *n = opaque;
> >> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
> >> +
> >> +    vdev->migration->early_load = true;
> >> +    return 0;
> >> +}
> >> +
> >> +static int virtio_net_early_post_load(void *opaque, int version_id)
> >> +{
> >> +    VirtIONet *n = opaque;
> >> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
> >> +
> >> +    vdev->migration->early_load = false;
> >> +    return 0;
> >> +}
> >> +
> >> +static const VMStateDescription vmstate_virtio_net_early = {
> >> +    .name = "virtio-net-early",
> >> +    .minimum_version_id = VIRTIO_NET_VM_VERSION,
> >> +    .version_id = VIRTIO_NET_VM_VERSION,
> >> +    .early_setup = true,
> >> +    .pre_load = virtio_net_early_pre_load,
> >> +    .post_load = virtio_net_early_post_load,
> >> +    .fields = (const VMStateField[]) {
> >> +        VMSTATE_VIRTIO_DEVICE,
> >> +        VMSTATE_END_OF_LIST()
> >> +    },
> >> +};
> >> +
> >>   static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> >>   {
> >>       VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> >> @@ -4046,6 +4077,21 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> >>               n->rss_data.specified_hash_types.on_bits |
> >>               n->rss_data.specified_hash_types.auto_bits;
> >>       }
> >> +
> >> +    if (n->early_mig) {
> >> +        if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> >> +            /*
> >> +             * vhost-user backend is not currently supported for the early
> >> +             * migration path.
> >> +             */
> >> +            n->early_mig = false;
> >> +        } else {
> >> +            vdev->migration = g_new0(VirtIODevMigration, 1);
> >> +            vdev->migration->early_load = false;
> >> +
> >> +            vmstate_register_any(VMSTATE_IF(n), &vmstate_virtio_net_early, n);
> >> +        }
> >> +    }
> >>   }
> >>
> >>   static void virtio_net_device_unrealize(DeviceState *dev)
> >> @@ -4090,6 +4136,13 @@ static void virtio_net_device_unrealize(DeviceState *dev)
> >>       g_free(n->rss_data.indirections_table);
> >>       net_rx_pkt_uninit(n->rx_pkt);
> >>       virtio_cleanup(vdev);
> >> +
> >> +    if (n->early_mig) {
> >> +        g_free(vdev->migration);
> >> +        vdev->migration = NULL;
> >
> > Nit: You can use g_clear_pointer(vdev->migration, g_free) here.
> >
>
> Ack. Will do!
>
> >> +
> >> +        vmstate_unregister(VMSTATE_IF(n), &vmstate_virtio_net_early, n);
> >> +    }
> >>   }
> >>
> >>   static void virtio_net_reset(VirtIODevice *vdev)
> >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >> index 8fcf6cfd0b..48de4a430b 100644
> >> --- a/hw/virtio/virtio.c
> >> +++ b/hw/virtio/virtio.c
> >> @@ -3323,6 +3323,7 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
> >>       int32_t config_len;
> >>       uint32_t num;
> >>       uint32_t features;
> >> +    bool inconsistent_indices;
> >>       BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> >>       VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> >>       VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
> >> @@ -3460,6 +3461,14 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
> >>           if (vdev->vq[i].vring.desc) {
> >>               uint16_t nheads;
> >>
> >> +            /*
> >> +             * Ring indices will be inconsistent for a VMStateDescription
> >> +             * performing an early load. This shouldn't be an issue as the
> >> +             * final indices will get sent later once the source has been
> >> +             * stopped.
> >> +             */
> >> +            inconsistent_indices = vdev->migration && vdev->migration->early_load;
> >> +
> >>               /*
> >>                * VIRTIO-1 devices migrate desc, used, and avail ring addresses so
> >>                * only the region cache needs to be set up.  Legacy devices need
> >> @@ -3481,12 +3490,15 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
> >>
> >>               nheads = vring_avail_idx(&vdev->vq[i]) - vdev->vq[i].last_avail_idx;
> >>               /* Check it isn't doing strange things with descriptor numbers. */
> >> -            if (nheads > vdev->vq[i].vring.num) {
> >> +            if (!inconsistent_indices && nheads > vdev->vq[i].vring.num) {
> >>                   virtio_error(vdev, "VQ %d size 0x%x Guest index 0x%x "
> >>                                "inconsistent with Host index 0x%x: delta 0x%x",
> >>                                i, vdev->vq[i].vring.num,
> >>                                vring_avail_idx(&vdev->vq[i]),
> >>                                vdev->vq[i].last_avail_idx, nheads);
> >> +                inconsistent_indices = true;
> >> +            }
> >> +            if (inconsistent_indices) {
> >>                   vdev->vq[i].used_idx = 0;
> >>                   vdev->vq[i].shadow_avail_idx = 0;
> >>                   vdev->vq[i].inuse = 0;
> >> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> >> index 6344bd7b68..4c886eb48b 100644
> >> --- a/include/hw/virtio/virtio.h
> >> +++ b/include/hw/virtio/virtio.h
> >> @@ -99,6 +99,14 @@ enum virtio_device_endian {
> >>       VIRTIO_DEVICE_ENDIAN_BIG,
> >>   };
> >>
> >> +/**
> >> + * struct VirtIODevMigration - Common VirtIODevice migration structure
> >> + * @early_load: Flag to indicate an early virtio_load for the device.
> >> + */
> >> +typedef struct VirtIODevMigration {
> >> +    bool early_load;
> >> +} VirtIODevMigration;
> >> +
> >>   /**
> >>    * struct VirtIODevice - common VirtIO structure
> >>    * @name: name of the device
> >> @@ -168,6 +176,7 @@ struct VirtIODevice
> >>        */
> >>       EventNotifier config_notifier;
> >>       bool device_iotlb_enabled;
> >> +    VirtIODevMigration *migration;
> >
> > Can we use something like net "struct VirtIONetMigTmp" for this so
> > VirtIODevice does not need to be expanded?
> >
>
> I wanted to keep it under VirtIODevice because it's meant to hold
> migration scratch state that's common for all VirtIODevices, not just
> virtio-net. For example, it could be reusable by other virtio devices in
> the future if they were to implement their own early-migration path.
>
> If we made this for virtio-net only, we'd need to modify generic virtio
> code (e.g. an extra callback) just for retrieving the state. I thought
> that this would be the cleaner way to go about handling common
> VirtIODevice state.
>

I wasn't clear enough. My goal was to move it to a struct allocated
only at migration time, not through all the live of the device.

Moving the allocation and freeing of the struct to
virtio_net_early_pre_load / virtio_net_early_post_load is a first
step. Removing the pointer altogether, the same way VirtIONet doesn't
need a pointer to hold VirtIONetMigTmp, would be even better. But I
see how keeping it live during all the migration may be problematic to
achieve it.