[PATCH v5 00/19] virtio-net: live-TAP local migration

[PATCH v5 00/19] virtio-net: live-TAP local migration

[Vladimir Sementsov-Ogievskiy]

Hi all!

Here is a  new migration parameter fds, which allows to enable the new
local migration of TAP device, including its properties
and open fds.

With this new option, management software doesn't need to
initialize new TAP and do a switch to it. Nothing should be
done around virtio-net in local migration: it just migrates
and continues to use same TAP device. So we avoid extra logic
in management software, extra allocations in kenel (for new TAP),
and corresponding extra delay in migration downtime.

v5:

Follows [PATCH v3 0/9] virtio-net: live-TAP local migration,
but called v5, to not conflict with

   [PATCH v4 0/8] TAP initialization refactoring

, which becomes obsolete, and with

   [PATCH v4 0/2] save qemu-file incoming non-blocking fds

(two patches simply included here, to not create extra
dependancy).

What was changed:

1. Now, based on master.

As already said patches 01-02 are from

   [PATCH v4 0/2] save qemu-file incoming non-blocking fds

(which still may be merged in separate)

Also note, that the series is in small conflict with
in-flight

   [PATCH v5 00/13] io: deal with blocking/non-blocking fds

, I'll rebase if it merged first.

2. New interface: simply one migration parameter

    fds = [ virtio-net ]

It could simply be reduced to boolean fds=true, but I think
that a possibility to specify target is good, see more in
patch 16.

The series supersedes:
[PATCH v4 0/8] TAP initialization refactoring
Supersedes: <20250911165101.1637608-1-vsementsov@yandex-team.ru>

Vladimir Sementsov-Ogievskiy (19):
  migration/qemu-file: don't make incoming fds blocking again
  io/channel: document how qio_channel_readv_full() handles fds
  net/tap: net_init_tap_one(): drop extra error propagation
  net/tap: net_init_tap_one(): move parameter checking earlier
  net/tap: rework net_tap_init()
  net/tap: setup exit notifier only when needed
  net/tap: split net_tap_fd_init()
  net/tap: rework tap_set_sndbuf()
  net/tap: rework sndbuf handling
  net/tap: introduce net_tap_setup()
  net/tap: move vhost fd initialization to net_tap_new()
  net/tap: use net_tap_setup() in net_init_bridge()
  net/tap: finalize net_tap_set_fd() logic
  migration: add MIG_EVENT_PRE_INCOMING
  net/tap: postpone tap setup to pre-incoming
  qapi: add interface for local TAP migration
  virtio-net: support fds migration of TAP backend
  tests/functional: add skipUnlessPasswordlessSudo() decorator
  tests/functional: add test_x86_64_tap_fd_migration

 hw/net/virtio-net.c                           | 138 +++++-
 include/io/channel.h                          |  18 +
 include/migration/misc.h                      |   1 +
 include/net/tap.h                             |   5 +
 include/qapi/util.h                           |  17 +
 io/channel-socket.c                           |  13 +-
 migration/migration.c                         |   8 +-
 migration/options.c                           |  25 ++
 migration/options.h                           |   2 +
 migration/qemu-file.c                         |   3 +-
 net/tap-bsd.c                                 |   3 +-
 net/tap-linux.c                               |  19 +-
 net/tap-solaris.c                             |   3 +-
 net/tap-stub.c                                |   3 +-
 net/tap.c                                     | 422 ++++++++++++++----
 net/tap_int.h                                 |   4 +-
 qapi/migration.json                           |  46 +-
 tests/functional/qemu_test/decorators.py      |  16 +
 .../test_x86_64_tap_fd_migration.py           | 343 ++++++++++++++
 19 files changed, 970 insertions(+), 119 deletions(-)
 create mode 100644 tests/functional/test_x86_64_tap_fd_migration.py

-- 
2.48.1

[PATCH v5 01/19] migration/qemu-file: don't make incoming fds blocking again

[Vladimir Sementsov-Ogievskiy]

In migration we want to pass fd "as is", not changing its
blocking status.

The only current user of these fds is CPR state (through VMSTATE_FD),
which of-course doesn't want to modify fds on target when source is
still running and use these fds.

Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/io/channel.h  |  1 +
 io/channel-socket.c   | 13 +++++++++----
 migration/qemu-file.c |  3 ++-
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/include/io/channel.h b/include/io/channel.h
index 234e5db70d..12266256a8 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -36,6 +36,7 @@ OBJECT_DECLARE_TYPE(QIOChannel, QIOChannelClass,
 
 #define QIO_CHANNEL_READ_FLAG_MSG_PEEK 0x1
 #define QIO_CHANNEL_READ_FLAG_RELAXED_EOF 0x2
+#define QIO_CHANNEL_READ_FLAG_FD_PRESERVE_BLOCKING 0x4
 
 typedef enum QIOChannelFeature QIOChannelFeature;
 
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 3b7ca924ff..21f8f2e0c5 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -464,7 +464,8 @@ static void qio_channel_socket_finalize(Object *obj)
 
 #ifndef WIN32
 static void qio_channel_socket_copy_fds(struct msghdr *msg,
-                                        int **fds, size_t *nfds)
+                                        int **fds, size_t *nfds,
+                                        bool preserve_blocking)
 {
     struct cmsghdr *cmsg;
 
@@ -497,8 +498,10 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
                 continue;
             }
 
-            /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
-            qemu_socket_set_block(fd);
+            if (!preserve_blocking) {
+                /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
+                qemu_socket_set_block(fd);
+            }
 
 #ifndef MSG_CMSG_CLOEXEC
             qemu_set_cloexec(fd);
@@ -556,7 +559,9 @@ static ssize_t qio_channel_socket_readv(QIOChannel *ioc,
     }
 
     if (fds && nfds) {
-        qio_channel_socket_copy_fds(&msg, fds, nfds);
+        qio_channel_socket_copy_fds(
+            &msg, fds, nfds,
+            flags & QIO_CHANNEL_READ_FLAG_FD_PRESERVE_BLOCKING);
     }
 
     return ret;
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index b6ac190034..d5c6e7ec61 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -340,7 +340,8 @@ static ssize_t coroutine_mixed_fn qemu_fill_buffer(QEMUFile *f)
 
     do {
         struct iovec iov = { f->buf + pending, IO_BUF_SIZE - pending };
-        len = qio_channel_readv_full(f->ioc, &iov, 1, pfds, pnfd, 0,
+        len = qio_channel_readv_full(f->ioc, &iov, 1, pfds, pnfd,
+                                     QIO_CHANNEL_READ_FLAG_FD_PRESERVE_BLOCKING,
                                      &local_error);
         if (len == QIO_CHANNEL_ERR_BLOCK) {
             if (qemu_in_coroutine()) {
-- 
2.48.1

[PATCH v5 02/19] io/channel: document how qio_channel_readv_full() handles fds

[Vladimir Sementsov-Ogievskiy]

The only realization, which may have incoming fds is
qio_channel_socket_readv() (in io/channel-socket.c).
qio_channel_socket_readv() do call (through
qio_channel_socket_copy_fds()) qemu_socket_set_block() and
qemu_set_cloexec() for each fd.

Also, qio_channel_socket_copy_fds() is called at the end of
qio_channel_socket_readv(), on success path.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Acked-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/io/channel.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/io/channel.h b/include/io/channel.h
index 12266256a8..c7f64506f7 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -118,6 +118,15 @@ struct QIOChannelClass {
                          size_t nfds,
                          int flags,
                          Error **errp);
+
+    /*
+     * The io_readv handler must guarantee that all
+     * incoming fds are set BLOCKING (unless
+     * QIO_CHANNEL_READ_FLAG_FD_PRESERVE_BLOCKING flag is set) and
+     * CLOEXEC (if available).
+     * @fds and @nfds are set only on success path, and untouched
+     * in case of errors.
+     */
     ssize_t (*io_readv)(QIOChannel *ioc,
                         const struct iovec *iov,
                         size_t niov,
@@ -125,6 +134,7 @@ struct QIOChannelClass {
                         size_t *nfds,
                         int flags,
                         Error **errp);
+
     int (*io_close)(QIOChannel *ioc,
                     Error **errp);
     GSource * (*io_create_watch)(QIOChannel *ioc,
@@ -235,6 +245,13 @@ void qio_channel_set_name(QIOChannel *ioc,
  * was allocated. It is the callers responsibility
  * to call close() on each file descriptor and to
  * call g_free() on the array pointer in @fds.
+ * @fds allocated and set (and @nfds is set too)
+ * _only_ on success path. These parameters are
+ * untouched in case of errors.
+ * qio_channel_readv_full() guarantees that all
+ * incoming fds are set BLOCKING (unless
+ * QIO_CHANNEL_READ_FLAG_FD_PRESERVE_BLOCKING flag
+ * is set) and CLOEXEC (if available).
  *
  * It is an error to pass a non-NULL @fds parameter
  * unless qio_channel_has_feature() returns a true
-- 
2.48.1

[PATCH v5 03/19] net/tap: net_init_tap_one(): drop extra error propagation

[Vladimir Sementsov-Ogievskiy]

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 net/tap.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index f7df702f97..10799ab055 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -724,9 +724,8 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
         }
 
         if (vhostfdname) {
-            vhostfd = monitor_fd_param(monitor_cur(), vhostfdname, &err);
+            vhostfd = monitor_fd_param(monitor_cur(), vhostfdname, errp);
             if (vhostfd == -1) {
-                error_propagate(errp, err);
                 goto failed;
             }
             if (!g_unix_set_fd_nonblocking(vhostfd, true, NULL)) {
-- 
2.48.1

[PATCH v5 04/19] net/tap: net_init_tap_one(): move parameter checking earlier

[Vladimir Sementsov-Ogievskiy]

Let's keep all similar argument checking in net_init_tap() function.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 net/tap.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index 10799ab055..e268d79a97 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -759,9 +759,6 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
                        "vhost-net requested but could not be initialized");
             goto failed;
         }
-    } else if (vhostfdname) {
-        error_setg(errp, "vhostfd(s)= is not valid without vhost");
-        goto failed;
     }
 
     return;
@@ -823,6 +820,11 @@ int net_init_tap(const Netdev *netdev, const char *name,
         return -1;
     }
 
+    if (tap->has_vhost && !tap->vhost && (tap->vhostfds || tap->vhostfd)) {
+        error_setg(errp, "vhostfd(s)= is not valid without vhost");
+        return -1;
+    }
+
     if (tap->fd) {
         if (tap->ifname || tap->script || tap->downscript ||
             tap->has_vnet_hdr || tap->helper || tap->has_queues ||
-- 
2.48.1

[PATCH v5 05/19] net/tap: rework net_tap_init()

[Vladimir Sementsov-Ogievskiy]

In future (to support live-TAP migration with fds passing through
unix socket) we'll want to postpone fd-initialization to the later
point, when QAPI structured parameters are not available. So, let's
now rework the function to interface without "tap" parameter.

Also, rename to net_tap_open(), as it's just a wrapper on tap_open(),
and having net_tap_init() and net_init_tap() functions in one file
is confusing.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 net/tap.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index e268d79a97..1e5eaae6e7 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -643,20 +643,12 @@ int net_init_bridge(const Netdev *netdev, const char *name,
     return 0;
 }
 
-static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr,
+static int net_tap_open(int *vnet_hdr, bool vnet_hdr_required,
                         const char *setup_script, char *ifname,
                         size_t ifname_sz, int mq_required, Error **errp)
 {
     Error *err = NULL;
-    int fd, vnet_hdr_required;
-
-    if (tap->has_vnet_hdr) {
-        *vnet_hdr = tap->vnet_hdr;
-        vnet_hdr_required = *vnet_hdr;
-    } else {
-        *vnet_hdr = 1;
-        vnet_hdr_required = 0;
-    }
+    int fd;
 
     fd = RETRY_ON_EINTR(tap_open(ifname, ifname_sz, vnet_hdr, vnet_hdr_required,
                       mq_required, errp));
@@ -973,6 +965,8 @@ free_fail:
     } else {
         g_autofree char *default_script = NULL;
         g_autofree char *default_downscript = NULL;
+        bool vnet_hdr_required = tap->has_vnet_hdr && tap->vnet_hdr;
+
         if (tap->vhostfds) {
             error_setg(errp, "vhostfds= is invalid if fds= wasn't specified");
             return -1;
@@ -993,7 +987,9 @@ free_fail:
         }
 
         for (i = 0; i < queues; i++) {
-            fd = net_tap_init(tap, &vnet_hdr, i >= 1 ? "no" : script,
+            vnet_hdr = tap->has_vnet_hdr ? tap->vnet_hdr : 1;
+            fd = net_tap_open(&vnet_hdr, vnet_hdr_required,
+                              i >= 1 ? "no" : script,
                               ifname, sizeof ifname, queues > 1, errp);
             if (fd == -1) {
                 return -1;
-- 
2.48.1

[PATCH v5 06/19] net/tap: setup exit notifier only when needed

[Vladimir Sementsov-Ogievskiy]

No reason to setup notifier on each queue of multique tap,
when we actually want to run downscript only once.
As well, let's not setup notifier, when downscript is
not enabled (downsciprt="no").

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 net/tap.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index 1e5eaae6e7..eabfc0b8d8 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -301,11 +301,9 @@ static void tap_exit_notify(Notifier *notifier, void *data)
     TAPState *s = container_of(notifier, TAPState, exit);
     Error *err = NULL;
 
-    if (s->down_script[0]) {
-        launch_script(s->down_script, s->down_script_arg, s->fd, &err);
-        if (err) {
-            error_report_err(err);
-        }
+    launch_script(s->down_script, s->down_script_arg, s->fd, &err);
+    if (err) {
+        error_report_err(err);
     }
 }
 
@@ -321,8 +319,11 @@ static void tap_cleanup(NetClientState *nc)
 
     qemu_purge_queued_packets(nc);
 
-    tap_exit_notify(&s->exit, NULL);
-    qemu_remove_exit_notifier(&s->exit);
+    if (s->exit.notify) {
+        tap_exit_notify(&s->exit, NULL);
+        qemu_remove_exit_notifier(&s->exit);
+        s->exit.notify = NULL;
+    }
 
     tap_read_poll(s, false);
     tap_write_poll(s, false);
@@ -415,9 +416,6 @@ static TAPState *net_tap_fd_init(NetClientState *peer,
     tap_read_poll(s, true);
     s->vhost_net = NULL;
 
-    s->exit.notify = tap_exit_notify;
-    qemu_add_exit_notifier(&s->exit);
-
     return s;
 }
 
@@ -700,6 +698,8 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
             snprintf(s->down_script, sizeof(s->down_script), "%s", downscript);
             snprintf(s->down_script_arg, sizeof(s->down_script_arg),
                      "%s", ifname);
+            s->exit.notify = tap_exit_notify;
+            qemu_add_exit_notifier(&s->exit);
         }
     }
 
-- 
2.48.1

[PATCH v5 07/19] net/tap: split net_tap_fd_init()

[Vladimir Sementsov-Ogievskiy]

Split the function into separate net_tap_new() and net_tap_set_fd().

We start move to the following picture:

net_tap_new() - take QAPI @tap parameter, but don't have @fd,
initialize the net client, called during initialization.

net_tap_setup() - don't have @tap (QAPI), but have @fd parameter,
may be called at later point.

In this commit we introduce the first function.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 net/tap.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index eabfc0b8d8..3050fdea2e 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -386,19 +386,19 @@ static NetClientInfo net_tap_info = {
     .get_vhost_net = tap_get_vhost_net,
 };
 
-static TAPState *net_tap_fd_init(NetClientState *peer,
-                                 const char *model,
-                                 const char *name,
-                                 int fd,
-                                 int vnet_hdr)
+static TAPState *net_tap_new(NetClientState *peer, const char *model,
+                             const char *name)
 {
-    NetClientState *nc;
-    TAPState *s;
+    NetClientState *nc = qemu_new_net_client(&net_tap_info, peer, model, name);
+    TAPState *s = DO_UPCAST(TAPState, nc, nc);
 
-    nc = qemu_new_net_client(&net_tap_info, peer, model, name);
+    s->fd = -1;
 
-    s = DO_UPCAST(TAPState, nc, nc);
+    return s;
+}
 
+static void net_tap_set_fd(TAPState *s, int fd, int vnet_hdr)
+{
     s->fd = fd;
     s->host_vnet_hdr_len = vnet_hdr ? sizeof(struct virtio_net_hdr) : 0;
     s->using_vnet_hdr = false;
@@ -415,8 +415,6 @@ static TAPState *net_tap_fd_init(NetClientState *peer,
     }
     tap_read_poll(s, true);
     s->vhost_net = NULL;
-
-    return s;
 }
 
 static void close_all_fds_after_fork(int excluded_fd)
@@ -634,7 +632,9 @@ int net_init_bridge(const Netdev *netdev, const char *name,
         close(fd);
         return -1;
     }
-    s = net_tap_fd_init(peer, "bridge", name, fd, vnet_hdr);
+
+    s = net_tap_new(peer, "bridge", name);
+    net_tap_set_fd(s, fd, vnet_hdr);
 
     qemu_set_info_str(&s->nc, "helper=%s,br=%s", helper, br);
 
@@ -677,9 +677,11 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
                              int vnet_hdr, int fd, Error **errp)
 {
     Error *err = NULL;
-    TAPState *s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
+    TAPState *s = net_tap_new(peer, model, name);
     int vhostfd;
 
+    net_tap_set_fd(s, fd, vnet_hdr);
+
     tap_set_sndbuf(s->fd, tap, &err);
     if (err) {
         error_propagate(errp, err);
-- 
2.48.1

[PATCH v5 08/19] net/tap: rework tap_set_sndbuf()

[Vladimir Sementsov-Ogievskiy]

First, follow common recommendations to avoid error propagation:
add return value to tap_set_sndbuf().

Second, keep NetdevTapOptions related logic in tap.c, and make
tap_set_sndbuf a simple system call wrapper, more like other functions
in tap-linux.c

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 net/tap-bsd.c     |  3 ++-
 net/tap-linux.c   | 19 +++++--------------
 net/tap-solaris.c |  3 ++-
 net/tap-stub.c    |  3 ++-
 net/tap.c         |  9 +++++----
 net/tap_int.h     |  4 +---
 6 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index b4c84441ba..3bfc1cc577 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -198,8 +198,9 @@ error:
 }
 #endif /* __FreeBSD__ */
 
-void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
+bool tap_set_sndbuf(int fd, int sndbuf, Error **errp)
 {
+    return true;
 }
 
 int tap_probe_vnet_hdr(int fd, Error **errp)
diff --git a/net/tap-linux.c b/net/tap-linux.c
index 22ec2f45d2..c46f488c08 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -138,23 +138,14 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
  * Ethernet NICs generally have txqueuelen=1000, so 1Mb is
  * a good value, given a 1500 byte MTU.
  */
-#define TAP_DEFAULT_SNDBUF 0
-
-void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
+bool tap_set_sndbuf(int fd, int sndbuf, Error **errp)
 {
-    int sndbuf;
-
-    sndbuf = !tap->has_sndbuf       ? TAP_DEFAULT_SNDBUF :
-             tap->sndbuf > INT_MAX  ? INT_MAX :
-             tap->sndbuf;
-
-    if (!sndbuf) {
-        sndbuf = INT_MAX;
-    }
-
-    if (ioctl(fd, TUNSETSNDBUF, &sndbuf) == -1 && tap->has_sndbuf) {
+    if (ioctl(fd, TUNSETSNDBUF, &sndbuf) == -1) {
         error_setg_errno(errp, errno, "TUNSETSNDBUF ioctl failed");
+        return false;
     }
+
+    return true;
 }
 
 int tap_probe_vnet_hdr(int fd, Error **errp)
diff --git a/net/tap-solaris.c b/net/tap-solaris.c
index 51b7830bef..2932c2de39 100644
--- a/net/tap-solaris.c
+++ b/net/tap-solaris.c
@@ -202,8 +202,9 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
     return fd;
 }
 
-void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
+bool tap_set_sndbuf(int fd, int sndbuf, Error **errp)
 {
+    return true;
 }
 
 int tap_probe_vnet_hdr(int fd, Error **errp)
diff --git a/net/tap-stub.c b/net/tap-stub.c
index 38673434cb..326e76843e 100644
--- a/net/tap-stub.c
+++ b/net/tap-stub.c
@@ -33,8 +33,9 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
     return -1;
 }
 
-void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
+bool tap_set_sndbuf(int fd, int sndbuf, Error **errp)
 {
+    return true;
 }
 
 int tap_probe_vnet_hdr(int fd, Error **errp)
diff --git a/net/tap.c b/net/tap.c
index 3050fdea2e..5cb639a71d 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -676,15 +676,16 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
                              const char *downscript, const char *vhostfdname,
                              int vnet_hdr, int fd, Error **errp)
 {
-    Error *err = NULL;
     TAPState *s = net_tap_new(peer, model, name);
     int vhostfd;
+    bool sndbuf_required = tap->has_sndbuf;
+    int sndbuf =
+        (tap->has_sndbuf && tap->sndbuf) ? MIN(tap->sndbuf, INT_MAX) : INT_MAX;
 
     net_tap_set_fd(s, fd, vnet_hdr);
 
-    tap_set_sndbuf(s->fd, tap, &err);
-    if (err) {
-        error_propagate(errp, err);
+    if (!tap_set_sndbuf(fd, sndbuf, sndbuf_required ? errp : NULL) &&
+        sndbuf_required) {
         goto failed;
     }
 
diff --git a/net/tap_int.h b/net/tap_int.h
index 8857ff299d..08e4a592a0 100644
--- a/net/tap_int.h
+++ b/net/tap_int.h
@@ -26,14 +26,12 @@
 #ifndef NET_TAP_INT_H
 #define NET_TAP_INT_H
 
-#include "qapi/qapi-types-net.h"
-
 int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
              int vnet_hdr_required, int mq_required, Error **errp);
 
 ssize_t tap_read_packet(int tapfd, uint8_t *buf, int maxlen);
 
-void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp);
+bool tap_set_sndbuf(int fd, int sndbuf, Error **errp);
 int tap_probe_vnet_hdr(int fd, Error **errp);
 int tap_probe_has_ufo(int fd);
 int tap_probe_has_uso(int fd);
-- 
2.48.1

[PATCH v5 09/19] net/tap: rework sndbuf handling

[Vladimir Sementsov-Ogievskiy]

Continue the main idea: avoid dependency on @tap in net_tap_setup().
So, move QAPI parsing to net_tap_new().
Move setting sndbuf to net_tap_set_fd(), as it's more appropriate place
(other initial fd settings are here).

Note that net_tap_new() and net_tap_set_fd() are shared with
net_init_bridge(), which didn't set sndbuf. Handle this case by sndbuf=0
(we never pass zero to tap_set_sndbuf(), so let this specific value mean
that we don't want touch sndbuf).

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 net/tap.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index 5cb639a71d..87d851203b 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -80,6 +80,9 @@ typedef struct TAPState {
     VHostNetState *vhost_net;
     unsigned host_vnet_hdr_len;
     Notifier exit;
+
+    bool sndbuf_required;
+    int sndbuf;
 } TAPState;
 
 static void launch_script(const char *setup_script, const char *ifname,
@@ -387,17 +390,25 @@ static NetClientInfo net_tap_info = {
 };
 
 static TAPState *net_tap_new(NetClientState *peer, const char *model,
-                             const char *name)
+                             const char *name, const NetdevTapOptions *tap)
 {
     NetClientState *nc = qemu_new_net_client(&net_tap_info, peer, model, name);
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
 
     s->fd = -1;
 
+    if (!tap) {
+        return s;
+    }
+
+    s->sndbuf_required = tap->has_sndbuf;
+    s->sndbuf =
+        (tap->has_sndbuf && tap->sndbuf) ? MIN(tap->sndbuf, INT_MAX) : INT_MAX;
+
     return s;
 }
 
-static void net_tap_set_fd(TAPState *s, int fd, int vnet_hdr)
+static bool net_tap_set_fd(TAPState *s, int fd, int vnet_hdr, Error **errp)
 {
     s->fd = fd;
     s->host_vnet_hdr_len = vnet_hdr ? sizeof(struct virtio_net_hdr) : 0;
@@ -415,6 +426,15 @@ static void net_tap_set_fd(TAPState *s, int fd, int vnet_hdr)
     }
     tap_read_poll(s, true);
     s->vhost_net = NULL;
+
+    if (s->sndbuf) {
+        Error **e = s->sndbuf_required ? errp : NULL;
+        if (!tap_set_sndbuf(s->fd, s->sndbuf, e) && s->sndbuf_required) {
+            return false;
+        }
+    }
+
+    return true;
 }
 
 static void close_all_fds_after_fork(int excluded_fd)
@@ -633,8 +653,8 @@ int net_init_bridge(const Netdev *netdev, const char *name,
         return -1;
     }
 
-    s = net_tap_new(peer, "bridge", name);
-    net_tap_set_fd(s, fd, vnet_hdr);
+    s = net_tap_new(peer, "bridge", name, NULL);
+    net_tap_set_fd(s, fd, vnet_hdr, &error_abort);
 
     qemu_set_info_str(&s->nc, "helper=%s,br=%s", helper, br);
 
@@ -676,16 +696,10 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
                              const char *downscript, const char *vhostfdname,
                              int vnet_hdr, int fd, Error **errp)
 {
-    TAPState *s = net_tap_new(peer, model, name);
+    TAPState *s = net_tap_new(peer, model, name, tap);
     int vhostfd;
-    bool sndbuf_required = tap->has_sndbuf;
-    int sndbuf =
-        (tap->has_sndbuf && tap->sndbuf) ? MIN(tap->sndbuf, INT_MAX) : INT_MAX;
-
-    net_tap_set_fd(s, fd, vnet_hdr);
 
-    if (!tap_set_sndbuf(fd, sndbuf, sndbuf_required ? errp : NULL) &&
-        sndbuf_required) {
+    if (!net_tap_set_fd(s, fd, vnet_hdr, errp)) {
         goto failed;
     }
 
-- 
2.48.1

[PATCH v5 10/19] net/tap: introduce net_tap_setup()

[Vladimir Sementsov-Ogievskiy]

Move most of net_init_tap_one() to net_tap_setup() - future pair
for net_tap_new(), for postponed setup.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 net/tap.c | 43 +++++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index 87d851203b..b4682d7400 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -85,6 +85,10 @@ typedef struct TAPState {
     int sndbuf;
 } TAPState;
 
+static bool net_tap_setup(TAPState *s, const NetdevTapOptions *tap,
+                          const char *vhostfdname,
+                          int fd, int vnet_hdr, Error **errp);
+
 static void launch_script(const char *setup_script, const char *ifname,
                           int fd, Error **errp);
 
@@ -697,11 +701,6 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
                              int vnet_hdr, int fd, Error **errp)
 {
     TAPState *s = net_tap_new(peer, model, name, tap);
-    int vhostfd;
-
-    if (!net_tap_set_fd(s, fd, vnet_hdr, errp)) {
-        goto failed;
-    }
 
     if (tap->fd || tap->fds) {
         qemu_set_info_str(&s->nc, "fd=%d", fd);
@@ -720,6 +719,21 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
         }
     }
 
+    if (!net_tap_setup(s, tap, vhostfdname, fd, vnet_hdr, errp)) {
+        qemu_del_net_client(&s->nc);
+    }
+}
+
+static bool net_tap_setup(TAPState *s, const NetdevTapOptions *tap,
+                          const char *vhostfdname,
+                          int fd, int vnet_hdr, Error **errp)
+{
+    int vhostfd;
+
+    if (!net_tap_set_fd(s, fd, vnet_hdr, errp)) {
+        return false;
+    }
+
     if (tap->has_vhost ? tap->vhost :
         vhostfdname || (tap->has_vhostforce && tap->vhostforce)) {
         VhostNetOptions options;
@@ -735,23 +749,23 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
         if (vhostfdname) {
             vhostfd = monitor_fd_param(monitor_cur(), vhostfdname, errp);
             if (vhostfd == -1) {
-                goto failed;
+                return false;
             }
             if (!g_unix_set_fd_nonblocking(vhostfd, true, NULL)) {
-                error_setg_errno(errp, errno, "%s: Can't use file descriptor %d",
-                                 name, fd);
-                goto failed;
+                error_setg_errno(errp, errno, "Can't use file descriptor %d",
+                                 fd);
+                return false;
             }
         } else {
             vhostfd = open("/dev/vhost-net", O_RDWR);
             if (vhostfd < 0) {
                 error_setg_errno(errp, errno,
                                  "tap: open vhost char device failed");
-                goto failed;
+                return false;
             }
             if (!g_unix_set_fd_nonblocking(vhostfd, true, NULL)) {
                 error_setg_errno(errp, errno, "Failed to set FD nonblocking");
-                goto failed;
+                return false;
             }
         }
         options.opaque = (void *)(uintptr_t)vhostfd;
@@ -766,14 +780,11 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
         if (!s->vhost_net) {
             error_setg(errp,
                        "vhost-net requested but could not be initialized");
-            goto failed;
+            return false;
         }
     }
 
-    return;
-
-failed:
-    qemu_del_net_client(&s->nc);
+    return true;
 }
 
 static int get_fds(char *str, char *fds[], int max)
-- 
2.48.1

[PATCH v5 11/19] net/tap: move vhost fd initialization to net_tap_new()

[Vladimir Sementsov-Ogievskiy]

Last step to get rid of dependency on @tap in net_tap_setup().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 net/tap.c | 96 ++++++++++++++++++++++++++++++-------------------------
 1 file changed, 53 insertions(+), 43 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index b4682d7400..b7175e4b10 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -83,11 +83,11 @@ typedef struct TAPState {
 
     bool sndbuf_required;
     int sndbuf;
+    int vhostfd;
+    uint32_t vhost_busyloop_timeout;
 } TAPState;
 
-static bool net_tap_setup(TAPState *s, const NetdevTapOptions *tap,
-                          const char *vhostfdname,
-                          int fd, int vnet_hdr, Error **errp);
+static bool net_tap_setup(TAPState *s, int fd, int vnet_hdr, Error **errp);
 
 static void launch_script(const char *setup_script, const char *ifname,
                           int fd, Error **errp);
@@ -336,6 +336,11 @@ static void tap_cleanup(NetClientState *nc)
     tap_write_poll(s, false);
     close(s->fd);
     s->fd = -1;
+
+    if (s->vhostfd != -1) {
+        close(s->vhostfd);
+        s->vhostfd = -1;
+    }
 }
 
 static void tap_poll(NetClientState *nc, bool enable)
@@ -394,12 +399,14 @@ static NetClientInfo net_tap_info = {
 };
 
 static TAPState *net_tap_new(NetClientState *peer, const char *model,
-                             const char *name, const NetdevTapOptions *tap)
+                             const char *name, const NetdevTapOptions *tap,
+                             const char *vhostfdname, Error **errp)
 {
     NetClientState *nc = qemu_new_net_client(&net_tap_info, peer, model, name);
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
 
     s->fd = -1;
+    s->vhostfd = -1;
 
     if (!tap) {
         return s;
@@ -409,7 +416,39 @@ static TAPState *net_tap_new(NetClientState *peer, const char *model,
     s->sndbuf =
         (tap->has_sndbuf && tap->sndbuf) ? MIN(tap->sndbuf, INT_MAX) : INT_MAX;
 
+    if (tap->has_vhost ? tap->vhost :
+        vhostfdname || (tap->has_vhostforce && tap->vhostforce)) {
+        if (vhostfdname) {
+            s->vhostfd = monitor_fd_param(monitor_cur(), vhostfdname, errp);
+            if (s->vhostfd == -1) {
+                goto failed;
+            }
+            if (!g_unix_set_fd_nonblocking(s->vhostfd, true, NULL)) {
+                error_setg_errno(errp, errno, "Can't use file descriptor %d",
+                                 s->fd);
+                goto failed;
+            }
+        } else {
+            s->vhostfd = open("/dev/vhost-net", O_RDWR);
+            if (s->vhostfd < 0) {
+                error_setg_errno(errp, errno,
+                                 "tap: open vhost char device failed");
+                goto failed;
+            }
+            if (!g_unix_set_fd_nonblocking(s->vhostfd, true, NULL)) {
+                error_setg_errno(errp, errno, "Failed to set FD nonblocking");
+                goto failed;
+            }
+        }
+
+        s->vhost_busyloop_timeout = tap->poll_us;
+    }
+
     return s;
+
+failed:
+    qemu_del_net_client(&s->nc);
+    return NULL;
 }
 
 static bool net_tap_set_fd(TAPState *s, int fd, int vnet_hdr, Error **errp)
@@ -657,7 +696,7 @@ int net_init_bridge(const Netdev *netdev, const char *name,
         return -1;
     }
 
-    s = net_tap_new(peer, "bridge", name, NULL);
+    s = net_tap_new(peer, "bridge", name, NULL, NULL, &error_abort);
     net_tap_set_fd(s, fd, vnet_hdr, &error_abort);
 
     qemu_set_info_str(&s->nc, "helper=%s,br=%s", helper, br);
@@ -700,7 +739,10 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
                              const char *downscript, const char *vhostfdname,
                              int vnet_hdr, int fd, Error **errp)
 {
-    TAPState *s = net_tap_new(peer, model, name, tap);
+    TAPState *s = net_tap_new(peer, model, name, tap, vhostfdname, errp);
+    if (!s) {
+        return;
+    }
 
     if (tap->fd || tap->fds) {
         qemu_set_info_str(&s->nc, "fd=%d", fd);
@@ -719,56 +761,24 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
         }
     }
 
-    if (!net_tap_setup(s, tap, vhostfdname, fd, vnet_hdr, errp)) {
+    if (!net_tap_setup(s, fd, vnet_hdr, errp)) {
         qemu_del_net_client(&s->nc);
     }
 }
 
-static bool net_tap_setup(TAPState *s, const NetdevTapOptions *tap,
-                          const char *vhostfdname,
-                          int fd, int vnet_hdr, Error **errp)
+static bool net_tap_setup(TAPState *s, int fd, int vnet_hdr, Error **errp)
 {
-    int vhostfd;
-
     if (!net_tap_set_fd(s, fd, vnet_hdr, errp)) {
         return false;
     }
 
-    if (tap->has_vhost ? tap->vhost :
-        vhostfdname || (tap->has_vhostforce && tap->vhostforce)) {
+    if (s->vhostfd != -1) {
         VhostNetOptions options;
 
         options.backend_type = VHOST_BACKEND_TYPE_KERNEL;
         options.net_backend = &s->nc;
-        if (tap->has_poll_us) {
-            options.busyloop_timeout = tap->poll_us;
-        } else {
-            options.busyloop_timeout = 0;
-        }
-
-        if (vhostfdname) {
-            vhostfd = monitor_fd_param(monitor_cur(), vhostfdname, errp);
-            if (vhostfd == -1) {
-                return false;
-            }
-            if (!g_unix_set_fd_nonblocking(vhostfd, true, NULL)) {
-                error_setg_errno(errp, errno, "Can't use file descriptor %d",
-                                 fd);
-                return false;
-            }
-        } else {
-            vhostfd = open("/dev/vhost-net", O_RDWR);
-            if (vhostfd < 0) {
-                error_setg_errno(errp, errno,
-                                 "tap: open vhost char device failed");
-                return false;
-            }
-            if (!g_unix_set_fd_nonblocking(vhostfd, true, NULL)) {
-                error_setg_errno(errp, errno, "Failed to set FD nonblocking");
-                return false;
-            }
-        }
-        options.opaque = (void *)(uintptr_t)vhostfd;
+        options.busyloop_timeout = s->vhost_busyloop_timeout;
+        options.opaque = (void *)(uintptr_t)s->vhostfd;
         options.nvqs = 2;
         options.feature_bits = kernel_feature_bits;
         options.get_acked_features = NULL;
-- 
2.48.1

[PATCH v5 12/19] net/tap: use net_tap_setup() in net_init_bridge()

[Vladimir Sementsov-Ogievskiy]

Finalize the concept of pair net_tap_new() + net_tap_setup().

Now, the only extra part for bridge in net_tap_setup() is initializing
vhost_net, but it's under if (s->vhostfd != -1), we never come into it
for bridge case.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 net/tap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/tap.c b/net/tap.c
index b7175e4b10..4ca3cc75d8 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -697,7 +697,7 @@ int net_init_bridge(const Netdev *netdev, const char *name,
     }
 
     s = net_tap_new(peer, "bridge", name, NULL, NULL, &error_abort);
-    net_tap_set_fd(s, fd, vnet_hdr, &error_abort);
+    net_tap_setup(s, fd, vnet_hdr, &error_abort);
 
     qemu_set_info_str(&s->nc, "helper=%s,br=%s", helper, br);
 
-- 
2.48.1

[PATCH v5 13/19] net/tap: finalize net_tap_set_fd() logic

[Vladimir Sementsov-Ogievskiy]

Let net_tap_set_fd() do only fd-related setup.

Actually, for further fds-incoming migration we'll want to skip
net_tap_set_fd() (as incoming fds are already prepared by source
QEMU). So move tap_read_poll() to net_tap_setup().

Don't care about using_vnet_hdr and vhost_net, the state is
zero-initialized.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 net/tap.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index 4ca3cc75d8..29568ce5d2 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -455,7 +455,6 @@ static bool net_tap_set_fd(TAPState *s, int fd, int vnet_hdr, Error **errp)
 {
     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;
@@ -467,8 +466,6 @@ static bool net_tap_set_fd(TAPState *s, int fd, int vnet_hdr, Error **errp)
     if (vnet_hdr) {
         tap_fd_set_vnet_hdr_len(s->fd, s->host_vnet_hdr_len);
     }
-    tap_read_poll(s, true);
-    s->vhost_net = NULL;
 
     if (s->sndbuf) {
         Error **e = s->sndbuf_required ? errp : NULL;
@@ -772,6 +769,8 @@ static bool net_tap_setup(TAPState *s, int fd, int vnet_hdr, Error **errp)
         return false;
     }
 
+    tap_read_poll(s, true);
+
     if (s->vhostfd != -1) {
         VhostNetOptions options;
 
-- 
2.48.1

[PATCH v5 14/19] migration: add MIG_EVENT_PRE_INCOMING

[Vladimir Sementsov-Ogievskiy]

We are going to implement fds-passing migration feature. When it's
enabled (by setting migration parameter), TAP device (and later other
devices of-course) is passed through migration channel (which should be
a unix socket to pass fds) to a new QEMU.

So we need to know during TAP initialization, should we open TAP
device, or wait for incoming fds. But we can't check for migration
parameter at TAP creation time, as use may set migration parameter
later (especially when TAP is added by command line parameter).

To solve this, we have prepared TAP initialization code so that
opening the device may be postponed to later point. And this later
point is obviously the early beginning of qmp_migrate_incoming():
here we already know all migration parameters and capabilities,
but we are not in downtime, so we can continue initialization
of TAP device.

This commit introduces MIG_EVENT_PRE_INCOMING, to be used by TAP
code in following commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 include/migration/misc.h | 1 +
 migration/migration.c    | 8 +++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/migration/misc.h b/include/migration/misc.h
index a261f99d89..5765fcc204 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -63,6 +63,7 @@ typedef enum MigrationEventType {
     MIG_EVENT_PRECOPY_SETUP,
     MIG_EVENT_PRECOPY_DONE,
     MIG_EVENT_PRECOPY_FAILED,
+    MIG_EVENT_PRE_INCOMING,
     MIG_EVENT_MAX
 } MigrationEventType;
 
diff --git a/migration/migration.c b/migration/migration.c
index 10c216d25d..88daa13960 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1696,7 +1696,8 @@ int migration_call_notifiers(MigrationState *s, MigrationEventType type,
     e.type = type;
     ret = notifier_with_return_list_notify(&migration_state_notifiers[mode],
                                            &e, errp);
-    assert(!ret || type == MIG_EVENT_PRECOPY_SETUP);
+    assert(!ret || type == MIG_EVENT_PRECOPY_SETUP ||
+           type == MIG_EVENT_PRE_INCOMING);
     return ret;
 }
 
@@ -1936,6 +1937,11 @@ void qmp_migrate_incoming(const char *uri, bool has_channels,
         return;
     }
 
+    if (migration_call_notifiers(migrate_get_current(), MIG_EVENT_PRE_INCOMING,
+                                 errp)) {
+        return;
+    }
+
     if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) {
         return;
     }
-- 
2.48.1

[PATCH v5 15/19] net/tap: postpone tap setup to pre-incoming

[Vladimir Sementsov-Ogievskiy]

As described in previous commit, to support fds-passing TAP migration,
we need to postpone the decision to open the device or to wait for
incoming fds up to pre-incoming point (when we actually can decide).

This commit only postpones TAP-open case of initialization.
We don't try to postpone the all cases of initialization, as it will
require a lot more work of refactoring the code.

So we postpone only the simple case, for which we are going to support
fd-incoming migration:

1. No fds / fd parameters: obviously, if user give fd/fds the should
be used, no incoming fds migration is possible.

2. No helper: just for simplicity. It probably possible to allow it
(and just ignore in case of fds-incoming migration), to allow user
use same cmdline on target QEMU.. But that questionable, and
postponable.

3. No sciprt/downscript. It's not simple to support downscript:
we should pass the responsiblity to call it on target QEMU with
migration.. And back to source QEMU on migration failure. It
feasible, but may be implemented later on demand.

3. Concrete ifname and vnet_hdr: to not try to share them between
queues, when we only can setup queues as separate entities.
Supporting undecided ifname will require to create some extra
netdev state, connecting all the taps, to be able to iterate through
them.

No part of incoming-fds migration is here, we only prepare the code
for future implementation of it.

Are net-drivers prepared to postponed initialization of NICs?
For future feature of fds-passing migration, we are mainly interested
in virtio-net. So, let's prepare virtio-net to work with postponed
initialization of TAP (two places about early set/get features) and
for other drivers let's simply finalize initialization on setting
netdev property. Support for other drivers may be added later if
needed.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 hw/net/virtio-net.c |  65 ++++++++++++++++-
 include/net/tap.h   |   2 +
 net/tap.c           | 172 +++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 237 insertions(+), 2 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 6b5b5dace3..70f688fc3a 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -688,6 +688,21 @@ default_value:
     return VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE;
 }
 
+static bool peer_wait_incoming(VirtIONet *n)
+{
+    NetClientState *nc = qemu_get_queue(n->nic);
+
+    if (!nc->peer) {
+        return false;
+    }
+
+    if (nc->peer->info->type != NET_CLIENT_DRIVER_TAP) {
+        return false;
+    }
+
+    return tap_wait_incoming(nc->peer);
+}
+
 static int peer_attach(VirtIONet *n, int index)
 {
     NetClientState *nc = qemu_get_subqueue(n->nic, index);
@@ -2999,7 +3014,17 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue)
     n->multiqueue = multiqueue;
     virtio_net_change_num_queues(n, max * 2 + 1);
 
-    virtio_net_set_queue_pairs(n);
+    /*
+     * virtio_net_set_multiqueue() called from set_features(0) on early
+     * reset, when peer may wait for incoming (and is not initialized
+     * yet).
+     * Don't worry about it: virtio_net_set_queue_pairs() will be called
+     * later form virtio_net_post_load_device(), and anyway will be
+     * noop for local incoming migration with live backend passing.
+     */
+    if (!peer_wait_incoming(n)) {
+        virtio_net_set_queue_pairs(n);
+    }
 }
 
 static int virtio_net_pre_load_queues(VirtIODevice *vdev, uint32_t n)
@@ -3028,6 +3053,17 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
 
     virtio_add_feature(&features, VIRTIO_NET_F_MAC);
 
+    if (peer_wait_incoming(n)) {
+        /*
+         * Excessive feature set is OK for early initialization when
+         * we wait for local incoming migration: actual guest-negotiated
+         * features will come with migration stream anyway. And we are sure
+         * that we support same host-features as source, because the backend
+         * is the same (the same TAP device, for example).
+         */
+        return features;
+    }
+
     if (!peer_has_vnet_hdr(n)) {
         virtio_clear_feature(&features, VIRTIO_NET_F_CSUM);
         virtio_clear_feature(&features, VIRTIO_NET_F_HOST_TSO4);
@@ -3106,6 +3142,32 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
     return features;
 }
 
+static int virtio_net_update_host_features(VirtIONet *n)
+{
+    Error *local_err = NULL;
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
+
+    peer_test_vnet_hdr(n);
+
+    vdev->host_features = virtio_net_get_features(vdev, vdev->host_features,
+                                                  &local_err);
+    if (local_err) {
+        error_report_err(local_err);
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+static int virtio_net_pre_load_device(void *opaque)
+{
+    /*
+     * Probably backend initialization was postponed to
+     * pre-incoming point. So, update information now.
+     */
+    return virtio_net_update_host_features(opaque);
+}
+
 static int virtio_net_post_load_device(void *opaque, int version_id)
 {
     VirtIONet *n = opaque;
@@ -3498,6 +3560,7 @@ static const VMStateDescription vmstate_virtio_net_device = {
     .name = "virtio-net-device",
     .version_id = VIRTIO_NET_VM_VERSION,
     .minimum_version_id = VIRTIO_NET_VM_VERSION,
+    .pre_load = virtio_net_pre_load_device,
     .post_load = virtio_net_post_load_device,
     .fields = (const VMStateField[]) {
         VMSTATE_UINT8_ARRAY(mac, VirtIONet, ETH_ALEN),
diff --git a/include/net/tap.h b/include/net/tap.h
index 6f34f13eae..c6f9c1aeb1 100644
--- a/include/net/tap.h
+++ b/include/net/tap.h
@@ -33,4 +33,6 @@ int tap_disable(NetClientState *nc);
 
 int tap_get_fd(NetClientState *nc);
 
+bool tap_wait_incoming(NetClientState *nc);
+
 #endif /* QEMU_NET_TAP_H */
diff --git a/net/tap.c b/net/tap.c
index 29568ce5d2..13efea7e4f 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -35,7 +35,9 @@
 #include "net/eth.h"
 #include "net/net.h"
 #include "clients.h"
+#include "migration/misc.h"
 #include "monitor/monitor.h"
+#include "system/runstate.h"
 #include "system/system.h"
 #include "qapi/error.h"
 #include "qemu/cutils.h"
@@ -85,8 +87,20 @@ typedef struct TAPState {
     int sndbuf;
     int vhostfd;
     uint32_t vhost_busyloop_timeout;
+
+    /* for postponed setup */
+    QTAILQ_ENTRY(TAPState) next;
+    bool vnet_hdr_required;
+    int vnet_hdr;
+    bool mq_required;
+    char *ifname;
 } TAPState;
 
+static QTAILQ_HEAD(, TAPState) postponed_taps =
+    QTAILQ_HEAD_INITIALIZER(postponed_taps);
+static NotifierWithReturn pre_incoming_notifier;
+
+static bool tap_postponed_init(TAPState *s, Error **errp);
 static bool net_tap_setup(TAPState *s, int fd, int vnet_hdr, Error **errp);
 
 static void launch_script(const char *setup_script, const char *ifname,
@@ -341,6 +355,8 @@ static void tap_cleanup(NetClientState *nc)
         close(s->vhostfd);
         s->vhostfd = -1;
     }
+
+    g_free(s->ifname);
 }
 
 static void tap_poll(NetClientState *nc, bool enable)
@@ -358,6 +374,25 @@ static bool tap_set_steering_ebpf(NetClientState *nc, int prog_fd)
     return tap_fd_set_steering_ebpf(s->fd, prog_fd) == 0;
 }
 
+static bool tap_check_peer_type(NetClientState *nc, ObjectClass *oc,
+                                Error **errp)
+{
+    TAPState *s = DO_UPCAST(TAPState, nc, nc);
+    const char *driver = object_class_get_name(oc);
+
+    if (!g_str_has_prefix(driver, "virtio-net-")) {
+        /*
+         * Only virtio-net support postponed TAP initialization, so
+         * for other drivers let's finalize initialization now.
+         */
+        if (tap_wait_incoming(nc)) {
+            return tap_postponed_init(s, errp);
+        }
+    }
+
+    return true;
+}
+
 int tap_get_fd(NetClientState *nc)
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
@@ -396,6 +431,7 @@ static NetClientInfo net_tap_info = {
     .set_vnet_be = tap_set_vnet_be,
     .set_steering_ebpf = tap_set_steering_ebpf,
     .get_vhost_net = tap_get_vhost_net,
+    .check_peer_type = tap_check_peer_type,
 };
 
 static TAPState *net_tap_new(NetClientState *peer, const char *model,
@@ -822,6 +858,124 @@ static int get_fds(char *str, char *fds[], int max)
     return i;
 }
 
+#define TAP_OPEN_IFNAME_SZ 128
+
+static bool tap_postponed_init(TAPState *s, Error **errp)
+{
+    char ifname[TAP_OPEN_IFNAME_SZ];
+    int vnet_hdr = s->vnet_hdr;
+    int fd;
+
+    pstrcpy(ifname, sizeof ifname, s->ifname);
+    fd = net_tap_open(&vnet_hdr, s->vnet_hdr_required, NULL,
+                      ifname, sizeof(ifname),
+                      s->mq_required, errp);
+    if (fd < 0) {
+        goto fail;
+    }
+
+    if (!net_tap_setup(s, fd, vnet_hdr, errp)) {
+        goto fail;
+    }
+
+    QTAILQ_REMOVE(&postponed_taps, s, next);
+    return true;
+
+fail:
+    QTAILQ_REMOVE(&postponed_taps, s, next);
+    qemu_del_net_client(&s->nc);
+    return false;
+}
+
+static int tap_pre_incoming(NotifierWithReturn *notifier,
+                            MigrationEvent *e,
+                            Error **errp)
+{
+    TAPState *s;
+    bool ok = true;
+
+    if (e->type != MIG_EVENT_PRE_INCOMING) {
+        return 0;
+    }
+
+    while (!QTAILQ_EMPTY(&postponed_taps)) {
+        s = QTAILQ_FIRST(&postponed_taps);
+        if (ok) {
+            ok = tap_postponed_init(s, errp);
+        } else {
+            QTAILQ_REMOVE(&postponed_taps, s, next);
+            qemu_del_net_client(&s->nc);
+        }
+    }
+
+    return ok ? 0 : -1;
+}
+
+static bool check_no_script(const char *script_arg)
+{
+    return script_arg &&
+        (script_arg[0] == '\0' || strcmp(script_arg, "no") == 0);
+}
+
+static bool tap_postpone_init(const NetdevTapOptions *tap,
+                              const char *name, NetClientState *peer,
+                              bool *postponed, Error **errp)
+{
+    int queues = tap->has_queues ? tap->queues : 1;
+
+    *postponed = false;
+
+    if (!runstate_check(RUN_STATE_INMIGRATE)) {
+        return true;
+    }
+
+    if (tap->fd || tap->fds || tap->helper || tap->vhostfds) {
+        return true;
+    }
+
+    if (!tap->ifname || tap->ifname[0] == '\0' ||
+        strstr(tap->ifname, "%d") != NULL) {
+        /*
+         * It's hard to postpone logic of parsing template or
+         * absent ifname
+         */
+        return true;
+    }
+
+    /*
+     * Supporting downscipt means understanding and realizing the logic of
+     * transfer of responsibility to call it in target QEMU process. Or in
+     * source QEMU process in case of migration failure. So for simplicity we
+     * don't support scripts together with fds migration.
+     */
+    if (!check_no_script(tap->script) || !check_no_script(tap->downscript)) {
+        return true;
+    }
+
+    for (int i = 0; i < queues; i++) {
+        TAPState *s = net_tap_new(peer, "tap", name, tap, NULL, errp);
+        if (!s) {
+            return false;
+        }
+
+        s->vnet_hdr_required = tap->has_vnet_hdr && tap->vnet_hdr;
+        s->vnet_hdr = tap->has_vnet_hdr ? tap->vnet_hdr : 1;
+        s->mq_required = queues > 1;
+        s->ifname = g_strdup(tap->ifname);
+        qemu_set_info_str(&s->nc, "ifname=%s,script=no,downscript=no",
+                          tap->ifname);
+
+        QTAILQ_INSERT_TAIL(&postponed_taps, s, next);
+    }
+
+    if (!pre_incoming_notifier.notify) {
+        migration_add_notifier(&pre_incoming_notifier, tap_pre_incoming);
+    }
+
+    *postponed = true;
+    return true;
+}
+
 int net_init_tap(const Netdev *netdev, const char *name,
                  NetClientState *peer, Error **errp)
 {
@@ -832,8 +986,9 @@ int net_init_tap(const Netdev *netdev, const char *name,
     const char *downscript;
     Error *err = NULL;
     const char *vhostfdname;
-    char ifname[128];
+    char ifname[TAP_OPEN_IFNAME_SZ];
     int ret = 0;
+    bool postponed = false;
 
     assert(netdev->type == NET_CLIENT_DRIVER_TAP);
     tap = &netdev->u.tap;
@@ -854,6 +1009,14 @@ int net_init_tap(const Netdev *netdev, const char *name,
         return -1;
     }
 
+    if (!tap_postpone_init(tap, name, peer, &postponed, errp)) {
+        return -1;
+    }
+
+    if (postponed) {
+        return 0;
+    }
+
     if (tap->fd) {
         if (tap->ifname || tap->script || tap->downscript ||
             tap->has_vnet_hdr || tap->helper || tap->has_queues ||
@@ -1089,3 +1252,10 @@ int tap_disable(NetClientState *nc)
         return ret;
     }
 }
+
+bool tap_wait_incoming(NetClientState *nc)
+{
+    TAPState *s = DO_UPCAST(TAPState, nc, nc);
+
+    return s->fd == -1;
+}
-- 
2.48.1

[PATCH v5 16/19] qapi: add interface for local TAP migration

[Vladimir Sementsov-Ogievskiy]

To migrate virtio-net TAP device backend (including open fds) locally,
user should simply set migration parameter

   fds = [virtio-net]

Why not simple boolean? To simplify migration to further versions,
when more devices will support fds migration.

Alternatively, we may add per-device option to disable fds-migration,
but still:

1. It's more comfortable to set same capabilities/parameters on both
source and target QEMU, than care about each device.

2. To not break the design, that machine-type + device options +
migration capabilites and parameters are fully define the resulting
migration stream. We'll break this if add in future more fds-passing
support in devices under same fds=true parameter.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 include/qapi/util.h | 17 +++++++++++++++++
 migration/options.c | 30 +++++++++++++++++++++++++++++
 migration/options.h |  2 ++
 qapi/migration.json | 46 ++++++++++++++++++++++++++++++++++++---------
 4 files changed, 86 insertions(+), 9 deletions(-)

diff --git a/include/qapi/util.h b/include/qapi/util.h
index 29bc4eb865..b953402416 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -69,4 +69,21 @@ int parse_qapi_name(const char *name, bool complete);
         _len;                                                       \
     })
 
+/*
+ * For any GenericList @list, return true if it contains specified
+ * element.
+ */
+#define QAPI_LIST_CONTAINS(list, el)                                \
+    ({                                                              \
+        bool _found = false;                                        \
+        typeof_strip_qual(list) _tail;                              \
+        for (_tail = list; _tail != NULL; _tail = _tail->next) {    \
+            if (_tail->value == el) {                               \
+                _found = true;                                      \
+                break;                                              \
+            }                                                       \
+        }                                                           \
+        _found;                                                     \
+    })
+
 #endif
diff --git a/migration/options.c b/migration/options.c
index 4e923a2e07..061a1b8eaf 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
+#include "qapi/util.h"
 #include "exec/target_page.h"
 #include "qapi/clone-visitor.h"
 #include "qapi/error.h"
@@ -262,6 +263,13 @@ bool migrate_mapped_ram(void)
     return s->capabilities[MIGRATION_CAPABILITY_MAPPED_RAM];
 }
 
+bool migrate_fds_virtio_net(void)
+{
+    MigrationState *s = migrate_get_current();
+
+    return QAPI_LIST_CONTAINS(s->parameters.fds, FDS_TARGET_VIRTIO_NET);
+}
+
 bool migrate_ignore_shared(void)
 {
     MigrationState *s = migrate_get_current();
@@ -960,6 +968,11 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
     params->has_direct_io = true;
     params->direct_io = s->parameters.direct_io;
 
+    if (s->parameters.has_fds) {
+        params->has_fds = true;
+        params->fds = QAPI_CLONE(FdsTargetList, s->parameters.fds);
+    }
+
     return params;
 }
 
@@ -1179,6 +1192,11 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
         return false;
     }
 
+    if (params->has_fds) {
+        error_setg(errp, "Not implemented");
+        return false;
+    }
+
     return true;
 }
 
@@ -1297,6 +1315,11 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
     if (params->has_direct_io) {
         dest->direct_io = params->direct_io;
     }
+
+    if (params->has_fds) {
+        dest->has_fds = true;
+        dest->fds = params->fds;
+    }
 }
 
 static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
@@ -1429,6 +1452,13 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
     if (params->has_direct_io) {
         s->parameters.direct_io = params->direct_io;
     }
+
+    if (params->has_fds) {
+        qapi_free_FdsTargetList(s->parameters.fds);
+
+        s->parameters.has_fds = true;
+        s->parameters.fds = QAPI_CLONE(FdsTargetList, params->fds);
+    }
 }
 
 void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
diff --git a/migration/options.h b/migration/options.h
index 82d839709e..a79472a235 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -87,6 +87,8 @@ const char *migrate_tls_hostname(void);
 uint64_t migrate_xbzrle_cache_size(void);
 ZeroPageDetection migrate_zero_page_detection(void);
 
+bool migrate_fds_virtio_net(void);
+
 /* parameters helpers */
 
 bool migrate_params_check(MigrationParameters *params, Error **errp);
diff --git a/qapi/migration.json b/qapi/migration.json
index 2387c21e9c..6ef9629c6d 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -747,6 +747,19 @@
       '*transform': 'BitmapMigrationBitmapAliasTransform'
   } }
 
+##
+# @FdsTarget:
+#
+# @virtio-net: Enable live backend migration for virtio-net.
+#     The only supported backend is TAP device. When enabled, TAP fds
+#     and all related state is passed to target QEMU through migration
+#     channel (which should be unix socket).
+#
+# Since: 10.2
+##
+{ 'enum': 'FdsTarget',
+  'data': [ 'virtio-net' ] }
+
 ##
 # @BitmapMigrationNodeAlias:
 #
@@ -924,10 +937,14 @@
 #     only has effect if the @mapped-ram capability is enabled.
 #     (Since 9.1)
 #
+# @fds: List of targets to enable live-backend migration for. This
+#     requires migration channel to be a unix socket (to pass fds
+#     through). (Since 10.2)
+#
 # Features:
 #
-# @unstable: Members @x-checkpoint-delay and
-#     @x-vcpu-dirty-limit-period are experimental.
+# @unstable: Members @x-checkpoint-delay,
+#     @x-vcpu-dirty-limit-period and @fds are experimental.
 #
 # Since: 2.4
 ##
@@ -950,7 +967,8 @@
            'vcpu-dirty-limit',
            'mode',
            'zero-page-detection',
-           'direct-io'] }
+           'direct-io',
+           'fds' ] }
 
 ##
 # @MigrateSetParameters:
@@ -1105,10 +1123,14 @@
 #     only has effect if the @mapped-ram capability is enabled.
 #     (Since 9.1)
 #
+# @fds: List of targets to enable live-backend migration for. This
+#     requires migration channel to be a unix socket (to pass fds
+#     through). (Since 10.2)
+#
 # Features:
 #
-# @unstable: Members @x-checkpoint-delay and
-#     @x-vcpu-dirty-limit-period are experimental.
+# @unstable: Members @x-checkpoint-delay,
+#     @x-vcpu-dirty-limit-period and @fds are experimental.
 #
 # TODO: either fuse back into `MigrationParameters`, or make
 #     `MigrationParameters` members mandatory
@@ -1146,7 +1168,8 @@
             '*vcpu-dirty-limit': 'uint64',
             '*mode': 'MigMode',
             '*zero-page-detection': 'ZeroPageDetection',
-            '*direct-io': 'bool' } }
+            '*direct-io': 'bool',
+            '*fds': { 'type': [ 'FdsTarget' ], 'features': [ 'unstable' ] } } }
 
 ##
 # @migrate-set-parameters:
@@ -1315,10 +1338,14 @@
 #     only has effect if the @mapped-ram capability is enabled.
 #     (Since 9.1)
 #
+# @fds: List of targets to enable live-backend migration for. This
+#     requires migration channel to be a unix socket (to pass fds
+#     through). (Since 10.2)
+#
 # Features:
 #
-# @unstable: Members @x-checkpoint-delay and
-#     @x-vcpu-dirty-limit-period are experimental.
+# @unstable: Members @x-checkpoint-delay,
+#     @x-vcpu-dirty-limit-period and @fds are experimental.
 #
 # Since: 2.4
 ##
@@ -1353,7 +1380,8 @@
             '*vcpu-dirty-limit': 'uint64',
             '*mode': 'MigMode',
             '*zero-page-detection': 'ZeroPageDetection',
-            '*direct-io': 'bool' } }
+            '*direct-io': 'bool',
+            '*fds': { 'type': [ 'FdsTarget' ], 'features': [ 'unstable' ] } } }
 
 ##
 # @query-migrate-parameters:
-- 
2.48.1

[PATCH v5 17/19] virtio-net: support fds migration of TAP backend

[Vladimir Sementsov-Ogievskiy]

Finally implement the new migration option fds = [virtio-net].

With this enabled (both on source and target) of-course, and with
unix-socket used as migration-channel, we do "migrate" the virtio-net
backend - TAP devices, with all its fds.

This way management tool should not care about creating new TAP, and
handling switching to it. Migration downtime become shorter.

How it works:

1. For incoming migration, we postpone TAP initialization up to
   pre-incoming point.

2. At pre-incoming point we see that virtio-net target for fds
   migration is set, so we postpone TAP initialization up to
   post-load

3. During virtio-load, we get TAP state (and fds) as part of
   virtio-net state

4. In post-load we finalize TAP initialization

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 hw/net/virtio-net.c | 73 +++++++++++++++++++++++++++++++++++++++++++++
 include/net/tap.h   |  3 ++
 migration/options.c |  5 ----
 net/tap.c           | 54 ++++++++++++++++++++++++++++++++-
 4 files changed, 129 insertions(+), 6 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 70f688fc3a..2817c0f198 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -26,6 +26,7 @@
 #include "qemu/option.h"
 #include "qemu/option_int.h"
 #include "qemu/config-file.h"
+#include "qemu/typedefs.h"
 #include "qobject/qdict.h"
 #include "hw/virtio/virtio-net.h"
 #include "net/vhost_net.h"
@@ -38,6 +39,8 @@
 #include "qapi/qapi-events-migration.h"
 #include "hw/virtio/virtio-access.h"
 #include "migration/misc.h"
+#include "migration/migration.h"
+#include "migration/options.h"
 #include "standard-headers/linux/ethtool.h"
 #include "system/system.h"
 #include "system/replay.h"
@@ -3147,6 +3150,11 @@ static int virtio_net_update_host_features(VirtIONet *n)
     Error *local_err = NULL;
     VirtIODevice *vdev = VIRTIO_DEVICE(n);
 
+    if (peer_wait_incoming(n)) {
+        /* It's too early for updating options. */
+        return 0;
+    }
+
     peer_test_vnet_hdr(n);
 
     vdev->host_features = virtio_net_get_features(vdev, vdev->host_features,
@@ -3287,6 +3295,9 @@ struct VirtIONetMigTmp {
     uint16_t        curr_queue_pairs_1;
     uint8_t         has_ufo;
     uint32_t        has_vnet_hdr;
+
+    NetClientState *ncs;
+    uint32_t max_queue_pairs;
 };
 
 /* The 2nd and subsequent tx_waiting flags are loaded later than
@@ -3556,6 +3567,65 @@ static const VMStateDescription vhost_user_net_backend_state = {
     }
 };
 
+static bool virtio_net_is_tap_fds_mig(void *opaque, int version_id)
+{
+    VirtIONet *n = opaque;
+    NetClientState *nc;
+
+    nc = qemu_get_queue(n->nic);
+
+    return migrate_fds_virtio_net() && nc->peer &&
+        nc->peer->info->type == NET_CLIENT_DRIVER_TAP;
+}
+
+static int virtio_net_nic_pre_save(void *opaque)
+{
+    struct VirtIONetMigTmp *tmp = opaque;
+
+    tmp->ncs = tmp->parent->nic->ncs;
+    tmp->max_queue_pairs = tmp->parent->max_queue_pairs;
+
+    return 0;
+}
+
+static int virtio_net_nic_pre_load(void *opaque)
+{
+    /* Reuse the pointer setup from save */
+    virtio_net_nic_pre_save(opaque);
+
+    return 0;
+}
+
+static int virtio_net_nic_post_load(void *opaque, int version_id)
+{
+    struct VirtIONetMigTmp *tmp = opaque;
+
+    return virtio_net_update_host_features(tmp->parent);
+}
+
+static const VMStateDescription vmstate_virtio_net_nic_nc = {
+    .name = "virtio-net-nic-nc",
+    .fields = (const VMStateField[]) {
+        VMSTATE_STRUCT_POINTER(peer, NetClientState, vmstate_tap,
+                               NetClientState),
+        VMSTATE_END_OF_LIST()
+   },
+};
+
+static const VMStateDescription vmstate_virtio_net_nic = {
+    .name      = "virtio-net-nic",
+    .pre_load  = virtio_net_nic_pre_load,
+    .pre_save  = virtio_net_nic_pre_save,
+    .post_load  = virtio_net_nic_post_load,
+    .fields    = (const VMStateField[]) {
+        VMSTATE_STRUCT_VARRAY_POINTER_UINT32(ncs, struct VirtIONetMigTmp,
+                                             max_queue_pairs,
+                                             vmstate_virtio_net_nic_nc,
+                                             struct NetClientState),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
 static const VMStateDescription vmstate_virtio_net_device = {
     .name = "virtio-net-device",
     .version_id = VIRTIO_NET_VM_VERSION,
@@ -3588,6 +3658,9 @@ static const VMStateDescription vmstate_virtio_net_device = {
          * but based on the uint.
          */
         VMSTATE_BUFFER_POINTER_UNSAFE(vlans, VirtIONet, 0, MAX_VLAN >> 3),
+        VMSTATE_WITH_TMP_TEST(VirtIONet, virtio_net_is_tap_fds_mig,
+                              struct VirtIONetMigTmp,
+                              vmstate_virtio_net_nic),
         VMSTATE_WITH_TMP(VirtIONet, struct VirtIONetMigTmp,
                          vmstate_virtio_net_has_vnet),
         VMSTATE_UINT8(mac_table.multi_overflow, VirtIONet),
diff --git a/include/net/tap.h b/include/net/tap.h
index c6f9c1aeb1..0be083f8da 100644
--- a/include/net/tap.h
+++ b/include/net/tap.h
@@ -26,6 +26,7 @@
 #ifndef QEMU_NET_TAP_H
 #define QEMU_NET_TAP_H
 
+#include "qemu/typedefs.h"
 #include "standard-headers/linux/virtio_net.h"
 
 int tap_enable(NetClientState *nc);
@@ -35,4 +36,6 @@ int tap_get_fd(NetClientState *nc);
 
 bool tap_wait_incoming(NetClientState *nc);
 
+extern const VMStateDescription vmstate_tap;
+
 #endif /* QEMU_NET_TAP_H */
diff --git a/migration/options.c b/migration/options.c
index 061a1b8eaf..c6eeadaab5 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1192,11 +1192,6 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
         return false;
     }
 
-    if (params->has_fds) {
-        error_setg(errp, "Not implemented");
-        return false;
-    }
-
     return true;
 }
 
diff --git a/net/tap.c b/net/tap.c
index 13efea7e4f..dcbc6466e5 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -36,6 +36,7 @@
 #include "net/net.h"
 #include "clients.h"
 #include "migration/misc.h"
+#include "migration/options.h"
 #include "monitor/monitor.h"
 #include "system/runstate.h"
 #include "system/system.h"
@@ -94,6 +95,7 @@ typedef struct TAPState {
     int vnet_hdr;
     bool mq_required;
     char *ifname;
+    bool attached_to_virtio_net;
 } TAPState;
 
 static QTAILQ_HEAD(, TAPState) postponed_taps =
@@ -390,6 +392,8 @@ static bool tap_check_peer_type(NetClientState *nc, ObjectClass *oc,
         }
     }
 
+    s->attached_to_virtio_net = true;
+
     return true;
 }
 
@@ -801,7 +805,7 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
 
 static bool net_tap_setup(TAPState *s, int fd, int vnet_hdr, Error **errp)
 {
-    if (!net_tap_set_fd(s, fd, vnet_hdr, errp)) {
+    if (fd > -1 && !net_tap_set_fd(s, fd, vnet_hdr, errp)) {
         return false;
     }
 
@@ -893,6 +897,7 @@ static int tap_pre_incoming(NotifierWithReturn *notifier,
 {
     TAPState *s;
     bool ok = true;
+    bool mig_fds = migrate_fds_virtio_net();
 
     if (e->type != MIG_EVENT_PRE_INCOMING) {
         return 0;
@@ -901,6 +906,11 @@ static int tap_pre_incoming(NotifierWithReturn *notifier,
     while (!QTAILQ_EMPTY(&postponed_taps)) {
         s = QTAILQ_FIRST(&postponed_taps);
         if (ok) {
+            if (mig_fds && s->attached_to_virtio_net) {
+                /* We'll get fds from incoming migration */
+                QTAILQ_REMOVE(&postponed_taps, s, next);
+                continue;
+            }
             ok = tap_postponed_init(s, errp);
         } else {
             QTAILQ_REMOVE(&postponed_taps, s, next);
@@ -1253,6 +1263,48 @@ int tap_disable(NetClientState *nc)
     }
 }
 
+static int tap_pre_load(void *opaque)
+{
+    TAPState *s = opaque;
+
+    if (s->fd != -1) {
+        error_report(
+            "TAP is already initialized and cannot receive incoming fd");
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+static int tap_post_load(void *opaque, int version_id)
+{
+    TAPState *s = opaque;
+    Error *local_err = NULL;
+
+    if (!net_tap_setup(s, -1, -1, &local_err)) {
+        error_report_err(local_err);
+        qemu_del_net_client(&s->nc);
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+const VMStateDescription vmstate_tap = {
+    .name = "net-tap",
+    .pre_load = tap_pre_load,
+    .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()
+    }
+};
+
 bool tap_wait_incoming(NetClientState *nc)
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
-- 
2.48.1

[PATCH v5 18/19] tests/functional: add skipUnlessPasswordlessSudo() decorator

[Vladimir Sementsov-Ogievskiy]

To be used in the next commit: that would be a test for TAP
networking, and it will need to setup TAP device.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/functional/qemu_test/decorators.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tests/functional/qemu_test/decorators.py b/tests/functional/qemu_test/decorators.py
index c0d1567b14..4b332804ef 100644
--- a/tests/functional/qemu_test/decorators.py
+++ b/tests/functional/qemu_test/decorators.py
@@ -6,6 +6,7 @@
 import os
 import platform
 import resource
+import subprocess
 from unittest import skipIf, skipUnless
 
 from .cmd import which
@@ -149,3 +150,18 @@ def skipLockedMemoryTest(locked_memory):
         ulimit_memory == resource.RLIM_INFINITY or ulimit_memory >= locked_memory * 1024,
         f'Test required {locked_memory} kB of available locked memory',
     )
+
+'''
+Decorator to skip execution of a test if passwordless
+sudo command is not available.
+'''
+def skipUnlessPasswordlessSudo():
+    proc = subprocess.run(["sudo", "-n", "/bin/true"],
+                          stdin=subprocess.PIPE,
+                          stdout=subprocess.PIPE,
+                          stderr=subprocess.STDOUT,
+                          universal_newlines=True,
+                          check=False)
+
+    return skipUnless(proc.returncode == 0,
+                      f'requires password-less sudo access: {proc.stdout}')
-- 
2.48.1

[PATCH v5 19/19] tests/functional: add test_x86_64_tap_fd_migration

[Vladimir Sementsov-Ogievskiy]

Add test for a new feature of local TAP migration with fd passing
through unix socket.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 .../test_x86_64_tap_fd_migration.py           | 343 ++++++++++++++++++
 1 file changed, 343 insertions(+)
 create mode 100644 tests/functional/test_x86_64_tap_fd_migration.py

diff --git a/tests/functional/test_x86_64_tap_fd_migration.py b/tests/functional/test_x86_64_tap_fd_migration.py
new file mode 100644
index 0000000000..ac80830eea
--- /dev/null
+++ b/tests/functional/test_x86_64_tap_fd_migration.py
@@ -0,0 +1,343 @@
+#!/usr/bin/env python3
+#
+# Functional test that tests TAP local migration
+# with fd passing
+#
+# Copyright (c) Yandex
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+import os
+import time
+import subprocess
+from subprocess import run
+import signal
+from typing import Tuple
+
+from qemu_test import (
+    LinuxKernelTest,
+    Asset,
+    exec_command_and_wait_for_pattern,
+)
+from qemu_test.decorators import skipUnlessPasswordlessSudo
+
+GUEST_IP = "10.0.1.2"
+GUEST_IP_MASK = f"{GUEST_IP}/24"
+GUEST_MAC = "d6:0d:75:f8:0f:b7"
+HOST_IP = "10.0.1.1"
+HOST_IP_MASK = f"{HOST_IP}/24"
+TAP_ID = "tap0"
+TAP_MAC = "e6:1d:44:b5:03:5d"
+
+
+def del_tap() -> None:
+    run(
+        ["sudo", "ip", "tuntap", "del", TAP_ID, "mode", "tap", "multi_queue"],
+        check=True,
+    )
+
+
+def init_tap() -> None:
+    run(
+        ["sudo", "ip", "tuntap", "add", "dev", TAP_ID, "mode", "tap", "multi_queue"],
+        check=True,
+    )
+    run(["sudo", "ip", "link", "set", "dev", TAP_ID, "address", TAP_MAC], check=True)
+    run(["sudo", "ip", "addr", "add", HOST_IP_MASK, "dev", TAP_ID], check=True)
+    run(["sudo", "ip", "link", "set", TAP_ID, "up"], check=True)
+
+
+def parse_ping_line(line: str) -> float:
+    # suspect lines like
+    # [1748524876.590509] 64 bytes from 94.245.155.3 \
+    #      (94.245.155.3): icmp_seq=1 ttl=250 time=101 ms
+    spl = line.split()
+    return float(spl[0][1:-1])
+
+
+def parse_ping_output(out) -> Tuple[bool, float, float]:
+    lines = [x for x in out.split("\n") if x.startswith("[")]
+
+    try:
+        first_no_ans = next(
+            (ind for ind in range(len(lines)) if lines[ind][20:26] == "no ans")
+        )
+    except StopIteration:
+        return False, parse_ping_line(lines[0]), parse_ping_line(lines[-1])
+
+    last_no_ans = next(
+        (ind for ind in range(len(lines) - 1, -1, -1) if lines[ind][20:26] == "no ans")
+    )
+
+    return (
+        True,
+        parse_ping_line(lines[first_no_ans]),
+        parse_ping_line(lines[last_no_ans]),
+    )
+
+
+def wait_migration_finish(source_vm, target_vm):
+    migr_events = (
+        ("MIGRATION", {"data": {"status": "completed"}}),
+        ("MIGRATION", {"data": {"status": "failed"}}),
+    )
+
+    source_e = source_vm.events_wait(migr_events)["data"]
+    target_e = target_vm.events_wait(migr_events)["data"]
+
+    source_s = source_vm.cmd("query-status")["status"]
+    target_s = target_vm.cmd("query-status")["status"]
+
+    assert (
+        source_e["status"] == "completed"
+        and target_e["status"] == "completed"
+        and source_s == "postmigrate"
+        and target_s == "paused"
+    ), f"""Migration failed:
+    SRC status: {source_s}
+    SRC event: {source_e}
+    TGT status: {target_s}
+    TGT event:{target_e}"""
+
+
+@skipUnlessPasswordlessSudo()
+class VhostUserBlkFdMigration(LinuxKernelTest):
+
+    ASSET_KERNEL = Asset(
+        (
+            "https://archives.fedoraproject.org/pub/archive/fedora/linux/releases"
+            "/31/Server/x86_64/os/images/pxeboot/vmlinuz"
+        ),
+        "d4738d03dbbe083ca610d0821d0a8f1488bebbdccef54ce33e3adb35fda00129",
+    )
+
+    ASSET_INITRD = Asset(
+        (
+            "https://archives.fedoraproject.org/pub/archive/fedora/linux/releases"
+            "/31/Server/x86_64/os/images/pxeboot/initrd.img"
+        ),
+        "277cd6c7adf77c7e63d73bbb2cded8ef9e2d3a2f100000e92ff1f8396513cd8b",
+    )
+
+    ASSET_ALPINE_ISO = Asset(
+        (
+            "https://dl-cdn.alpinelinux.org/"
+            "alpine/v3.22/releases/x86_64/alpine-standard-3.22.1-x86_64.iso"
+        ),
+        "96d1b44ea1b8a5a884f193526d92edb4676054e9fa903ad2f016441a0fe13089",
+    )
+
+    def setUp(self):
+        super().setUp()
+
+        init_tap()
+
+        self.outer_ping_proc = None
+
+    def tearDown(self):
+        try:
+            del_tap()
+
+            if self.outer_ping_proc:
+                self.stop_outer_ping()
+        finally:
+            super().tearDown()
+
+    def start_outer_ping(self) -> None:
+        assert self.outer_ping_proc is None
+        self.outer_ping_log = self.scratch_file("ping.log")
+        with open(self.outer_ping_log, "w") as f:
+            self.outer_ping_proc = subprocess.Popen(
+                ["ping", "-i", "0", "-O", "-D", GUEST_IP],
+                text=True,
+                stdout=f,
+            )
+
+    def stop_outer_ping(self) -> str:
+        assert self.outer_ping_proc
+        self.outer_ping_proc.send_signal(signal.SIGINT)
+
+        self.outer_ping_proc.communicate(timeout=5)
+        self.outer_ping_proc = None
+
+        with open(self.outer_ping_log) as f:
+            return f.read()
+
+    def stop_ping_and_check(self, stop_time, resume_time):
+        ping_res = self.stop_outer_ping()
+
+        discon, a, b = parse_ping_output(ping_res)
+
+        if not discon:
+            text = f"STOP: {stop_time}, RESUME: {resume_time}," f"PING: {a} - {b}"
+            if a > stop_time or b < resume_time:
+                self.fail(f"PING failed: {text}")
+            self.log.info(f"PING: no packets lost: {text}")
+            return
+
+        text = (
+            f"STOP: {stop_time}, RESUME: {resume_time}," f"PING: disconnect: {a} - {b}"
+        )
+        self.log.info(text)
+        eps = 0.01
+        if a < stop_time - eps or b > resume_time + eps:
+            self.fail(text)
+
+    def one_ping_from_guest(self, vm) -> None:
+        exec_command_and_wait_for_pattern(
+            self,
+            f"ping -c 1 -W 1 {HOST_IP}",
+            "1 packets transmitted, 1 packets received",
+            "1 packets transmitted, 0 packets received",
+            vm=vm,
+        )
+        self.wait_for_console_pattern("# ", vm=vm)
+
+    def one_ping_from_host(self) -> None:
+        run(["ping", "-c", "1", "-W", "1", GUEST_IP])
+
+    def setup_shared_memory(self):
+        shm_path = f"/dev/shm/qemu_test_{os.getpid()}"
+
+        try:
+            with open(shm_path, "wb") as f:
+                f.write(b"\0" * (1024 * 1024 * 1024))  # 1GB
+        except Exception as e:
+            self.fail(f"Failed to create shared memory file: {e}")
+
+        return shm_path
+
+    def prepare_and_launch_vm(self, shm_path, vhost, incoming=False, vm=None):
+        if not vm:
+            vm = self.vm
+
+        vm.set_console()
+        vm.add_args("-accel", "kvm")
+        vm.add_args("-device", "pcie-pci-bridge,id=pci.1,bus=pcie.0")
+        vm.add_args("-m", "1G")
+
+        vm.add_args(
+            "-object",
+            f"memory-backend-file,id=ram0,size=1G,mem-path={shm_path},share=on",
+        )
+        vm.add_args("-machine", "memory-backend=ram0")
+
+        vm.add_args(
+            "-drive", f"file={self.ASSET_ALPINE_ISO.fetch()},media=cdrom,format=raw"
+        )
+
+        vm.add_args("-S")
+
+        if incoming:
+            vm.add_args("-incoming", "defer")
+
+        vm_s = "target" if incoming else "source"
+        self.log.info(f"Launching {vm_s} VM")
+        vm.launch()
+
+        self.set_migration_capabilities(vm)
+        self.add_virtio_net(vm, vhost)
+
+    def add_virtio_net(self, vm, vhost: bool):
+        netdev_params = {
+            "id": "netdev.1",
+            "vhost": vhost,
+            "type": "tap",
+            "ifname": "tap0",
+            "script": "no",
+            "downscript": "no",
+            "queues": 4,
+            "vnet_hdr": True,
+        }
+
+        vm.cmd("netdev_add", netdev_params)
+
+        vm.cmd(
+            "device_add",
+            driver="virtio-net-pci",
+            romfile="",
+            id="vnet.1",
+            netdev="netdev.1",
+            mq=True,
+            vectors=18,
+            bus="pci.1",
+            mac=GUEST_MAC,
+            disable_legacy="off",
+        )
+
+    def set_migration_capabilities(self, vm):
+        capabilities = [
+            {"capability": "events", "state": True},
+            {"capability": "x-ignore-shared", "state": True},
+        ]
+        vm.cmd("migrate-set-capabilities", {"capabilities": capabilities})
+        vm.cmd("migrate-set-parameters", {"fds": ["virtio-net"]})
+
+    def setup_guest_network(self) -> None:
+        exec_command_and_wait_for_pattern(self, "ip addr", "# ")
+        exec_command_and_wait_for_pattern(
+            self,
+            f"ip addr add {GUEST_IP_MASK} dev eth0 && ip link set eth0 up && echo OK",
+            "OK",
+        )
+        self.wait_for_console_pattern("# ")
+
+    def do_test_tap_fd_migration(self, vhost):
+        self.require_accelerator("kvm")
+        self.set_machine("q35")
+
+        socket_dir = self.socket_dir()
+        migration_socket = os.path.join(socket_dir.name, "migration.sock")
+
+        shm_path = self.setup_shared_memory()
+
+        self.prepare_and_launch_vm(shm_path, vhost)
+        self.vm.cmd("cont")
+        self.wait_for_console_pattern("login:")
+        exec_command_and_wait_for_pattern(self, "root", "# ")
+
+        self.setup_guest_network()
+
+        self.one_ping_from_guest(self.vm)
+        self.one_ping_from_host()
+        self.start_outer_ping()
+
+        # Get some successful pings before migration
+        time.sleep(0.5)
+
+        target_vm = self.get_vm(name="target")
+        self.prepare_and_launch_vm(shm_path, vhost, incoming=True, vm=target_vm)
+
+        target_vm.cmd("migrate-incoming", {"uri": f"unix:{migration_socket}"})
+
+        self.log.info("Starting migration")
+        freeze_start = time.time()
+        self.vm.cmd("migrate", {"uri": f"unix:{migration_socket}"})
+
+        self.log.info("Waiting for migration completion")
+        wait_migration_finish(self.vm, target_vm)
+
+        target_vm.cmd("cont")
+        freeze_end = time.time()
+
+        self.vm.shutdown()
+
+        self.log.info("Verifying PING on target VM after migration")
+        self.one_ping_from_guest(target_vm)
+        self.one_ping_from_host()
+
+        # And a bit more pings after source shutdown
+        time.sleep(0.3)
+        self.stop_ping_and_check(freeze_start, freeze_end)
+
+        target_vm.shutdown()
+
+    def test_tap_fd_migration(self):
+        self.do_test_tap_fd_migration(False)
+
+    def test_tap_fd_migration_vhost(self):
+        self.do_test_tap_fd_migration(True)
+
+
+if __name__ == "__main__":
+    LinuxKernelTest.main()
-- 
2.48.1

Re: [PATCH v5 18/19] tests/functional: add skipUnlessPasswordlessSudo() decorator

[Thomas Huth]

On 19/09/2025 11.55, Vladimir Sementsov-Ogievskiy wrote:
> To be used in the next commit: that would be a test for TAP
> networking, and it will need to setup TAP device.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/functional/qemu_test/decorators.py | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/tests/functional/qemu_test/decorators.py b/tests/functional/qemu_test/decorators.py
> index c0d1567b14..4b332804ef 100644
> --- a/tests/functional/qemu_test/decorators.py
> +++ b/tests/functional/qemu_test/decorators.py
> @@ -6,6 +6,7 @@
>   import os
>   import platform
>   import resource
> +import subprocess
>   from unittest import skipIf, skipUnless
>   
>   from .cmd import which
> @@ -149,3 +150,18 @@ def skipLockedMemoryTest(locked_memory):
>           ulimit_memory == resource.RLIM_INFINITY or ulimit_memory >= locked_memory * 1024,
>           f'Test required {locked_memory} kB of available locked memory',
>       )
> +
> +'''
> +Decorator to skip execution of a test if passwordless
> +sudo command is not available.
> +'''
> +def skipUnlessPasswordlessSudo():
> +    proc = subprocess.run(["sudo", "-n", "/bin/true"],
> +                          stdin=subprocess.PIPE,
> +                          stdout=subprocess.PIPE,
> +                          stderr=subprocess.STDOUT,
> +                          universal_newlines=True,
> +                          check=False)
> +
> +    return skipUnless(proc.returncode == 0,
> +                      f'requires password-less sudo access: {proc.stdout}')

I'd maybe rather just call it "skipWithoutSudo" ... but anyway:

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v5 16/19] qapi: add interface for local TAP migration

[Fabiano Rosas]

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

Hi Vladimir, as usual with "qapi:" patches, some comments about
language:

> To migrate virtio-net TAP device backend (including open fds) locally,
> user should simply set migration parameter
>
>    fds = [virtio-net]
>
> Why not simple boolean? To simplify migration to further versions,
> when more devices will support fds migration.
>
> Alternatively, we may add per-device option to disable fds-migration,
> but still:
>
> 1. It's more comfortable to set same capabilities/parameters on both
> source and target QEMU, than care about each device.
>
> 2. To not break the design, that machine-type + device options +
> migration capabilites and parameters are fully define the resulting
> migration stream. We'll break this if add in future more fds-passing
> support in devices under same fds=true parameter.
>

These arguments look convincing to me.

> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  include/qapi/util.h | 17 +++++++++++++++++
>  migration/options.c | 30 +++++++++++++++++++++++++++++
>  migration/options.h |  2 ++
>  qapi/migration.json | 46 ++++++++++++++++++++++++++++++++++++---------
>  4 files changed, 86 insertions(+), 9 deletions(-)
>
> diff --git a/include/qapi/util.h b/include/qapi/util.h
> index 29bc4eb865..b953402416 100644
> --- a/include/qapi/util.h
> +++ b/include/qapi/util.h
> @@ -69,4 +69,21 @@ int parse_qapi_name(const char *name, bool complete);
>          _len;                                                       \
>      })
>  
> +/*
> + * For any GenericList @list, return true if it contains specified
> + * element.
> + */
> +#define QAPI_LIST_CONTAINS(list, el)                                \
> +    ({                                                              \
> +        bool _found = false;                                        \
> +        typeof_strip_qual(list) _tail;                              \
> +        for (_tail = list; _tail != NULL; _tail = _tail->next) {    \
> +            if (_tail->value == el) {                               \
> +                _found = true;                                      \
> +                break;                                              \
> +            }                                                       \
> +        }                                                           \
> +        _found;                                                     \
> +    })
> +
>  #endif
> diff --git a/migration/options.c b/migration/options.c
> index 4e923a2e07..061a1b8eaf 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -13,6 +13,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "qemu/error-report.h"
> +#include "qapi/util.h"
>  #include "exec/target_page.h"
>  #include "qapi/clone-visitor.h"
>  #include "qapi/error.h"
> @@ -262,6 +263,13 @@ bool migrate_mapped_ram(void)
>      return s->capabilities[MIGRATION_CAPABILITY_MAPPED_RAM];
>  }
>  
> +bool migrate_fds_virtio_net(void)
> +{
> +    MigrationState *s = migrate_get_current();
> +
> +    return QAPI_LIST_CONTAINS(s->parameters.fds, FDS_TARGET_VIRTIO_NET);
> +}
> +
>  bool migrate_ignore_shared(void)
>  {
>      MigrationState *s = migrate_get_current();
> @@ -960,6 +968,11 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
>      params->has_direct_io = true;
>      params->direct_io = s->parameters.direct_io;
>  
> +    if (s->parameters.has_fds) {
> +        params->has_fds = true;
> +        params->fds = QAPI_CLONE(FdsTargetList, s->parameters.fds);
> +    }
> +
>      return params;
>  }
>  
> @@ -1179,6 +1192,11 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
>          return false;
>      }
>  
> +    if (params->has_fds) {
> +        error_setg(errp, "Not implemented");
> +        return false;
> +    }
> +
>      return true;
>  }
>  
> @@ -1297,6 +1315,11 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
>      if (params->has_direct_io) {
>          dest->direct_io = params->direct_io;
>      }
> +
> +    if (params->has_fds) {
> +        dest->has_fds = true;

I think what you want is to set this in
migrate_params_init(). block_bitmap_mapping is a bit of an outlier in
that it takes an empty *input* as valid.

> +        dest->fds = params->fds;
> +    }
>  }
>  
>  static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
> @@ -1429,6 +1452,13 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
>      if (params->has_direct_io) {
>          s->parameters.direct_io = params->direct_io;
>      }
> +
> +    if (params->has_fds) {
> +        qapi_free_FdsTargetList(s->parameters.fds);
> +
> +        s->parameters.has_fds = true;
> +        s->parameters.fds = QAPI_CLONE(FdsTargetList, params->fds);

Same here, has_fds should always be true in s->parameters.

> +    }
>  }
>  
>  void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
> diff --git a/migration/options.h b/migration/options.h
> index 82d839709e..a79472a235 100644
> --- a/migration/options.h
> +++ b/migration/options.h
> @@ -87,6 +87,8 @@ const char *migrate_tls_hostname(void);
>  uint64_t migrate_xbzrle_cache_size(void);
>  ZeroPageDetection migrate_zero_page_detection(void);
>  
> +bool migrate_fds_virtio_net(void);
> +
>  /* parameters helpers */
>  
>  bool migrate_params_check(MigrationParameters *params, Error **errp);
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 2387c21e9c..6ef9629c6d 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -747,6 +747,19 @@
>        '*transform': 'BitmapMigrationBitmapAliasTransform'
>    } }
>  
> +##
> +# @FdsTarget:
> +#
> +# @virtio-net: Enable live backend migration for virtio-net.

So you're assuming normal migration is "non-live backend migration"
because the backend is stopped and started again on the destination. I
think it makes sense.

> +#     The only supported backend is TAP device. When enabled, TAP fds
> +#     and all related state is passed to target QEMU through migration
> +#     channel (which should be unix socket).
> +#
> +# Since: 10.2
> +##
> +{ 'enum': 'FdsTarget',
> +  'data': [ 'virtio-net' ] }
> +
>  ##
>  # @BitmapMigrationNodeAlias:
>  #
> @@ -924,10 +937,14 @@
>  #     only has effect if the @mapped-ram capability is enabled.
>  #     (Since 9.1)
>  #
> +# @fds: List of targets to enable live-backend migration for. This
> +#     requires migration channel to be a unix socket (to pass fds
> +#     through). (Since 10.2)

I think I prefer live-backend as written here rather than the non
hyphenated version above. This clears up the ambiguity and can be used
in variable names, as a name for the feature and ... _thinks really
hard_ ... as the parameter name! (maybe)

On that note, fds is just too broad, I'm sure you know that, but to be
specific, we already have fd migration, multifd, fdset (as argument of
file: migration), etc. Talking just about the user interface here, of
course.

Also: "@fds: List of targets..." is super confusing.

And although "to pass fds through" is helping clarify the meaning of
@fds, it exposes implementation details on the QAPI, I don't think it's
relevant in the parameter description.

> +#
>  # Features:
>  #
> -# @unstable: Members @x-checkpoint-delay and
> -#     @x-vcpu-dirty-limit-period are experimental.
> +# @unstable: Members @x-checkpoint-delay,
> +#     @x-vcpu-dirty-limit-period and @fds are experimental.
>  #
>  # Since: 2.4
>  ##
> @@ -950,7 +967,8 @@
>             'vcpu-dirty-limit',
>             'mode',
>             'zero-page-detection',
> -           'direct-io'] }
> +           'direct-io',
> +           'fds' ] }
>  
>  ##
>  # @MigrateSetParameters:
> @@ -1105,10 +1123,14 @@
>  #     only has effect if the @mapped-ram capability is enabled.
>  #     (Since 9.1)
>  #
> +# @fds: List of targets to enable live-backend migration for. This
> +#     requires migration channel to be a unix socket (to pass fds
> +#     through). (Since 10.2)
> +#
>  # Features:
>  #
> -# @unstable: Members @x-checkpoint-delay and
> -#     @x-vcpu-dirty-limit-period are experimental.
> +# @unstable: Members @x-checkpoint-delay,
> +#     @x-vcpu-dirty-limit-period and @fds are experimental.
>  #
>  # TODO: either fuse back into `MigrationParameters`, or make
>  #     `MigrationParameters` members mandatory
> @@ -1146,7 +1168,8 @@
>              '*vcpu-dirty-limit': 'uint64',
>              '*mode': 'MigMode',
>              '*zero-page-detection': 'ZeroPageDetection',
> -            '*direct-io': 'bool' } }
> +            '*direct-io': 'bool',
> +            '*fds': { 'type': [ 'FdsTarget' ], 'features': [ 'unstable' ] } } }
>  
>  ##
>  # @migrate-set-parameters:
> @@ -1315,10 +1338,14 @@
>  #     only has effect if the @mapped-ram capability is enabled.
>  #     (Since 9.1)
>  #
> +# @fds: List of targets to enable live-backend migration for. This
> +#     requires migration channel to be a unix socket (to pass fds
> +#     through). (Since 10.2)
> +#
>  # Features:
>  #
> -# @unstable: Members @x-checkpoint-delay and
> -#     @x-vcpu-dirty-limit-period are experimental.
> +# @unstable: Members @x-checkpoint-delay,
> +#     @x-vcpu-dirty-limit-period and @fds are experimental.
>  #
>  # Since: 2.4
>  ##
> @@ -1353,7 +1380,8 @@
>              '*vcpu-dirty-limit': 'uint64',
>              '*mode': 'MigMode',
>              '*zero-page-detection': 'ZeroPageDetection',
> -            '*direct-io': 'bool' } }
> +            '*direct-io': 'bool',
> +            '*fds': { 'type': [ 'FdsTarget' ], 'features': [ 'unstable' ] } } }
>  
>  ##
>  # @query-migrate-parameters:

Re: [PATCH v5 16/19] qapi: add interface for local TAP migration

[Vladimir Sementsov-Ogievskiy]

On 19.09.25 18:20, Fabiano Rosas wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> 
> Hi Vladimir, as usual with "qapi:" patches, some comments about
> language:
> 
>> To migrate virtio-net TAP device backend (including open fds) locally,
>> user should simply set migration parameter
>>
>>     fds = [virtio-net]
>>
>> Why not simple boolean? To simplify migration to further versions,
>> when more devices will support fds migration.
>>
>> Alternatively, we may add per-device option to disable fds-migration,
>> but still:
>>
>> 1. It's more comfortable to set same capabilities/parameters on both
>> source and target QEMU, than care about each device.
>>
>> 2. To not break the design, that machine-type + device options +
>> migration capabilites and parameters are fully define the resulting
>> migration stream. We'll break this if add in future more fds-passing
>> support in devices under same fds=true parameter.
>>
> 
> These arguments look convincing to me.
> 

[..]

>>       return true;
>>   }
>>   
>> @@ -1297,6 +1315,11 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
>>       if (params->has_direct_io) {
>>           dest->direct_io = params->direct_io;
>>       }
>> +
>> +    if (params->has_fds) {
>> +        dest->has_fds = true;
> 
> I think what you want is to set this in
> migrate_params_init(). block_bitmap_mapping is a bit of an outlier in
> that it takes an empty *input* as valid.

Hmm I made it behave like block_bitmap_mapping because it also a list.

As I understand, for list we have to treat empty list not like absent parameter: we should have a way
to clear previously set list by subsequent migrate-set-parameters call.

> 
>> +        dest->fds = params->fds;
>> +    }
>>   }
>>   
>>   static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
>> @@ -1429,6 +1452,13 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
>>       if (params->has_direct_io) {
>>           s->parameters.direct_io = params->direct_io;
>>       }
>> +
>> +    if (params->has_fds) {
>> +        qapi_free_FdsTargetList(s->parameters.fds);
>> +
>> +        s->parameters.has_fds = true;
>> +        s->parameters.fds = QAPI_CLONE(FdsTargetList, params->fds);
> 
> Same here, has_fds should always be true in s->parameters.
> 
>> +    }
>>   }
>>   
>>   void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
>> diff --git a/migration/options.h b/migration/options.h
>> index 82d839709e..a79472a235 100644
>> --- a/migration/options.h
>> +++ b/migration/options.h
>> @@ -87,6 +87,8 @@ const char *migrate_tls_hostname(void);
>>   uint64_t migrate_xbzrle_cache_size(void);
>>   ZeroPageDetection migrate_zero_page_detection(void);
>>   
>> +bool migrate_fds_virtio_net(void);
>> +
>>   /* parameters helpers */
>>   
>>   bool migrate_params_check(MigrationParameters *params, Error **errp);
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 2387c21e9c..6ef9629c6d 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -747,6 +747,19 @@
>>         '*transform': 'BitmapMigrationBitmapAliasTransform'
>>     } }
>>   
>> +##
>> +# @FdsTarget:
>> +#
>> +# @virtio-net: Enable live backend migration for virtio-net.
> 
> So you're assuming normal migration is "non-live backend migration"
> because the backend is stopped and started again on the destination. I
> think it makes sense.
> 
>> +#     The only supported backend is TAP device. When enabled, TAP fds
>> +#     and all related state is passed to target QEMU through migration
>> +#     channel (which should be unix socket).
>> +#
>> +# Since: 10.2
>> +##
>> +{ 'enum': 'FdsTarget',
>> +  'data': [ 'virtio-net' ] }
>> +
>>   ##
>>   # @BitmapMigrationNodeAlias:
>>   #
>> @@ -924,10 +937,14 @@
>>   #     only has effect if the @mapped-ram capability is enabled.
>>   #     (Since 9.1)
>>   #
>> +# @fds: List of targets to enable live-backend migration for. This
>> +#     requires migration channel to be a unix socket (to pass fds
>> +#     through). (Since 10.2)
> 
> I think I prefer live-backend as written here rather than the non
> hyphenated version above. This clears up the ambiguity and can be used
> in variable names, as a name for the feature and ... _thinks really
> hard_ ... as the parameter name! (maybe)
> 
> On that note, fds is just too broad, I'm sure you know that, but to be
> specific, we already have fd migration, multifd, fdset (as argument of
> file: migration), etc. Talking just about the user interface here, of
> course.
> 
> Also: "@fds: List of targets..." is super confusing.
> 
> And although "to pass fds through" is helping clarify the meaning of
> @fds, it exposes implementation details on the QAPI, I don't think it's
> relevant in the parameter description.

Agree. I see all this mess, each time I come with some new name:
live-tap, live-backend, fds-passing, fds migration, local tap migration, fds...
Finally, only one should be chosen for all names and in documentation.

With your comments, "live-backend" really looks the best one.

Still, we can't say live-backends: [virtio-net], as virtio-net is not
a backed.

Maybe
    
    live-backends: [virtio-net-tap]

to show that it's about virtio-net+TAP pair.

> 
>> +#
>>   # Features:
>>   #
>> -# @unstable: Members @x-checkpoint-delay and
>> -#     @x-vcpu-dirty-limit-period are experimental.


Thanks!


-- 
Best regards,
Vladimir

Re: [PATCH v5 16/19] qapi: add interface for local TAP migration

[Fabiano Rosas]

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> On 19.09.25 18:20, Fabiano Rosas wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>> 
>> Hi Vladimir, as usual with "qapi:" patches, some comments about
>> language:
>> 
>>> To migrate virtio-net TAP device backend (including open fds) locally,
>>> user should simply set migration parameter
>>>
>>>     fds = [virtio-net]
>>>
>>> Why not simple boolean? To simplify migration to further versions,
>>> when more devices will support fds migration.
>>>
>>> Alternatively, we may add per-device option to disable fds-migration,
>>> but still:
>>>
>>> 1. It's more comfortable to set same capabilities/parameters on both
>>> source and target QEMU, than care about each device.
>>>
>>> 2. To not break the design, that machine-type + device options +
>>> migration capabilites and parameters are fully define the resulting
>>> migration stream. We'll break this if add in future more fds-passing
>>> support in devices under same fds=true parameter.
>>>
>> 
>> These arguments look convincing to me.
>> 
>
> [..]
>
>>>       return true;
>>>   }
>>>   
>>> @@ -1297,6 +1315,11 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
>>>       if (params->has_direct_io) {
>>>           dest->direct_io = params->direct_io;
>>>       }
>>> +
>>> +    if (params->has_fds) {
>>> +        dest->has_fds = true;
>> 
>> I think what you want is to set this in
>> migrate_params_init(). block_bitmap_mapping is a bit of an outlier in
>> that it takes an empty *input* as valid.
>
> Hmm I made it behave like block_bitmap_mapping because it also a list.
>
> As I understand, for list we have to treat empty list not like absent parameter: we should have a way
> to clear previously set list by subsequent migrate-set-parameters call.
>

Sorry, I explained myself poorly. Yes, the empty list is valid and it
clears a previously set value. But that's just:

migrate_params_init(MigrationParameters *params):
    ...
    params->has_fds = true;

migrate_params_test_apply(MigrateSetParameters *params,
                          MigrationParameters *dest):
    ...
    /* means user provided it */                          
    if (params->has_fds) {
        dest->fds = params->fds;
    }

migrate_params_check(MigrationParameters *params):
    ...
    if (params->has_fds) {
       /* empty list ok */
    }

migrate_params_apply(MigrateSetParameters *params):
    ...
    if (params->has_fds) {
        qapi_free_...(s->parameters.fds);
        /* clones the full list or the empty list, no difference */
        s->parameters.fds = QAPI_CLONE(..., params->fds);
    }

The block_bitmap_mapping does the has_ part a bit differently because it
wants to carry that flag over to the rest of the code. See 3cba22c9ad
("migration: Fix block_bitmap_mapping migration"). In that case, even if
the list is empty, it needs to know when it was made empty on purpose to
carry on with the removal of the mappings. In your case, it doesn't
matter, empty is empty, whether forcefully, or because it was never
set. Right?

(this migrate_params code is tricky, FYI I'm reworking that in:
https://lore.kernel.org/r/20250603013810.4772-1-farosas@suse.de)

>> 
>>> +        dest->fds = params->fds;
>>> +    }
>>>   }
>>>   
>>>   static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
>>> @@ -1429,6 +1452,13 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
>>>       if (params->has_direct_io) {
>>>           s->parameters.direct_io = params->direct_io;
>>>       }
>>> +
>>> +    if (params->has_fds) {
>>> +        qapi_free_FdsTargetList(s->parameters.fds);
>>> +
>>> +        s->parameters.has_fds = true;
>>> +        s->parameters.fds = QAPI_CLONE(FdsTargetList, params->fds);
>> 
>> Same here, has_fds should always be true in s->parameters.
>> 
>>> +    }
>>>   }
>>>   
>>>   void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
>>> diff --git a/migration/options.h b/migration/options.h
>>> index 82d839709e..a79472a235 100644
>>> --- a/migration/options.h
>>> +++ b/migration/options.h
>>> @@ -87,6 +87,8 @@ const char *migrate_tls_hostname(void);
>>>   uint64_t migrate_xbzrle_cache_size(void);
>>>   ZeroPageDetection migrate_zero_page_detection(void);
>>>   
>>> +bool migrate_fds_virtio_net(void);
>>> +
>>>   /* parameters helpers */
>>>   
>>>   bool migrate_params_check(MigrationParameters *params, Error **errp);
>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>> index 2387c21e9c..6ef9629c6d 100644
>>> --- a/qapi/migration.json
>>> +++ b/qapi/migration.json
>>> @@ -747,6 +747,19 @@
>>>         '*transform': 'BitmapMigrationBitmapAliasTransform'
>>>     } }
>>>   
>>> +##
>>> +# @FdsTarget:
>>> +#
>>> +# @virtio-net: Enable live backend migration for virtio-net.
>> 
>> So you're assuming normal migration is "non-live backend migration"
>> because the backend is stopped and started again on the destination. I
>> think it makes sense.
>> 
>>> +#     The only supported backend is TAP device. When enabled, TAP fds
>>> +#     and all related state is passed to target QEMU through migration
>>> +#     channel (which should be unix socket).
>>> +#
>>> +# Since: 10.2
>>> +##
>>> +{ 'enum': 'FdsTarget',
>>> +  'data': [ 'virtio-net' ] }
>>> +
>>>   ##
>>>   # @BitmapMigrationNodeAlias:
>>>   #
>>> @@ -924,10 +937,14 @@
>>>   #     only has effect if the @mapped-ram capability is enabled.
>>>   #     (Since 9.1)
>>>   #
>>> +# @fds: List of targets to enable live-backend migration for. This
>>> +#     requires migration channel to be a unix socket (to pass fds
>>> +#     through). (Since 10.2)
>> 
>> I think I prefer live-backend as written here rather than the non
>> hyphenated version above. This clears up the ambiguity and can be used
>> in variable names, as a name for the feature and ... _thinks really
>> hard_ ... as the parameter name! (maybe)
>> 
>> On that note, fds is just too broad, I'm sure you know that, but to be
>> specific, we already have fd migration, multifd, fdset (as argument of
>> file: migration), etc. Talking just about the user interface here, of
>> course.
>> 
>> Also: "@fds: List of targets..." is super confusing.
>> 
>> And although "to pass fds through" is helping clarify the meaning of
>> @fds, it exposes implementation details on the QAPI, I don't think it's
>> relevant in the parameter description.
>
> Agree. I see all this mess, each time I come with some new name:
> live-tap, live-backend, fds-passing, fds migration, local tap migration, fds...
> Finally, only one should be chosen for all names and in documentation.
>

Yeah, not to mention the similarities with CPR. I thought of borrowing
the "transfer" term to underline that's a common concept between the two
approaches, but I'm not sure it's worth the trouble.

[live-]backend-transfer

> With your comments, "live-backend" really looks the best one.
>
> Still, we can't say live-backends: [virtio-net], as virtio-net is not
> a backed.
>
> Maybe
>     
>     live-backends: [virtio-net-tap]
>
> to show that it's about virtio-net+TAP pair.
>

Works for me.

>> 
>>> +#
>>>   # Features:
>>>   #
>>> -# @unstable: Members @x-checkpoint-delay and
>>> -#     @x-vcpu-dirty-limit-period are experimental.
>
>
> Thanks!

Re: [PATCH v5 15/19] net/tap: postpone tap setup to pre-incoming

[Vladimir Sementsov-Ogievskiy]

On 19.09.25 12:55, Vladimir Sementsov-Ogievskiy wrote:
> As described in previous commit, to support fds-passing TAP migration,
> we need to postpone the decision to open the device or to wait for
> incoming fds up to pre-incoming point (when we actually can decide).
> 
> This commit only postpones TAP-open case of initialization.
> We don't try to postpone the all cases of initialization, as it will
> require a lot more work of refactoring the code.
> 
> So we postpone only the simple case, for which we are going to support
> fd-incoming migration:
> 
> 1. No fds / fd parameters: obviously, if user give fd/fds the should
> be used, no incoming fds migration is possible.
> 
> 2. No helper: just for simplicity. It probably possible to allow it
> (and just ignore in case of fds-incoming migration), to allow user
> use same cmdline on target QEMU.. But that questionable, and
> postponable.
> 
> 3. No sciprt/downscript. It's not simple to support downscript:
> we should pass the responsiblity to call it on target QEMU with
> migration.. And back to source QEMU on migration failure. It
> feasible, but may be implemented later on demand.
> 
> 3. Concrete ifname and vnet_hdr: to not try to share them between
> queues, when we only can setup queues as separate entities.
> Supporting undecided ifname will require to create some extra
> netdev state, connecting all the taps, to be able to iterate through
> them.
> 
> No part of incoming-fds migration is here, we only prepare the code
> for future implementation of it.
> 
> Are net-drivers prepared to postponed initialization of NICs?
> For future feature of fds-passing migration, we are mainly interested
> in virtio-net. So, let's prepare virtio-net to work with postponed
> initialization of TAP (two places about early set/get features) and
> for other drivers let's simply finalize initialization on setting
> netdev property. Support for other drivers may be added later if
> needed.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy<vsementsov@yandex-team.ru>


gitlab CI helps: "undefined reference to `tap_wait_incoming'" in win64 system build, will fix.

-- 
Best regards,
Vladimir

Re: [PATCH v5 16/19] qapi: add interface for local TAP migration

[Vladimir Sementsov-Ogievskiy]

On 19.09.25 20:13, Fabiano Rosas wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> 
>> On 19.09.25 18:20, Fabiano Rosas wrote:
>>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>>>
>>> Hi Vladimir, as usual with "qapi:" patches, some comments about
>>> language:
>>>
>>>> To migrate virtio-net TAP device backend (including open fds) locally,
>>>> user should simply set migration parameter
>>>>
>>>>      fds = [virtio-net]
>>>>
>>>> Why not simple boolean? To simplify migration to further versions,
>>>> when more devices will support fds migration.
>>>>
>>>> Alternatively, we may add per-device option to disable fds-migration,
>>>> but still:
>>>>
>>>> 1. It's more comfortable to set same capabilities/parameters on both
>>>> source and target QEMU, than care about each device.
>>>>
>>>> 2. To not break the design, that machine-type + device options +
>>>> migration capabilites and parameters are fully define the resulting
>>>> migration stream. We'll break this if add in future more fds-passing
>>>> support in devices under same fds=true parameter.
>>>>
>>>
>>> These arguments look convincing to me.
>>>
>>
>> [..]
>>
>>>>        return true;
>>>>    }
>>>>    
>>>> @@ -1297,6 +1315,11 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
>>>>        if (params->has_direct_io) {
>>>>            dest->direct_io = params->direct_io;
>>>>        }
>>>> +
>>>> +    if (params->has_fds) {
>>>> +        dest->has_fds = true;
>>>
>>> I think what you want is to set this in
>>> migrate_params_init(). block_bitmap_mapping is a bit of an outlier in
>>> that it takes an empty *input* as valid.
>>
>> Hmm I made it behave like block_bitmap_mapping because it also a list.
>>
>> As I understand, for list we have to treat empty list not like absent parameter: we should have a way
>> to clear previously set list by subsequent migrate-set-parameters call.
>>
> 
> Sorry, I explained myself poorly. Yes, the empty list is valid and it
> clears a previously set value. But that's just:
> 
> migrate_params_init(MigrationParameters *params):
>      ...
>      params->has_fds = true;
> 
> migrate_params_test_apply(MigrateSetParameters *params,
>                            MigrationParameters *dest):
>      ...
>      /* means user provided it */
>      if (params->has_fds) {
>          dest->fds = params->fds;
>      }
> 
> migrate_params_check(MigrationParameters *params):
>      ...
>      if (params->has_fds) {
>         /* empty list ok */
>      }
> 
> migrate_params_apply(MigrateSetParameters *params):
>      ...
>      if (params->has_fds) {
>          qapi_free_...(s->parameters.fds);
>          /* clones the full list or the empty list, no difference */
>          s->parameters.fds = QAPI_CLONE(..., params->fds);
>      }
> 
> The block_bitmap_mapping does the has_ part a bit differently because it
> wants to carry that flag over to the rest of the code. See 3cba22c9ad
> ("migration: Fix block_bitmap_mapping migration"). In that case, even if
> the list is empty, it needs to know when it was made empty on purpose to
> carry on with the removal of the mappings. In your case, it doesn't
> matter, empty is empty, whether forcefully, or because it was never
> set. Right?

Oh, yes, understand now, thanks.



-- 
Best regards,
Vladimir