Re: [RFC v2 13/14] virtio-net: detect pending Tx work for VQs mid-migration changes

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

On Tue, Mar 24, 2026 at 5:48 PM Jonah Palmer <jonah.palmer@oracle.com> wrote:
>
>
>
> On 3/24/26 7:35 AM, Eugenio Perez Martin wrote:
> > On Fri, Mar 20, 2026 at 3:21 PM Jonah Palmer <jonah.palmer@oracle.com> wrote:
> >>
> >> Track per-queue pending Tx work state during early migration time.
> >>
> >> This includes a snapshot of pending Tx state for active queue pairs,
> >> checking for deltas at the end of migration, and freeing the snapshot
> >> buffer during device unrealize.
> >>
> >> With this final delta signal in place, drop the temporary always-true
> >> fallback in virtio_net_has_delta and return false when no deltas are
> >> found.
> >>
> >> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
> >> ---
> >>   hw/net/virtio-net.c            | 26 +++++++++++++++++++++-----
> >>   include/hw/virtio/virtio-net.h |  2 ++
> >>   2 files changed, 23 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >> index 3ee49a043a..483a43be4f 100644
> >> --- a/hw/net/virtio-net.c
> >> +++ b/hw/net/virtio-net.c
> >> @@ -3943,6 +3943,14 @@ static int virtio_net_early_pre_save(void *opaque)
> >>       vnet_mig->mq_early = n->multiqueue;
> >>       vnet_mig->queue_pairs_early = n->curr_queue_pairs;
> >>
> >> +    /* Tx waiting snapshot for active queue pairs */
> >> +    if (!vnet_mig->tx_waiting_early) {
> >> +        vnet_mig->tx_waiting_early = g_new0(uint32_t, n->max_queue_pairs);
> >> +    }
> >> +    for (int i = 0; i < n->curr_queue_pairs; i++) {
> >> +        vnet_mig->tx_waiting_early[i] = n->vqs[i].tx_waiting;
> >> +    }
> >> +
> >>       /* RSS state snapshot */
> >>       vnet_mig->rss_enabled_early = n->rss_data.enabled;
> >>       vnet_mig->rss_redirect_early = n->rss_data.redirect;
> >> @@ -4254,6 +4262,8 @@ static void virtio_net_device_unrealize(DeviceState *dev)
> >>           n->migration->mtable_macs_early = NULL;
> >>           g_free(n->migration->vlans_early);
> >>           n->migration->vlans_early = NULL;
> >> +        g_free(n->migration->tx_waiting_early);
> >> +        n->migration->tx_waiting_early = NULL;
> >>           g_free(n->migration->rss_indirections_table_early);
> >>           n->migration->rss_indirections_table_early = NULL;
> >>           g_free(n->migration);
> >> @@ -4412,6 +4422,16 @@ static bool virtio_net_has_delta(VirtIONet *n, VirtIODevice *vdev)
> >>           return true;
> >>       }
> >>
> >> +    /* Has any active queue's tx_waiting changed? */
> >> +    if (!vnet_mig->tx_waiting_early) {
> >> +        return true;
> >> +    }
> >> +    for (int i = 0; i < n->curr_queue_pairs; i++) {
> >> +        if (n->vqs[i].tx_waiting != vnet_mig->tx_waiting_early[i]) {
> >> +            return true;
> >> +        }
> >> +    }
> >> +
> >>       /* Has the VirtIONet's RSS state changed? */
> >>       if (n->rss_data.enabled != vnet_mig->rss_enabled_early ||
> >>           n->rss_data.redirect != vnet_mig->rss_redirect_early ||
> >> @@ -4439,11 +4459,7 @@ static bool virtio_net_has_delta(VirtIONet *n, VirtIODevice *vdev)
> >>           }
> >>       }
> >>
> >> -    /*
> >> -     * Always return true for now until we're able to detect all possible
> >> -     * changes to a VirtIONet device.
> >> -     */
> >> -    return true;
> >> +    return false;
> >
> > I'm failing to see one thing: If we add a new feature that the guest
> > can change during migration and it's only added in the
> > VMStateDescription vmstate_virtio_net_device, will this "return false"
> > prevent the state from being resent in the stop-and-copy phase?
> >
> > Mandating its addition here is ok somehow, but I'm not sure if I'm
> > missing something.
> >
>
> Correct. But this is also the same case today. That is, any new
> migratable device state would still need to be added to the appropriate
> VMSD/subsection. For example, if today we added a new member to
> VirtIONet that also should be migrated, it would also need to be added
> to vmstate_virtio_net_device (or the appropriate subsection).
>
> So it's not a new *kind* of maintenance burden, but it is an additional
> place that'd need to be updated.
>

Right, we're on the same page.

Half-baked idea: Is it possible to update it only in
vmstate_virtio_net_device? I'm thinking in a method to store only the
relevant fields by iterating vmstate_virtio_net_device, instead of
manually coding each if (n->field != vnet_mig->field) return true.

> >>   }
> >>
> >>   static bool virtio_net_needed(void *opaque)
> >> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> >> index 88074a0976..dbbacc83bb 100644
> >> --- a/include/hw/virtio/virtio-net.h
> >> +++ b/include/hw/virtio/virtio-net.h
> >> @@ -180,6 +180,7 @@ typedef struct VirtIONetQueue {
> >>    * @guest_offloads_early: Guest offloads snapshot.
> >>    * @mq_early: Multiqueue state snapshot.
> >>    * @queue_pairs_early: Queue pairs snapshot.
> >> + * @tx_waiting_early: Per-queue pending-Tx snapshot.
> >>    * @rss_enabled_early: RSS enabled flag.
> >>    * @rss_redirect_early: RSS redirect flag.
> >>    * @rss_populate_hash_early: RSS populate hash flag.
> >> @@ -201,6 +202,7 @@ typedef struct VirtIONetMigration {
> >>       uint64_t guest_offloads_early;
> >>       int mq_early;
> >>       uint16_t queue_pairs_early;
> >> +    uint32_t *tx_waiting_early;
> >>       bool rss_enabled_early;
> >>       bool rss_redirect_early;
> >>       bool rss_populate_hash_early;
> >> --
> >> 2.51.0
> >>
> >
>