[PATCH 0/5] RFC: Assorted migration improvements

[PATCH 0/5] RFC: Assorted migration improvements

[David Gibson]

Here are some things that I think improve the draft migration logic.
This is based on both the draft migration series (v2) and my
epoll_del() series.

Feel free to fold into the existing patches if that seems sensible.

David Gibson (5):
  migrate: Add passt-repair to .gitignore
  migrate: vu_migrate_{source,target}() aren't actually vu speciic
  migrate: Move repair_sock_init() to vu_init()
  migrate: Make more handling common rather than vhost-user specific
  migrate: Don't handle the migration channel through epoll

 .gitignore   |   1 +
 epoll_type.h |   2 -
 migrate.c    | 148 +++++++++++++++++++++++++++++++++++++++++++++++----
 migrate.h    |  12 ++---
 passt.c      |   6 +--
 passt.h      |   8 +++
 tap.c        |   3 --
 vhost_user.c |  58 +++-----------------
 virtio.h     |   4 --
 vu_common.c  |  89 -------------------------------
 vu_common.h  |   2 +-
 11 files changed, 162 insertions(+), 171 deletions(-)

-- 
2.48.1

[PATCH 1/5] migrate: Add passt-repair to .gitignore

[David Gibson]

Like our other executable targets.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index d1c8be98..5824a71e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,6 +3,7 @@
 /passt.avx2
 /pasta
 /pasta.avx2
+/passt-repair
 /qrap
 /pasta.1
 /seccomp.h
-- 
@@ -3,6 +3,7 @@
 /passt.avx2
 /pasta
 /pasta.avx2
+/passt-repair
 /qrap
 /pasta.1
 /seccomp.h
-- 
2.48.1

[PATCH 2/5] migrate: vu_migrate_{source,target}() aren't actually vu speciic

[David Gibson]

vu_migrate_source() and vu_migrate_target() don't directly rely on anything
vhost-user specific - it's just that they'll only be called for vhost-user
so far.  They are suitable as general top-level dispatchers for
migration.  Move them to migrate.c, rename to migrate_{source,target}() and
make the lower-level functions they call local to migrate.c.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 migrate.c   | 80 +++++++++++++++++++++++++++++++++++++++++++++++------
 migrate.h   | 10 ++-----
 vu_common.c | 66 ++-----------------------------------------
 3 files changed, 75 insertions(+), 81 deletions(-)

diff --git a/migrate.c b/migrate.c
index 6707c029..148cd660 100644
--- a/migrate.c
+++ b/migrate.c
@@ -92,7 +92,7 @@ struct migrate_target_handlers target_handlers[] = {
  *
  * Return: 0 on success, error code on failure
  */
-int migrate_source_pre(struct ctx *c, struct migrate_meta *m)
+static int migrate_source_pre(struct ctx *c, struct migrate_meta *m)
 {
 	struct migrate_handler *h;
 
@@ -107,13 +107,13 @@ int migrate_source_pre(struct ctx *c, struct migrate_meta *m)
 }
 
 /**
- * migrate_source() - Perform migration as source: send state to hypervisor
+ * migrate_source_state() - Send device state as migration source
  * @fd:		Descriptor for state transfer
  * @m:		Migration metadata
  *
  * Return: 0 on success, error code on failure
  */
-int migrate_source(int fd, const struct migrate_meta *m)
+static int migrate_source_state(int fd, const struct migrate_meta *m)
 {
 	static struct migrate_data *d;
 	int count, rc;
@@ -139,7 +139,7 @@ int migrate_source(int fd, const struct migrate_meta *m)
  *
  * Return: 0 on success, error code on failure
  */
-void migrate_source_post(struct ctx *c, struct migrate_meta *m)
+static void migrate_source_post(struct ctx *c, struct migrate_meta *m)
 {
 	struct migrate_handler *h;
 
@@ -147,6 +147,34 @@ void migrate_source_post(struct ctx *c, struct migrate_meta *m)
 		h->fn(c, m);
 }
 
+/**
+ * migrate_source() - Migration as source, send state to hypervisor
+ * @c:		Execution context
+ * @fd:		File descriptor for state transfer
+ *
+ * Return: 0 on success, positive error code on failure
+ */
+int migrate_source(struct ctx *c, int fd)
+{
+	struct migrate_meta m;
+	int rc;
+
+	if ((rc = migrate_source_pre(c, &m))) {
+		err("Source pre-migration failed: %s, abort", strerror_(rc));
+		return rc;
+	}
+
+	debug("Saving backend state");
+
+	rc = migrate_source_state(fd, &m);
+	if (rc)
+		err("Source migration failed: %s", strerror_(rc));
+	else
+		migrate_source_post(c, &m);
+
+	return rc;
+}
+
 /**
  * migrate_target_read_header() - Set metadata in target from source header
  * @fd:		Descriptor for state transfer
@@ -154,7 +182,7 @@ void migrate_source_post(struct ctx *c, struct migrate_meta *m)
  *
  * Return: 0 on success, error code on failure
  */
-int migrate_target_read_header(int fd, struct migrate_meta *m)
+static int migrate_target_read_header(int fd, struct migrate_meta *m)
 {
 	static struct migrate_data *d;
 	union migrate_header h;
@@ -204,7 +232,7 @@ int migrate_target_read_header(int fd, struct migrate_meta *m)
  *
  * Return: 0 on success, error code on failure
  */
-int migrate_target_pre(struct ctx *c, struct migrate_meta *m)
+static int migrate_target_pre(struct ctx *c, struct migrate_meta *m)
 {
 	struct migrate_target_handlers *th;
 	struct migrate_handler *h;
@@ -222,7 +250,7 @@ int migrate_target_pre(struct ctx *c, struct migrate_meta *m)
 }
 
 /**
- * migrate_target() - Perform migration as target: receive state from hypervisor
+ * migrate_target_state() - Receive device state as migration target
  * @fd:		Descriptor for state transfer
  * @m:		Migration metadata
  *
@@ -230,7 +258,7 @@ int migrate_target_pre(struct ctx *c, struct migrate_meta *m)
  *
  * #syscalls:vu readv
  */
-int migrate_target(int fd, const struct migrate_meta *m)
+static int migrate_target_state(int fd, const struct migrate_meta *m)
 {
 	static struct migrate_data *d;
 	unsigned cnt;
@@ -253,7 +281,7 @@ int migrate_target(int fd, const struct migrate_meta *m)
  * @c:		Execution context
  * @m:		Migration metadata
  */
-void migrate_target_post(struct ctx *c, struct migrate_meta *m)
+static void migrate_target_post(struct ctx *c, struct migrate_meta *m)
 {
 	struct migrate_target_handlers *th;
 	struct migrate_handler *h;
@@ -263,3 +291,37 @@ void migrate_target_post(struct ctx *c, struct migrate_meta *m)
 	for (h = th->post; h->fn; h++)
 		h->fn(c, m);
 }
+
+/**
+ * migrate_target() - Migration as target, receive state from hypervisor
+ * @c:		Execution context
+ * @fd:		File descriptor for state transfer
+ *
+ * Return: 0 on success, positive error code on failure
+ */
+int migrate_target(struct ctx *c, int fd)
+{
+	struct migrate_meta m;
+	int rc;
+
+	rc = migrate_target_read_header(fd, &m);
+	if (rc) {
+		err("Migration header check failed: %s, abort", strerror_(rc));
+		return rc;
+	}
+
+	if ((rc = migrate_target_pre(c, &m))) {
+		err("Target pre-migration failed: %s, abort", strerror_(rc));
+		return rc;
+	}
+
+	debug("Loading backend state");
+
+	rc = migrate_target_state(fd, &m);
+	if (rc)
+		err("Target migration failed: %s", strerror_(rc));
+	else
+		migrate_target_post(c, &m);
+
+	return rc;
+}
diff --git a/migrate.h b/migrate.h
index f9635ac2..edf6303e 100644
--- a/migrate.h
+++ b/migrate.h
@@ -76,13 +76,7 @@ struct migrate_target_handlers {
 	struct migrate_handler *post;
 };
 
-int migrate_source_pre(struct ctx *c, struct migrate_meta *m);
-int migrate_source(int fd, const struct migrate_meta *m);
-void migrate_source_post(struct ctx *c, struct migrate_meta *m);
-
-int migrate_target_read_header(int fd, struct migrate_meta *m);
-int migrate_target_pre(struct ctx *c, struct migrate_meta *m);
-int migrate_target(int fd, const struct migrate_meta *m);
-void migrate_target_post(struct ctx *c, struct migrate_meta *m);
+int migrate_source(struct ctx *c, int fd);
+int migrate_target(struct ctx *c, int fd);
 
 #endif /* MIGRATE_H */
diff --git a/vu_common.c b/vu_common.c
index c1980d99..d7e39e63 100644
--- a/vu_common.c
+++ b/vu_common.c
@@ -306,68 +306,6 @@ err:
 	return -1;
 }
 
-/**
- * vu_migrate_source() - Migration as source, send state to hypervisor
- * @c:		Execution context
- * @fd:		File descriptor for state transfer
- *
- * Return: 0 on success, positive error code on failure
- */
-static int vu_migrate_source(struct ctx *c, int fd)
-{
-	struct migrate_meta m;
-	int rc;
-
-	if ((rc = migrate_source_pre(c, &m))) {
-		err("Source pre-migration failed: %s, abort", strerror_(rc));
-		return rc;
-	}
-
-	debug("Saving backend state");
-
-	rc = migrate_source(fd, &m);
-	if (rc)
-		err("Source migration failed: %s", strerror_(rc));
-	else
-		migrate_source_post(c, &m);
-
-	return rc;
-}
-
-/**
- * vu_migrate_target() - Migration as target, receive state from hypervisor
- * @c:		Execution context
- * @fd:		File descriptor for state transfer
- *
- * Return: 0 on success, positive error code on failure
- */
-static int vu_migrate_target(struct ctx *c, int fd)
-{
-	struct migrate_meta m;
-	int rc;
-
-	rc = migrate_target_read_header(fd, &m);
-	if (rc) {
-		err("Migration header check failed: %s, abort", strerror_(rc));
-		return rc;
-	}
-
-	if ((rc = migrate_target_pre(c, &m))) {
-		err("Target pre-migration failed: %s, abort", strerror_(rc));
-		return rc;
-	}
-
-	debug("Loading backend state");
-
-	rc = migrate_target(fd, &m);
-	if (rc)
-		err("Target migration failed: %s", strerror_(rc));
-	else
-		migrate_target_post(c, &m);
-
-	return rc;
-}
-
 /**
  * vu_migrate() - Send/receive passt internal state to/from QEMU
  * @c:		Execution context
@@ -381,9 +319,9 @@ void vu_migrate(struct ctx *c, uint32_t events)
 	debug("vu_migrate fd %d events %x", vdev->device_state_fd, events);
 
 	if (events & EPOLLOUT)
-		rc = vu_migrate_source(c, vdev->device_state_fd);
+		rc = migrate_source(c, vdev->device_state_fd);
 	else if (events & EPOLLIN)
-		rc = vu_migrate_target(c, vdev->device_state_fd);
+		rc = migrate_target(c, vdev->device_state_fd);
 
 	/* EPOLLHUP without EPOLLIN/EPOLLOUT, or EPOLLERR? Migration failed */
 
-- 
@@ -306,68 +306,6 @@ err:
 	return -1;
 }
 
-/**
- * vu_migrate_source() - Migration as source, send state to hypervisor
- * @c:		Execution context
- * @fd:		File descriptor for state transfer
- *
- * Return: 0 on success, positive error code on failure
- */
-static int vu_migrate_source(struct ctx *c, int fd)
-{
-	struct migrate_meta m;
-	int rc;
-
-	if ((rc = migrate_source_pre(c, &m))) {
-		err("Source pre-migration failed: %s, abort", strerror_(rc));
-		return rc;
-	}
-
-	debug("Saving backend state");
-
-	rc = migrate_source(fd, &m);
-	if (rc)
-		err("Source migration failed: %s", strerror_(rc));
-	else
-		migrate_source_post(c, &m);
-
-	return rc;
-}
-
-/**
- * vu_migrate_target() - Migration as target, receive state from hypervisor
- * @c:		Execution context
- * @fd:		File descriptor for state transfer
- *
- * Return: 0 on success, positive error code on failure
- */
-static int vu_migrate_target(struct ctx *c, int fd)
-{
-	struct migrate_meta m;
-	int rc;
-
-	rc = migrate_target_read_header(fd, &m);
-	if (rc) {
-		err("Migration header check failed: %s, abort", strerror_(rc));
-		return rc;
-	}
-
-	if ((rc = migrate_target_pre(c, &m))) {
-		err("Target pre-migration failed: %s, abort", strerror_(rc));
-		return rc;
-	}
-
-	debug("Loading backend state");
-
-	rc = migrate_target(fd, &m);
-	if (rc)
-		err("Target migration failed: %s", strerror_(rc));
-	else
-		migrate_target_post(c, &m);
-
-	return rc;
-}
-
 /**
  * vu_migrate() - Send/receive passt internal state to/from QEMU
  * @c:		Execution context
@@ -381,9 +319,9 @@ void vu_migrate(struct ctx *c, uint32_t events)
 	debug("vu_migrate fd %d events %x", vdev->device_state_fd, events);
 
 	if (events & EPOLLOUT)
-		rc = vu_migrate_source(c, vdev->device_state_fd);
+		rc = migrate_source(c, vdev->device_state_fd);
 	else if (events & EPOLLIN)
-		rc = vu_migrate_target(c, vdev->device_state_fd);
+		rc = migrate_target(c, vdev->device_state_fd);
 
 	/* EPOLLHUP without EPOLLIN/EPOLLOUT, or EPOLLERR? Migration failed */
 
-- 
2.48.1

[PATCH 3/5] migrate: Move repair_sock_init() to vu_init()

[David Gibson]

Currently we call repair_sock_init() immediately before
tap_sock_unix_init().  However, this means it will be skipped if the
vhost-user control fd is passed with --fd instead of being created at a
specific path.  We still need the repair socket in that case.

Move it, instead, to vu_init(), which has the added advantage of moving
all migration related one-time init to the same place.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tap.c        | 3 ---
 vhost_user.c | 3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tap.c b/tap.c
index 3659aabd..d1a9f522 100644
--- a/tap.c
+++ b/tap.c
@@ -56,7 +56,6 @@
 #include "netlink.h"
 #include "pasta.h"
 #include "packet.h"
-#include "repair.h"
 #include "tap.h"
 #include "log.h"
 #include "vhost_user.h"
@@ -1362,8 +1361,6 @@ void tap_backend_init(struct ctx *c)
 		tap_sock_tun_init(c);
 		break;
 	case MODE_VU:
-		repair_sock_init(c);
-		/* fall through */
 	case MODE_PASST:
 		tap_sock_unix_init(c);
 
diff --git a/vhost_user.c b/vhost_user.c
index bbbf504f..5df29c47 100644
--- a/vhost_user.c
+++ b/vhost_user.c
@@ -44,6 +44,7 @@
 #include "tap.h"
 #include "vhost_user.h"
 #include "pcap.h"
+#include "repair.h"
 
 /* vhost-user version we are compatible with */
 #define VHOST_USER_VERSION 1
@@ -1106,8 +1107,10 @@ void vu_init(struct ctx *c)
 	}
 	c->vdev->log_table = NULL;
 	c->vdev->log_call_fd = -1;
+
 	c->vdev->device_state_fd = -1;
 	c->vdev->device_state_result = -1;
+	repair_sock_init(c);
 }
 
 
-- 
@@ -44,6 +44,7 @@
 #include "tap.h"
 #include "vhost_user.h"
 #include "pcap.h"
+#include "repair.h"
 
 /* vhost-user version we are compatible with */
 #define VHOST_USER_VERSION 1
@@ -1106,8 +1107,10 @@ void vu_init(struct ctx *c)
 	}
 	c->vdev->log_table = NULL;
 	c->vdev->log_call_fd = -1;
+
 	c->vdev->device_state_fd = -1;
 	c->vdev->device_state_result = -1;
+	repair_sock_init(c);
 }
 
 
-- 
2.48.1

[PATCH 4/5] migrate: Make more handling common rather than vhost-user specific

[David Gibson]

A lot of the migration logic in vhost_user.c and vu_common.c isn't really
specific to vhost-user, but matches the overall structure of migration.
This applies to vu_migrate() and to the parts of of
vu_set_device_state_fd_exec() which aren't related to parsing the specific
vhost-user control request.

Move this logic to migrate.c, with matching renames.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 epoll_type.h |  4 +--
 migrate.c    | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 migrate.h    |  6 ++--
 passt.c      |  6 ++--
 passt.h      |  6 ++++
 vhost_user.c | 59 ++++-----------------------------
 virtio.h     |  4 ---
 vu_common.c  | 27 ---------------
 vu_common.h  |  2 +-
 9 files changed, 112 insertions(+), 94 deletions(-)

diff --git a/epoll_type.h b/epoll_type.h
index 706238aa..b981d30e 100644
--- a/epoll_type.h
+++ b/epoll_type.h
@@ -40,8 +40,8 @@ enum epoll_type {
 	EPOLL_TYPE_VHOST_CMD,
 	/* vhost-user kick event socket */
 	EPOLL_TYPE_VHOST_KICK,
-	/* vhost-user migration socket */
-	EPOLL_TYPE_VHOST_MIGRATION,
+	/* migration device state channel */
+	EPOLL_TYPE_DEVICE_STATE,
 	/* TCP_REPAIR helper listening socket */
 	EPOLL_TYPE_REPAIR_LISTEN,
 	/* TCP_REPAIR helper socket */
diff --git a/migrate.c b/migrate.c
index 148cd660..04df6822 100644
--- a/migrate.c
+++ b/migrate.c
@@ -21,6 +21,7 @@
 #include "inany.h"
 #include "flow.h"
 #include "flow_table.h"
+#include "repair.h"
 
 #include "migrate.h"
 
@@ -154,7 +155,7 @@ static void migrate_source_post(struct ctx *c, struct migrate_meta *m)
  *
  * Return: 0 on success, positive error code on failure
  */
-int migrate_source(struct ctx *c, int fd)
+static int migrate_source(struct ctx *c, int fd)
 {
 	struct migrate_meta m;
 	int rc;
@@ -299,7 +300,7 @@ static void migrate_target_post(struct ctx *c, struct migrate_meta *m)
  *
  * Return: 0 on success, positive error code on failure
  */
-int migrate_target(struct ctx *c, int fd)
+static int migrate_target(struct ctx *c, int fd)
 {
 	struct migrate_meta m;
 	int rc;
@@ -325,3 +326,90 @@ int migrate_target(struct ctx *c, int fd)
 
 	return rc;
 }
+
+/**
+ * set_migration_watch() - Add the migration file descriptor to epoll
+ * @c:		Execution context
+ * @fd:		File descriptor to add
+ * @target:	Are we the target of the migration?
+ */
+static void set_migration_watch(const struct ctx *c, int fd, bool target)
+{
+	union epoll_ref ref = {
+		.type = EPOLL_TYPE_DEVICE_STATE,
+		.fd = fd,
+	 };
+	struct epoll_event ev = { 0 };
+
+	ev.data.u64 = ref.u64;
+	ev.events = target ? EPOLLIN : EPOLLOUT;
+
+	epoll_ctl(c->epollfd, EPOLL_CTL_ADD, ref.fd, &ev);
+}
+
+/**
+ * migrate_init() - Set up things necessary for migration
+ * @c:		Execution context
+ */
+void migrate_init(struct ctx *c)
+{
+	c->device_state_fd = -1;
+	c->device_state_result = -1;
+	repair_sock_init(c);
+}
+
+/**
+ * migrate_close() - Close migration channel
+ * @c:		Execution context
+ */
+void migrate_close(struct ctx *c)
+{
+	if (c->device_state_fd != -1) {
+		debug("Closing migration channel, fd: %d", c->device_state_fd);
+		epoll_del(c, c->device_state_fd);
+		close(c->device_state_fd);
+		c->device_state_fd = -1;
+		c->device_state_result = -1;
+	}
+}
+
+/**
+ * migrate_request() - Request a migration of device state
+ * @c:		Execution context
+ * @fd:		fd to transfer state
+ * @target:	Are we the target of the migration?
+ */
+void migrate_request(struct ctx *c, int fd, bool target)
+{
+	debug("Migration requested, fd: %d", c->device_state_fd);
+
+	if (c->device_state_fd != -1)
+		migrate_close(c);
+
+	c->device_state_fd = fd;
+	set_migration_watch(c, c->device_state_fd, target);
+
+}
+
+/**
+ * migrate_handler() - Send/receive passt internal state to/from QEMU
+ * @c:		Execution context
+ * @events:	epoll events
+ */
+void migrate_handler(struct ctx *c, uint32_t events)
+{
+	int rc = EIO;
+
+	debug("migrate_handler fd %d events %x", c->device_state_fd, events);
+
+	if (events & EPOLLOUT)
+		rc = migrate_source(c, c->device_state_fd);
+	else if (events & EPOLLIN)
+		rc = migrate_target(c, c->device_state_fd);
+
+	/* EPOLLHUP without EPOLLIN/EPOLLOUT, or EPOLLERR? Migration failed */
+
+	migrate_close(c);
+
+	c->device_state_result = rc;
+}
diff --git a/migrate.h b/migrate.h
index edf6303e..b97e55e7 100644
--- a/migrate.h
+++ b/migrate.h
@@ -76,7 +76,9 @@ struct migrate_target_handlers {
 	struct migrate_handler *post;
 };
 
-int migrate_source(struct ctx *c, int fd);
-int migrate_target(struct ctx *c, int fd);
+void migrate_init(struct ctx *c);
+void migrate_close(struct ctx *c);
+void migrate_request(struct ctx *c, int fd, bool target);
+void migrate_handler(struct ctx *c, uint32_t events);
 
 #endif /* MIGRATE_H */
diff --git a/passt.c b/passt.c
index 1fa2ddd2..3c3a3313 100644
--- a/passt.c
+++ b/passt.c
@@ -76,7 +76,7 @@ char *epoll_type_str[] = {
 	[EPOLL_TYPE_TAP_LISTEN]		= "listening qemu socket",
 	[EPOLL_TYPE_VHOST_CMD]		= "vhost-user command socket",
 	[EPOLL_TYPE_VHOST_KICK]		= "vhost-user kick socket",
-	[EPOLL_TYPE_VHOST_MIGRATION]	= "vhost-user migration socket",
+	[EPOLL_TYPE_DEVICE_STATE]	= "migration device state channel",
 	[EPOLL_TYPE_REPAIR_LISTEN]	= "TCP_REPAIR helper listening socket",
 	[EPOLL_TYPE_REPAIR]		= "TCP_REPAIR helper socket",
 };
@@ -360,8 +360,8 @@ loop:
 		case EPOLL_TYPE_VHOST_KICK:
 			vu_kick_cb(c.vdev, ref, &now);
 			break;
-		case EPOLL_TYPE_VHOST_MIGRATION:
-			vu_migrate(&c, eventmask);
+		case EPOLL_TYPE_DEVICE_STATE:
+			migrate_handler(&c, eventmask);
 			break;
 		case EPOLL_TYPE_REPAIR_LISTEN:
 			repair_listen_handler(&c, eventmask);
diff --git a/passt.h b/passt.h
index 85b0a10f..5992cbeb 100644
--- a/passt.h
+++ b/passt.h
@@ -239,6 +239,8 @@ struct ip6_ctx {
  * @low_wmem:		Low probed net.core.wmem_max
  * @low_rmem:		Low probed net.core.rmem_max
  * @vdev:		vhost-user device
+ * @device_state_fd:	Device state migration channel
+ * @device_state_result: Device state migration result
  */
 struct ctx {
 	enum passt_modes mode;
@@ -307,6 +309,10 @@ struct ctx {
 	int low_rmem;
 
 	struct vu_dev *vdev;
+
+	/* Migration */
+	int device_state_fd;
+	int device_state_result;
 };
 
 void proto_update_l2_buf(const unsigned char *eth_d,
diff --git a/vhost_user.c b/vhost_user.c
index 5df29c47..2dde405a 100644
--- a/vhost_user.c
+++ b/vhost_user.c
@@ -44,7 +44,6 @@
 #include "tap.h"
 #include "vhost_user.h"
 #include "pcap.h"
-#include "repair.h"
 
 /* vhost-user version we are compatible with */
 #define VHOST_USER_VERSION 1
@@ -998,36 +997,6 @@ static bool vu_send_rarp_exec(struct vu_dev *vdev,
 	return false;
 }
 
-/**
- * vu_set_migration_watch() - Add the migration file descriptor to epoll
- * @vdev:	vhost-user device
- * @fd:		File descriptor to add
- * @direction:	Direction of the migration (save or load backend state)
- */
-static void vu_set_migration_watch(const struct vu_dev *vdev, int fd,
-				   uint32_t direction)
-{
-	union epoll_ref ref = {
-		.type = EPOLL_TYPE_VHOST_MIGRATION,
-		.fd = fd,
-	 };
-	struct epoll_event ev = { 0 };
-
-	ev.data.u64 = ref.u64;
-	switch (direction) {
-	case VHOST_USER_TRANSFER_STATE_DIRECTION_SAVE:
-		ev.events = EPOLLOUT;
-		break;
-	case VHOST_USER_TRANSFER_STATE_DIRECTION_LOAD:
-		ev.events = EPOLLIN;
-		break;
-	default:
-		ASSERT(0);
-	}
-
-	epoll_ctl(vdev->context->epollfd, EPOLL_CTL_ADD, ref.fd, &ev);
-}
-
 /**
  * vu_set_device_state_fd_exec() - Set the device state migration channel
  * @vdev:	vhost-user device
@@ -1052,16 +1021,8 @@ static bool vu_set_device_state_fd_exec(struct vu_dev *vdev,
 	    direction != VHOST_USER_TRANSFER_STATE_DIRECTION_LOAD)
 		die("Invalide device_state_fd direction: %d", direction);
 
-	if (vdev->device_state_fd != -1) {
-		epoll_del(vdev->context, vdev->device_state_fd);
-		close(vdev->device_state_fd);
-	}
-
-	vdev->device_state_fd = msg->fds[0];
-	vdev->device_state_result = -1;
-	vu_set_migration_watch(vdev, vdev->device_state_fd, direction);
-
-	debug("Got device_state_fd: %d", vdev->device_state_fd);
+	migrate_request(vdev->context, msg->fds[0],
+			direction == VHOST_USER_TRANSFER_STATE_DIRECTION_LOAD);
 
 	/* We don't provide a new fd for the data transfer */
 	vmsg_set_reply_u64(msg, VHOST_USER_VRING_NOFD_MASK);
@@ -1079,9 +1040,7 @@ static bool vu_set_device_state_fd_exec(struct vu_dev *vdev,
 static bool vu_check_device_state_exec(struct vu_dev *vdev,
 				       struct vhost_user_msg *msg)
 {
-	(void)vdev;
-
-	vmsg_set_reply_u64(msg, vdev->device_state_result);
+	vmsg_set_reply_u64(msg, vdev->context->device_state_result);
 
 	return true;
 }
@@ -1108,9 +1067,7 @@ void vu_init(struct ctx *c)
 	c->vdev->log_table = NULL;
 	c->vdev->log_call_fd = -1;
 
-	c->vdev->device_state_fd = -1;
-	c->vdev->device_state_result = -1;
-	repair_sock_init(c);
+	migrate_init(c);
 }
 
 
@@ -1160,12 +1117,8 @@ void vu_cleanup(struct vu_dev *vdev)
 
 	vu_close_log(vdev);
 
-	if (vdev->device_state_fd != -1) {
-		epoll_del(vdev->context, vdev->device_state_fd);
-		close(vdev->device_state_fd);
-		vdev->device_state_fd = -1;
-		vdev->device_state_result = -1;
-	}
+	/* If we lose the VU dev, we also lose our migration channel */
+	migrate_close(vdev->context);
 }
 
 /**
diff --git a/virtio.h b/virtio.h
index 7bef2d27..0a59441b 100644
--- a/virtio.h
+++ b/virtio.h
@@ -106,8 +106,6 @@ struct vu_dev_region {
  * @log_call_fd:		Eventfd to report logging update
  * @log_size:			Size of the logging memory region
  * @log_table:			Base of the logging memory region
- * @device_state_fd:		Device state migration channel
- * @device_state_result:	Device state migration result
  */
 struct vu_dev {
 	struct ctx *context;
@@ -119,8 +117,6 @@ struct vu_dev {
 	int log_call_fd;
 	uint64_t log_size;
 	uint8_t *log_table;
-	int device_state_fd;
-	int device_state_result;
 };
 
 /**
diff --git a/vu_common.c b/vu_common.c
index d7e39e63..78d1c1ba 100644
--- a/vu_common.c
+++ b/vu_common.c
@@ -305,30 +305,3 @@ err:
 
 	return -1;
 }
-
-/**
- * vu_migrate() - Send/receive passt internal state to/from QEMU
- * @c:		Execution context
- * @events:	epoll events
- */
-void vu_migrate(struct ctx *c, uint32_t events)
-{
-	struct vu_dev *vdev = c->vdev;
-	int rc = EIO;
-
-	debug("vu_migrate fd %d events %x", vdev->device_state_fd, events);
-
-	if (events & EPOLLOUT)
-		rc = migrate_source(c, vdev->device_state_fd);
-	else if (events & EPOLLIN)
-		rc = migrate_target(c, vdev->device_state_fd);
-
-	/* EPOLLHUP without EPOLLIN/EPOLLOUT, or EPOLLERR? Migration failed */
-
-	vdev->device_state_result = rc;
-
-	epoll_del(c, vdev->device_state_fd);
-	debug("Closing migration channel");
-	close(vdev->device_state_fd);
-	vdev->device_state_fd = -1;
-}
diff --git a/vu_common.h b/vu_common.h
index 69c40063..f538f237 100644
--- a/vu_common.h
+++ b/vu_common.h
@@ -57,5 +57,5 @@ void vu_flush(const struct vu_dev *vdev, struct vu_virtq *vq,
 void vu_kick_cb(struct vu_dev *vdev, union epoll_ref ref,
 		const struct timespec *now);
 int vu_send_single(const struct ctx *c, const void *buf, size_t size);
-void vu_migrate(struct ctx *c, uint32_t events);
+
 #endif /* VU_COMMON_H */
-- 
@@ -57,5 +57,5 @@ void vu_flush(const struct vu_dev *vdev, struct vu_virtq *vq,
 void vu_kick_cb(struct vu_dev *vdev, union epoll_ref ref,
 		const struct timespec *now);
 int vu_send_single(const struct ctx *c, const void *buf, size_t size);
-void vu_migrate(struct ctx *c, uint32_t events);
+
 #endif /* VU_COMMON_H */
-- 
2.48.1

[PATCH 5/5] migrate: Don't handle the migration channel through epoll

[David Gibson]

Currently, once a migration device state fd is assigned, we wait for
EPOLLIN or EPOLLOUT events on it to actually perform the migration.  Change
it so that once a migration is requested it we complete it synchronously
at the end of the current epoll cycle.  This has several advantages:

  1. It makes it clear that everything about the migration must be dealt
     with at once, not split between multiple epoll events on the channel
  2. It ensures the migration always takes place between epoll cycles,
     rather than, for example, between handling TCP events and their
     deferred handling in post_handler().
  3. It reduces code setting up the epoll watch on the fd.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 epoll_type.h |  2 --
 migrate.c    | 42 +++++++++++-------------------------------
 migrate.h    |  2 +-
 passt.c      |  6 ++----
 passt.h      |  2 ++
 5 files changed, 16 insertions(+), 38 deletions(-)

diff --git a/epoll_type.h b/epoll_type.h
index b981d30e..7f2a1217 100644
--- a/epoll_type.h
+++ b/epoll_type.h
@@ -40,8 +40,6 @@ enum epoll_type {
 	EPOLL_TYPE_VHOST_CMD,
 	/* vhost-user kick event socket */
 	EPOLL_TYPE_VHOST_KICK,
-	/* migration device state channel */
-	EPOLL_TYPE_DEVICE_STATE,
 	/* TCP_REPAIR helper listening socket */
 	EPOLL_TYPE_REPAIR_LISTEN,
 	/* TCP_REPAIR helper socket */
diff --git a/migrate.c b/migrate.c
index 04df6822..4279d1f8 100644
--- a/migrate.c
+++ b/migrate.c
@@ -327,26 +327,6 @@ static int migrate_target(struct ctx *c, int fd)
 	return rc;
 }
 
-/**
- * set_migration_watch() - Add the migration file descriptor to epoll
- * @c:		Execution context
- * @fd:		File descriptor to add
- * @target:	Are we the target of the migration?
- */
-static void set_migration_watch(const struct ctx *c, int fd, bool target)
-{
-	union epoll_ref ref = {
-		.type = EPOLL_TYPE_DEVICE_STATE,
-		.fd = fd,
-	 };
-	struct epoll_event ev = { 0 };
-
-	ev.data.u64 = ref.u64;
-	ev.events = target ? EPOLLIN : EPOLLOUT;
-
-	epoll_ctl(c->epollfd, EPOLL_CTL_ADD, ref.fd, &ev);
-}
-
 /**
  * migrate_init() - Set up things necessary for migration
  * @c:		Execution context
@@ -387,27 +367,27 @@ void migrate_request(struct ctx *c, int fd, bool target)
 		migrate_close(c);
 
 	c->device_state_fd = fd;
-	set_migration_watch(c, c->device_state_fd, target);
-
+	c->migrate_target = target;
 }
 
 /**
  * migrate_handler() - Send/receive passt internal state to/from QEMU
  * @c:		Execution context
- * @events:	epoll events
  */
-void migrate_handler(struct ctx *c, uint32_t events)
+void migrate_handler(struct ctx *c)
 {
-	int rc = EIO;
+	int rc;
 
-	debug("migrate_handler fd %d events %x", c->device_state_fd, events);
+	if (c->device_state_fd < 0)
+		return;
 
-	if (events & EPOLLOUT)
-		rc = migrate_source(c, c->device_state_fd);
-	else if (events & EPOLLIN)
-		rc = migrate_target(c, c->device_state_fd);
+	debug("migrate_handler fd %d target %d",
+	      c->device_state_fd, c->migrate_target);
 
-	/* EPOLLHUP without EPOLLIN/EPOLLOUT, or EPOLLERR? Migration failed */
+	if (c->migrate_target)
+		rc = migrate_target(c, c->device_state_fd);
+	else
+		rc = migrate_source(c, c->device_state_fd);
 
 	migrate_close(c);
 
diff --git a/migrate.h b/migrate.h
index b97e55e7..3432940d 100644
--- a/migrate.h
+++ b/migrate.h
@@ -79,6 +79,6 @@ struct migrate_target_handlers {
 void migrate_init(struct ctx *c);
 void migrate_close(struct ctx *c);
 void migrate_request(struct ctx *c, int fd, bool target);
-void migrate_handler(struct ctx *c, uint32_t events);
+void migrate_handler(struct ctx *c);
 
 #endif /* MIGRATE_H */
diff --git a/passt.c b/passt.c
index 3c3a3313..1938290f 100644
--- a/passt.c
+++ b/passt.c
@@ -76,7 +76,6 @@ char *epoll_type_str[] = {
 	[EPOLL_TYPE_TAP_LISTEN]		= "listening qemu socket",
 	[EPOLL_TYPE_VHOST_CMD]		= "vhost-user command socket",
 	[EPOLL_TYPE_VHOST_KICK]		= "vhost-user kick socket",
-	[EPOLL_TYPE_DEVICE_STATE]	= "migration device state channel",
 	[EPOLL_TYPE_REPAIR_LISTEN]	= "TCP_REPAIR helper listening socket",
 	[EPOLL_TYPE_REPAIR]		= "TCP_REPAIR helper socket",
 };
@@ -360,9 +359,6 @@ loop:
 		case EPOLL_TYPE_VHOST_KICK:
 			vu_kick_cb(c.vdev, ref, &now);
 			break;
-		case EPOLL_TYPE_DEVICE_STATE:
-			migrate_handler(&c, eventmask);
-			break;
 		case EPOLL_TYPE_REPAIR_LISTEN:
 			repair_listen_handler(&c, eventmask);
 			break;
@@ -377,5 +373,7 @@ loop:
 
 	post_handler(&c, &now);
 
+	migrate_handler(&c);
+
 	goto loop;
 }
diff --git a/passt.h b/passt.h
index 5992cbeb..4189a4a5 100644
--- a/passt.h
+++ b/passt.h
@@ -241,6 +241,7 @@ struct ip6_ctx {
  * @vdev:		vhost-user device
  * @device_state_fd:	Device state migration channel
  * @device_state_result: Device state migration result
+ * @migrate_target:	Is this the target for next migration?
  */
 struct ctx {
 	enum passt_modes mode;
@@ -313,6 +314,7 @@ struct ctx {
 	/* Migration */
 	int device_state_fd;
 	int device_state_result;
+	bool migrate_target;
 };
 
 void proto_update_l2_buf(const unsigned char *eth_d,
-- 
@@ -241,6 +241,7 @@ struct ip6_ctx {
  * @vdev:		vhost-user device
  * @device_state_fd:	Device state migration channel
  * @device_state_result: Device state migration result
+ * @migrate_target:	Is this the target for next migration?
  */
 struct ctx {
 	enum passt_modes mode;
@@ -313,6 +314,7 @@ struct ctx {
 	/* Migration */
 	int device_state_fd;
 	int device_state_result;
+	bool migrate_target;
 };
 
 void proto_update_l2_buf(const unsigned char *eth_d,
-- 
2.48.1

Re: [PATCH 5/5] migrate: Don't handle the migration channel through epoll

[Stefano Brivio]

On Thu, 30 Jan 2025 19:33:30 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> --- a/migrate.c
> +++ b/migrate.c
> @@ -327,26 +327,6 @@ static int migrate_target(struct ctx *c, int fd)
>  	return rc;
>  }
>  
> -/**
> - * set_migration_watch() - Add the migration file descriptor to epoll
> - * @c:		Execution context
> - * @fd:		File descriptor to add
> - * @target:	Are we the target of the migration?
> - */
> -static void set_migration_watch(const struct ctx *c, int fd, bool target)
> -{
> -	union epoll_ref ref = {
> -		.type = EPOLL_TYPE_DEVICE_STATE,
> -		.fd = fd,
> -	 };
> -	struct epoll_event ev = { 0 };
> -
> -	ev.data.u64 = ref.u64;
> -	ev.events = target ? EPOLLIN : EPOLLOUT;
> -
> -	epoll_ctl(c->epollfd, EPOLL_CTL_ADD, ref.fd, &ev);

This change should have dropped:

		epoll_del(c, c->device_state_fd);

from migrate_close().

-- 
Stefano

Re: [PATCH 5/5] migrate: Don't handle the migration channel through epoll

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1327 bytes --]

On Fri, Jan 31, 2025 at 06:37:09AM +0100, Stefano Brivio wrote:
> On Thu, 30 Jan 2025 19:33:30 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > --- a/migrate.c
> > +++ b/migrate.c
> > @@ -327,26 +327,6 @@ static int migrate_target(struct ctx *c, int fd)
> >  	return rc;
> >  }
> >  
> > -/**
> > - * set_migration_watch() - Add the migration file descriptor to epoll
> > - * @c:		Execution context
> > - * @fd:		File descriptor to add
> > - * @target:	Are we the target of the migration?
> > - */
> > -static void set_migration_watch(const struct ctx *c, int fd, bool target)
> > -{
> > -	union epoll_ref ref = {
> > -		.type = EPOLL_TYPE_DEVICE_STATE,
> > -		.fd = fd,
> > -	 };
> > -	struct epoll_event ev = { 0 };
> > -
> > -	ev.data.u64 = ref.u64;
> > -	ev.events = target ? EPOLLIN : EPOLLOUT;
> > -
> > -	epoll_ctl(c->epollfd, EPOLL_CTL_ADD, ref.fd, &ev);
> 
> This change should have dropped:
> 
> 		epoll_del(c, c->device_state_fd);
> 
> from migrate_close().

Oops, yes.  I've fixed it in my local version, in case I need to
rebase before you've folded it all in.


-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/5] RFC: Assorted migration improvements

[Stefano Brivio]

On Thu, 30 Jan 2025 19:33:25 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> Here are some things that I think improve the draft migration logic.
> This is based on both the draft migration series (v2) and my
> epoll_del() series.
> 
> Feel free to fold into the existing patches if that seems sensible.

Sure, let me post a v3 with these included.

Do you prefer to keep all the patches as they are or should I merge 2/5
and 3/5 with the patches in my current series?

-- 
Stefano

Re: [PATCH 0/5] RFC: Assorted migration improvements

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 848 bytes --]

On Thu, Jan 30, 2025 at 09:46:19AM +0100, Stefano Brivio wrote:
> On Thu, 30 Jan 2025 19:33:25 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > Here are some things that I think improve the draft migration logic.
> > This is based on both the draft migration series (v2) and my
> > epoll_del() series.
> > 
> > Feel free to fold into the existing patches if that seems sensible.
> 
> Sure, let me post a v3 with these included.
> 
> Do you prefer to keep all the patches as they are or should I merge 2/5
> and 3/5 with the patches in my current series?

Whichever you prefer.  And 1/5 probably _should_ be folded in.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/5] RFC: Assorted migration improvements

[Stefano Brivio]

On Thu, 30 Jan 2025 19:55:11 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, Jan 30, 2025 at 09:46:19AM +0100, Stefano Brivio wrote:
> > On Thu, 30 Jan 2025 19:33:25 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > Here are some things that I think improve the draft migration logic.
> > > This is based on both the draft migration series (v2) and my
> > > epoll_del() series.
> > > 
> > > Feel free to fold into the existing patches if that seems sensible.  
> > 
> > Sure, let me post a v3 with these included.
> > 
> > Do you prefer to keep all the patches as they are or should I merge 2/5
> > and 3/5 with the patches in my current series?  
> 
> Whichever you prefer.  And 1/5 probably _should_ be folded in.

So, yeah, I have a merged series ready, but I'm trying to solve the
concrete problem I'm facing before posting it.

Actually, merging clean-ups with no _intended_ impact on functionality,
while trying to do this, isn't terribly helpful, but it didn't take
that long.

-- 
Stefano

Re: [PATCH 0/5] RFC: Assorted migration improvements

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]

On Fri, Jan 31, 2025 at 06:46:25AM +0100, Stefano Brivio wrote:
> On Thu, 30 Jan 2025 19:55:11 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Thu, Jan 30, 2025 at 09:46:19AM +0100, Stefano Brivio wrote:
> > > On Thu, 30 Jan 2025 19:33:25 +1100
> > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > >   
> > > > Here are some things that I think improve the draft migration logic.
> > > > This is based on both the draft migration series (v2) and my
> > > > epoll_del() series.
> > > > 
> > > > Feel free to fold into the existing patches if that seems sensible.  
> > > 
> > > Sure, let me post a v3 with these included.
> > > 
> > > Do you prefer to keep all the patches as they are or should I merge 2/5
> > > and 3/5 with the patches in my current series?  
> > 
> > Whichever you prefer.  And 1/5 probably _should_ be folded in.
> 
> So, yeah, I have a merged series ready, but I'm trying to solve the
> concrete problem I'm facing before posting it.
> 
> Actually, merging clean-ups with no _intended_ impact on functionality,
> while trying to do this, isn't terribly helpful, but it didn't take
> that long.

So, the cleanups aren't purely cleanups for the sake of cleanups:
they're things I think I need in preparation for the more compact flow
table representation stuff.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]