Handle local-incoming option:
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
include/net/tap.h | 4 ++
net/tap.c | 139 +++++++++++++++++++++++++++++++++++++++-------
2 files changed, 122 insertions(+), 21 deletions(-)
diff --git a/include/net/tap.h b/include/net/tap.h
index 6f34f13eae..3ef2e2dbae 100644
--- a/include/net/tap.h
+++ b/include/net/tap.h
@@ -30,7 +30,11 @@
int tap_enable(NetClientState *nc);
int tap_disable(NetClientState *nc);
+bool tap_local_incoming(NetClientState *nc);
int tap_get_fd(NetClientState *nc);
+int tap_load(NetClientState *nc, QEMUFile *f);
+int tap_save(NetClientState *nc, QEMUFile *f);
+
#endif /* QEMU_NET_TAP_H */
diff --git a/net/tap.c b/net/tap.c
index cf7f704a92..0fd7a7671c 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -35,6 +35,7 @@
#include "net/eth.h"
#include "net/net.h"
#include "clients.h"
+#include "migration/vmstate.h"
#include "monitor/monitor.h"
#include "system/system.h"
#include "qapi/error.h"
@@ -44,6 +45,7 @@
#include "qemu/sockets.h"
#include "hw/virtio/vhost.h"
#include "trace.h"
+#include "system/runstate.h"
#include "net/tap.h"
@@ -82,6 +84,7 @@ typedef struct TAPState {
VHostNetState *vhost_net;
unsigned host_vnet_hdr_len;
Notifier exit;
+ bool local_incoming;
} TAPState;
static void launch_script(const char *setup_script, const char *ifname,
@@ -756,6 +759,43 @@ static int net_tap_init_vhost(TAPState *s, Error **errp)
return 0;
}
+static int tap_post_load(void *opaque, int version_id)
+{
+ TAPState *s = opaque;
+
+ tap_read_poll(s, true);
+
+ return net_tap_init_vhost(s, NULL);
+}
+
+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);
+}
+
static int net_tap_fd_init_common(const Netdev *netdev, NetClientState *peer,
const char *model, const char *name,
const char *ifname, const char *script,
@@ -763,30 +803,40 @@ static int net_tap_fd_init_common(const Netdev *netdev, NetClientState *peer,
const char *vhostfdname,
int vnet_hdr, int fd, Error **errp)
{
- const NetdevTapOptions *tap;
+ const NetdevTapOptions *tap = NULL;
int ret;
NetClientState *nc;
TAPState *s;
+ bool local_incoming = false;
+
+ if (netdev->type == NET_CLIENT_DRIVER_TAP) {
+ tap = &netdev->u.tap;
+ local_incoming = tap->local_incoming;
+ }
nc = qemu_new_net_client(&net_tap_info, peer, model, name);
s = DO_UPCAST(TAPState, nc, nc);
-
- s->fd = fd;
- s->host_vnet_hdr_len = vnet_hdr ? sizeof(struct virtio_net_hdr) : 0;
- s->using_vnet_hdr = false;
- s->has_ufo = tap_probe_has_ufo(s->fd);
- s->has_uso = tap_probe_has_uso(s->fd);
- s->enabled = true;
- tap_set_offload(&s->nc, 0, 0, 0, 0, 0, 0, 0);
- /*
- * Make sure host header length is set correctly in tap:
- * it might have been modified by another instance of qemu.
- */
- if (vnet_hdr) {
- tap_fd_set_vnet_hdr_len(s->fd, s->host_vnet_hdr_len);
+ s->local_incoming = local_incoming;
+
+ if (!local_incoming) {
+ s->fd = fd;
+ s->host_vnet_hdr_len = vnet_hdr ? sizeof(struct virtio_net_hdr) : 0;
+ s->using_vnet_hdr = false;
+ s->has_ufo = tap_probe_has_ufo(s->fd);
+ s->has_uso = tap_probe_has_uso(s->fd);
+ s->enabled = true;
+ tap_set_offload(&s->nc, 0, 0, 0, 0, 0, 0, 0);
+ /*
+ * Make sure host header length is set correctly in tap:
+ * it might have been modified by another instance of qemu.
+ */
+ if (vnet_hdr) {
+ tap_fd_set_vnet_hdr_len(s->fd, s->host_vnet_hdr_len);
+ }
+ tap_read_poll(s, true);
}
- tap_read_poll(s, true);
+
s->vhost_net = NULL;
s->exit.notify = NULL;
@@ -798,9 +848,8 @@ static int net_tap_fd_init_common(const Netdev *netdev, NetClientState *peer,
}
assert(netdev->type == NET_CLIENT_DRIVER_TAP);
- tap = &netdev->u.tap;
- if (tap_set_sndbuf(s->fd, tap, errp) < 0) {
+ if (!local_incoming && tap_set_sndbuf(s->fd, tap, errp) < 0) {
goto failed;
}
@@ -826,9 +875,11 @@ static int net_tap_fd_init_common(const Netdev *netdev, NetClientState *peer,
goto failed;
}
- ret = net_tap_init_vhost(s, errp);
- if (ret < 0) {
- goto failed;
+ if (!local_incoming) {
+ ret = net_tap_init_vhost(s, errp);
+ if (ret < 0) {
+ goto failed;
+ }
}
return 0;
@@ -895,6 +946,38 @@ static int net_tap_open(const Netdev *netdev,
return 0;
}
+static int net_init_local_incoming(const Netdev *netdev,
+ const char *name,
+ NetClientState *peer,
+ Error **errp)
+{
+ const NetdevTapOptions *tap = &netdev->u.tap;
+ const char *downscript = tap->downscript;
+ int queues = tap->has_queues ? tap->queues : 1;
+ g_autofree char *default_downscript = NULL;
+ int i, ret;
+
+ assert(netdev->type == NET_CLIENT_DRIVER_TAP);
+ assert(!tap->script);
+
+ if (!downscript) {
+ downscript = default_downscript =
+ get_relocated_path(DEFAULT_NETWORK_DOWN_SCRIPT);
+ }
+
+ for (i = 0; i < queues; i++) {
+ ret = net_tap_fd_init_common(netdev, peer, "tap", name,
+ tap->ifname ?: "",
+ "no", downscript,
+ tap->vhostfd, -1, -1, errp);
+ if (ret < 0) {
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
static int net_init_tap_fds(const Netdev *netdev, const char *name,
NetClientState *peer, Error **errp)
{
@@ -983,6 +1066,13 @@ int net_init_tap(const Netdev *netdev, const char *name,
}
return net_init_bridge(netdev, name, peer, errp);
+ } else if (tap->local_incoming) {
+ if (tap->script) {
+ error_setg(errp, "script= is invalid with local-incoming");
+ return -1;
+ }
+
+ return net_init_local_incoming(netdev, name, peer, errp);
}
if (tap->vhostfds) {
@@ -1031,3 +1121,10 @@ int tap_disable(NetClientState *nc)
return ret;
}
}
+
+bool tap_local_incoming(NetClientState *nc)
+{
+ TAPState *s = DO_UPCAST(TAPState, nc, nc);
+
+ return s->local_incoming && runstate_check(RUN_STATE_INMIGRATE);
+}
--
2.48.1On 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.
Thanks,
--
Peter Xu
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),
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.
--
Best regards,
Vladimir
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
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?
>
>>
>> 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.
--
Best regards,
Vladimir
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
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
© 2016 - 2026 Red Hat, Inc.