On Tue, Jul 22, 2025 at 2:41 PM Jonah Palmer <jonah.pal...@oracle.com> wrote:
>
> Iterative live migration for virtio-net sends an initial
> VMStateDescription while the source is still active. Because data
> continues to flow for virtio-net, the guest's avail index continues to
> increment after last_avail_idx had already been sent. This causes the
> destination to often see something like this from virtio_error():
>
> VQ 0 size 0x100 Guest index 0x0 inconsistent with Host index 0xc: delta 0xfff4
>
> This patch suppresses this consistency check if we're loading the
> initial VMStateDescriptions via iterative migration and unsuppresses
> it for the stop-and-copy phase when the final VMStateDescriptions
> (carrying the correct indices) are loaded.
>
> A temporary VirtIODevMigration migration data structure is introduced here to
> represent the iterative migration process for a VirtIODevice. For now it
> just holds a flag to indicate whether or not the initial
> VMStateDescription was sent during the iterative live migration process.
>
> Signed-off-by: Jonah Palmer <jonah.pal...@oracle.com>
> ---
> hw/net/virtio-net.c | 13 +++++++++++++
> hw/virtio/virtio.c | 32 ++++++++++++++++++++++++--------
> include/hw/virtio/virtio.h | 6 ++++++
> 3 files changed, 43 insertions(+), 8 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 86a6fe5b91..b7ac5e8278 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -3843,12 +3843,19 @@ static void virtio_net_save_cleanup(void *opaque)
>
> static int virtio_net_load_setup(QEMUFile *f, void *opaque, Error **errp)
> {
> + VirtIONet *n = opaque;
> + VirtIODevice *vdev = VIRTIO_DEVICE(n);
> + vdev->migration = g_new0(VirtIODevMigration, 1);
> + vdev->migration->iterative_vmstate_loaded = false;
> +
> return 0;
> }
>
> static int virtio_net_load_state(QEMUFile *f, void *opaque, int version_id)
> {
> VirtIONet *n = opaque;
> + VirtIODevice *vdev = VIRTIO_DEVICE(n);
> + VirtIODevMigration *mig = vdev->migration;
> uint64_t flag;
>
> flag = qemu_get_be64(f);
> @@ -3861,6 +3868,7 @@ static int virtio_net_load_state(QEMUFile *f, void
> *opaque, int version_id)
> case VNET_MIG_F_INIT_STATE:
> {
> vmstate_load_state(f, &vmstate_virtio_net, n,
> VIRTIO_NET_VM_VERSION);
> + mig->iterative_vmstate_loaded = true;
This code will need to change if we send the status iteratively more
than once. For example, if the guest changes the mac address, the
number of vqs, etc.
In my opinion, we should set a flag named "in_iterative_migration" (or
equivalent) in virtio_net_load_setup and clear it in
virtio_net_load_cleanup. That's enough to tell in virtio_load if we
should perform actions like checking for inconsistent indices.
> break;
> }
> default:
> @@ -3875,6 +3883,11 @@ static int virtio_net_load_state(QEMUFile *f, void
> *opaque, int version_id)
>
> static int virtio_net_load_cleanup(void *opaque)
> {
> + VirtIONet *n = opaque;
> + VirtIODevice *vdev = VIRTIO_DEVICE(n);
> + g_free(vdev->migration);
> + vdev->migration = NULL;
> +
> return 0;
> }
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 5534251e01..68957ee7d1 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3222,6 +3222,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);
> @@ -3365,6 +3366,16 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int
> version_id)
> if (vdev->vq[i].vring.desc) {
> uint16_t nheads;
>
> + /*
> + * Ring indices will be inconsistent during iterative migration.
> The actual
> + * indices will be sent later during the stop-and-copy phase.
> + */
> + if (vdev->migration) {
> + inconsistent_indices =
> !vdev->migration->iterative_vmstate_loaded;
> + } else {
> + inconsistent_indices = false;
> + }
Nit, "inconsistent_indices = vdev->migration &&
!vdev->migration->iterative_vmstate_loaded" ? I'm happy with the
current "if else" too, but I think the one line is clearer. Your call
:).
> +
> /*
> * VIRTIO-1 devices migrate desc, used, and avail ring addresses
> so
> * only the region cache needs to be set up. Legacy devices need
> @@ -3384,14 +3395,19 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int
> version_id)
> continue;
> }
>
> - 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) {
> - 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);
> + if (!inconsistent_indices) {
> + 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) {
> + 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 214d4a77e9..06b6e6ba65 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -98,6 +98,11 @@ enum virtio_device_endian {
> VIRTIO_DEVICE_ENDIAN_BIG,
> };
>
> +/* VirtIODevice iterative live migration data structure */
> +typedef struct VirtIODevMigration {
> + bool iterative_vmstate_loaded;
> +} VirtIODevMigration;
> +
> /**
> * struct VirtIODevice - common VirtIO structure
> * @name: name of the device
> @@ -151,6 +156,7 @@ struct VirtIODevice
> bool disable_legacy_check;
> bool vhost_started;
> VMChangeStateEntry *vmstate;
> + VirtIODevMigration *migration;
> char *bus_name;
> uint8_t device_endian;
> /**
> --
> 2.47.1
>
