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?
>
> 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())?
--
Peter Xu
