On 09.09.25 17:56, Peter Xu wrote:
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.
Sounds good, will try.
--
Best regards,
Vladimir
