On Tue, Sep 09, 2025 at 10:44:22AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 08.09.25 23:01, Peter Xu wrote:
> > On Mon, Sep 08, 2025 at 07:48:09PM +0300, Vladimir Sementsov-Ogievskiy
> > wrote:
> > > On 08.09.25 18:42, Peter Xu wrote:
> > > > On Fri, Sep 05, 2025 at 04:50:35PM +0300, Vladimir Sementsov-Ogievskiy
> > > > wrote:
> > > > > +static const VMStateDescription vmstate_tap = {
> > > > > + .name = "virtio-net-device",
> > > > > + .post_load = tap_post_load,
> > > > > + .fields = (const VMStateField[]) {
> > > > > + VMSTATE_FD(fd, TAPState),
> > > > > + VMSTATE_BOOL(using_vnet_hdr, TAPState),
> > > > > + VMSTATE_BOOL(has_ufo, TAPState),
> > > > > + VMSTATE_BOOL(has_uso, TAPState),
> > > > > + VMSTATE_BOOL(enabled, TAPState),
> > > > > + VMSTATE_UINT32(host_vnet_hdr_len, TAPState),
> > > > > + VMSTATE_END_OF_LIST()
> > > > > + }
> > > > > +};
> > > > > +
> > > > > +int tap_save(NetClientState *nc, QEMUFile *f)
> > > > > +{
> > > > > + TAPState *s = DO_UPCAST(TAPState, nc, nc);
> > > > > +
> > > > > + return vmstate_save_state(f, &vmstate_tap, s, 0);
> > > > > +}
> > > > > +
> > > > > +int tap_load(NetClientState *nc, QEMUFile *f)
> > > > > +{
> > > > > + TAPState *s = DO_UPCAST(TAPState, nc, nc);
> > > > > +
> > > > > + return vmstate_load_state(f, &vmstate_tap, s, 0);
> > > > > +}
> > > >
> > > > Instead of hard-coding vmstate_save_state() / vmstate_load_state(),
> > > > could
> > > > we make tap's VMSD to be a subsection of virtio-net's?
> > > >
> > > > Multifd already doesn't support qemufile, but only iochannels (which is
> > > > the
> > > > internal impl of qemufiles). We might at some point start to
> > > > concurrently
> > > > load devices with multifd, then anything with qemufile will be a no-go
> > > > and
> > > > need to be serialized as legacy code in the main channel, or rewritten.
> > > >
> > > > IMHO it'll be great if we can avoid adding new codes operating on
> > > > qemufiles, and also avoid adding any new custom VMSD fields'
> > > > put()/get() if
> > > > ever possible.
> > > >
> > >
> > > Subsections are loaded after fields.
> > >
> > > And virtio-net already has fields
> > >
> > > VMSTATE_WITH_TMP(VirtIONet, struct VirtIONetMigTmp,
> > > vmstate_virtio_net_has_vnet),
> > >
> > > and
> > >
> > > VMSTATE_WITH_TMP(VirtIONet, struct VirtIONetMigTmp,
> > > vmstate_virtio_net_has_ufo),
> >
> > Side note: I'm actually a bit confused on why it needs to use
> > VMSTATE_WITH_TMP(), or say, the get()/put() directly.
> >
> > Taking example of vmstate_virtio_net_has_ufo, afaiu, the only point here is
> > that virtio_net_ufo_post_load() would check peer UFO support. I wonder if
> > that should work too to check that in virtio_net_post_load_device(), and
> > fail there if anything is wrong.. then would has_ufo be able to be
> > migrated as a VMSTATE_U8 field?
[1]
> >
> > >
> > > Which do check on virtio-net level some parameters, which should come
> > > from local migration of TAP.
> > >
> > > That's why I made TAP a field, and put it before these two ones. This way
> > > these two checks works.
> > >
> > >
> > > Still, from your comment I understand that hard-coding save/load is worse
> > > problem. So I can just
> > > skip checking in vmstate_virtio_net_has_vnet / vmstate_virtio_net_has_ufo
> > > with enabled "local-tap"
> > > (or "fd-passing") capability (or better migration parameter). This way
> > > TAP may be a subsection.
> >
> > That'll be nice, thanks. Or would a VMSTATE_STRUCT() for TAP to work (so
> > that it can also be put before the two _TMPs, but avoid raw get()/put())?
> >
>
> I considered using VMSTATE_STRUCT, but it would mean, I should access
> TAPState directly from virtio-net
> code. That's not possible now, as TAPState is a static type in tap.c, and
> think it's better to keep
> it static. So, in this case, subsection should be better, if I understand
> correctly.
Ah OK. Then I wonder if this is a good chance too to move the peer feature
checks above [1] directly over to virtio-net's post_load as a pre-requisite
change, which should also be after the subsections. Then the hope is it'll
also help removing some _TMP users.
--
Peter Xu
