[PATCH 6/6] migrate: Make migration handlers simpler and more flexible

[David Gibson <david@gibson.dropbear.id.au>]

Currently the structure of the migration callbacks is quite rigid.  There
are separate tables for pre and post handlers, for source and target.
Those only prepare or fix up with no reading or writing of the stream.

The actual reading and writing is done with an iovec of buffers to
transfer.  That can't handle any variably sized structures, nor sending
state which is only obtained during migration rather than being tracked
usually.

Replace both the handlers and the sections with an ordered list of
migration "stages".  Each stage has a callback for both source and target
side doing whatever is necessary - these can be NULL, for example for
preparation steps that have no equivalent on the other side.  Things that
are just buffers to be transferred are handled with a macro generating a
suitable stage entry.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 flow.c    |  14 ++-
 flow.h    |   6 +-
 migrate.c | 271 ++++++++++++++++++------------------------------------
 migrate.h |  54 ++++++-----
 4 files changed, 138 insertions(+), 207 deletions(-)

diff --git a/flow.c b/flow.c
index ceb20183..812b6a2b 100644
--- a/flow.c
+++ b/flow.c
@@ -875,15 +875,20 @@ void flow_defer_handler(const struct ctx *c, const struct timespec *now)
  * flow_migrate_source_pre() - Prepare all source flows for migration
  * @c:		Execution context
  * @m:		Migration metadata
+ * @stage:	Migration stage information (unused)
+ * @fd:		Migration fd (unused)
  *
  * Return: 0 on success
  */
-int flow_migrate_source_pre(struct ctx *c, struct migrate_meta *m)
+int flow_migrate_source_pre(struct ctx *c, struct migrate_meta *m,
+			    const struct migrate_stage *stage, int fd)
 {
 	unsigned i;
 	int rc;
 
 	(void)m;
+	(void)stage;
+	(void)fd;
 
 	for (i = 0; i < FLOW_MAX; i++) {	/* TODO: iterator with skip */
 		union flow *flow = &flowtab[i];
@@ -918,15 +923,20 @@ int flow_migrate_source_pre(struct ctx *c, struct migrate_meta *m)
  * flow_migrate_target_post() - Restore all flows after migration
  * @c:		Execution context
  * @m:		Migration metadata
+ * @stage:	Migration stage (unused)
+ * @fd:		Migration fd (unused)
  *
  * Return: 0 on success
  */
-int flow_migrate_target_post(struct ctx *c, struct migrate_meta *m)
+int flow_migrate_target_post(struct ctx *c, struct migrate_meta *m,
+			     const struct migrate_stage *stage, int fd)
 {
 	unsigned i;
 	int rc;
 
 	(void)m;
+	(void)stage;
+	(void)fd;
 
 	for (i = 0; i < FLOW_MAX; i++) {	/* TODO: iterator with skip */
 		union flow *flow = &flowtab[i];
diff --git a/flow.h b/flow.h
index 43fb5078..f1734008 100644
--- a/flow.h
+++ b/flow.h
@@ -255,8 +255,10 @@ union flow;
 
 void flow_init(void);
 void flow_defer_handler(const struct ctx *c, const struct timespec *now);
-int flow_migrate_source_pre(struct ctx *c, struct migrate_meta *m);
-int flow_migrate_target_post(struct ctx *c, struct migrate_meta *m);
+int flow_migrate_source_pre(struct ctx *c, struct migrate_meta *m,
+			    const struct migrate_stage *stage, int fd);
+int flow_migrate_target_post(struct ctx *c, struct migrate_meta *m,
+			     const struct migrate_stage *stage, int fd);
 
 void flow_log_(const struct flow_common *f, int pri, const char *fmt, ...)
 	__attribute__((format(printf, 3, 4)));
diff --git a/migrate.c b/migrate.c
index 025f3b71..9f34eb58 100644
--- a/migrate.c
+++ b/migrate.c
@@ -48,116 +48,82 @@ static union migrate_header header = {
 	.voidp_size	= htonl_constant(sizeof(void *)),
 };
 
-/* Data sections for version 1 */
-static struct iovec sections_v1[] = {
-	{ &flow_first_free,	sizeof(flow_first_free) },
-	{ flowtab,		sizeof(flowtab) },
-	{ flow_hashtab,		sizeof(flow_hashtab) },
-	{ g_hash_secret,	sizeof(g_hash_secret) },
-	{ 0 },
-};
-
-/* Set of data versions */
-static struct migrate_data data_versions[] = {
-	{
-		1,	sections_v1,
-	},
-	{ 0 },
-};
-
-/* Handlers to call in source before sending data */
-struct migrate_handler handlers_source_pre[] = {
-	{ flow_migrate_source_pre },
-	{ 0 },
-};
-
-/* Handlers to call in source after sending data */
-struct migrate_handler handlers_source_post[] = {
-	{ 0 },
-};
-
-/* Handlers to call in target before receiving data with version 1 */
-struct migrate_handler handlers_target_pre_v1[] = {
-	{ 0 },
-};
-
-/* Handlers to call in target after receiving data with version 1 */
-struct migrate_handler handlers_target_post_v1[] = {
-	{ flow_migrate_target_post },
-	{ 0 },
-};
-
-/* Versioned sets of migration handlers */
-struct migrate_target_handlers target_handlers[] = {
-	{
-		1,
-		handlers_target_pre_v1,
-		handlers_target_post_v1,
-	},
-	{ 0 },
-};
-
 /**
- * migrate_source_pre() - Pre-migration tasks as source
+ * migrate_send_block() - Migration stage handler to send verbatim data
  * @c:		Execution context
  * @m:		Migration metadata
+ * @stage:	Migration stage
+ * @fd:		Migration fd
  *
- * Return: 0 on success, error code on failure
+ * Sends the buffer in @stage->iov over the migration channel.
  */
-static int migrate_source_pre(struct ctx *c, struct migrate_meta *m)
+static int migrate_send_block(struct ctx *c, struct migrate_meta *m,
+			      const struct migrate_stage *stage, int fd)
 {
-	struct migrate_handler *h;
-
-	for (h = handlers_source_pre; h->fn; h++) {
-		int rc;
+	(void)c;
+	(void)m;
 
-		if ((rc = h->fn(c, m)))
-			return rc;
-	}
+	if (write_remainder(fd, &stage->iov, 1, 0) < 0)
+		return errno;
 
 	return 0;
 }
 
 /**
- * migrate_source_state() - Send device state as migration source
- * @fd:		Descriptor for state transfer
+ * migrate_recv_block() - Migration stage handler to receive verbatim data
+ * @c:		Execution context
  * @m:		Migration metadata
+ * @stage:	Migration stage
+ * @fd:		Migration fd
  *
- * Return: 0 on success, error code on failure
+ * Reads the buffer in @stage->iov from the migration channel.
+ *
+ * #syscalls:vu readv
  */
-static int migrate_source_state(int fd, const struct migrate_meta *m)
+static int migrate_recv_block(struct ctx *c, struct migrate_meta *m,
+			      const struct migrate_stage *stage, int fd)
 {
-	static struct migrate_data *d;
-	int count, rc;
-
+	(void)c;
 	(void)m;
 
-	for (d = data_versions; d->v != MIGRATE_VERSION; d++);
-
-	for (count = 0; d->sections[count].iov_len; count++);
-
-	debug("Writing %u migration sections", count - 1 /* minus header */);
-	rc = write_remainder(fd, d->sections, count, 0);
-	if (rc < 0)
+	if (read_remainder(fd, &stage->iov, 1, 0) < 0)
 		return errno;
 
 	return 0;
 }
 
-/**
- * migrate_source_post() - Post-migration tasks as source
- * @c:		Execution context
- * @m:		Migration metadata
- *
- * Return: 0 on success, error code on failure
- */
-static void migrate_source_post(struct ctx *c, struct migrate_meta *m)
-{
-	struct migrate_handler *h;
+#define DATA_STAGE(v) \
+	{					\
+		.name = #v,			\
+		.source = migrate_send_block,	\
+		.target = migrate_recv_block,	\
+		.iov = { &(v), sizeof(v) },	\
+	}
 
-	for (h = handlers_source_post; h->fn; h++)
-		h->fn(c, m);
-}
+/* Stages for version 1 */
+static const struct migrate_stage stages_v1[] = {
+	{
+		.name = "flow pre",
+		.source = flow_migrate_source_pre,
+		.target = NULL,
+	},
+	DATA_STAGE(flow_first_free),
+	DATA_STAGE(flowtab),
+	DATA_STAGE(flow_hashtab),
+	DATA_STAGE(g_hash_secret),
+	{
+		.name = "flow post",
+		.source = NULL,
+		.target = flow_migrate_target_post,
+	},
+};
+
+/* Set of data versions */
+static const struct migrate_version versions[] = {
+	{
+		1,	stages_v1,	ARRAY_SIZE(stages_v1),
+	},
+};
 
 /**
  * migrate_source() - Migration as source, send state to hypervisor
@@ -169,6 +135,7 @@ static void migrate_source_post(struct ctx *c, struct migrate_meta *m)
 static int migrate_source(struct ctx *c, int fd)
 {
 	struct migrate_meta m;
+	unsigned i;
 	int rc;
 
 	rc = write_all_buf(fd, &header, sizeof(header));
@@ -177,20 +144,22 @@ static int migrate_source(struct ctx *c, int fd)
 		return rc;
 	}
 
-	if ((rc = migrate_source_pre(c, &m))) {
-		err("Source pre-migration failed: %s, abort", strerror_(rc));
-		return rc;
-	}
+	for (i = 0; i < ARRAY_SIZE(stages_v1); i++) {
+		const struct migrate_stage *stage = &stages_v1[i];
 
-	debug("Saving backend state");
+		if (!stage->source)
+			continue;
 
-	rc = migrate_source_state(fd, &m);
-	if (rc)
-		err("Source migration failed: %s", strerror_(rc));
-	else
-		migrate_source_post(c, &m);
+		debug("Source side migration: %s", stage->name);
 
-	return rc;
+		if ((rc = stage->source(c, &m, stage, fd))) {
+			err("Source migration failed stage %s: %s, abort",
+			    stage->name, strerror_(rc));
+			return rc;
+		}
+	}
+
+	return 0;
 }
 
 /**
@@ -202,8 +171,8 @@ static int migrate_source(struct ctx *c, int fd)
  */
 static int migrate_target_read_header(int fd, struct migrate_meta *m)
 {
-	static struct migrate_data *d;
 	union migrate_header h;
+	unsigned i;
 
 	if (read_all_buf(fd, &h, sizeof(h)))
 		return errno;
@@ -211,10 +180,14 @@ static int migrate_target_read_header(int fd, struct migrate_meta *m)
 	debug("Source magic: 0x%016" PRIx64 ", sizeof(void *): %u, version: %u",
 	      h.magic, ntohl(h.voidp_size), ntohl(h.version));
 
-	for (d = data_versions; d->v != ntohl(h.version) && d->v; d++);
-	if (!d->v)
+	m->version = ntohl(h.version);
+	m->v = NULL;
+	for (i = 0; i < ARRAY_SIZE(versions); i++) {
+		if (versions[i].version == m->version)
+			m->v = &versions[i];
+	}
+	if (!m->v)
 		return ENOTSUP;
-	m->v = d->v;
 
 	if (h.magic == MIGRATE_MAGIC)
 		m->bswap = false;
@@ -243,75 +216,6 @@ static int migrate_target_read_header(int fd, struct migrate_meta *m)
 	return 0;
 }
 
-/**
- * migrate_target_pre() - Pre-migration tasks as target
- * @c:		Execution context
- * @m:		Migration metadata
- *
- * Return: 0 on success, error code on failure
- */
-static int migrate_target_pre(struct ctx *c, struct migrate_meta *m)
-{
-	struct migrate_target_handlers *th;
-	struct migrate_handler *h;
-
-	for (th = target_handlers; th->v != m->v && th->v; th++);
-
-	for (h = th->pre; h->fn; h++) {
-		int rc;
-
-		if ((rc = h->fn(c, m)))
-			return rc;
-	}
-
-	return 0;
-}
-
-/**
- * migrate_target_state() - Receive device state as migration target
- * @fd:		Descriptor for state transfer
- * @m:		Migration metadata
- *
- * Return: 0 on success, error code on failure
- *
- * #syscalls:vu readv
- */
-static int migrate_target_state(int fd, const struct migrate_meta *m)
-{
-	static struct migrate_data *d;
-	unsigned cnt;
-	int rc;
-
-	for (d = data_versions; d->v != m->v && d->v; d++);
-
-	for (cnt = 0; d->sections[cnt].iov_len; cnt++);
-
-	debug("Reading %u migration sections", cnt);
-	rc = read_remainder(fd, d->sections, cnt, 0);
-	if (rc < 0)
-		return errno;
-
-	return 0;
-}
-
-/**
- * migrate_target_post() - Post-migration tasks as target
- * @c:		Execution context
- * @m:		Migration metadata
- */
-static void migrate_target_post(struct ctx *c, struct migrate_meta *m)
-{
-	struct migrate_target_handlers *th;
-	struct migrate_handler *h;
-
-	memcpy(c->hash_secret, g_hash_secret, sizeof(g_hash_secret));
-
-	for (th = target_handlers; th->v != m->v && th->v; th++);
-
-	for (h = th->post; h->fn; h++)
-		h->fn(c, m);
-}
-
 /**
  * migrate_target() - Migration as target, receive state from hypervisor
  * @c:		Execution context
@@ -322,6 +226,7 @@ static void migrate_target_post(struct ctx *c, struct migrate_meta *m)
 static int migrate_target(struct ctx *c, int fd)
 {
 	struct migrate_meta m;
+	unsigned i;
 	int rc;
 
 	rc = migrate_target_read_header(fd, &m);
@@ -330,20 +235,22 @@ static int migrate_target(struct ctx *c, int fd)
 		return rc;
 	}
 
-	if ((rc = migrate_target_pre(c, &m))) {
-		err("Target pre-migration failed: %s, abort", strerror_(rc));
-		return rc;
-	}
-
-	debug("Loading backend state");
+	for (i = 0; i < m.v->nstages; i++) {
+		const struct migrate_stage *stage = &m.v->stages[i];
 
-	rc = migrate_target_state(fd, &m);
-	if (rc)
-		err("Target migration failed: %s", strerror_(rc));
-	else
-		migrate_target_post(c, &m);
+		if (!stage->target)
+			continue;
 
-	return rc;
+		debug("Target side migration: %s", stage->name);
+		
+		if ((rc = stage->target(c, &m, stage, fd))) {
+			err("Target migration failed stage %s: %s, abort",
+			    stage->name, strerror_(rc));
+			return rc;
+		}
+	}
+	
+	return 0;
 }
 
 /**
diff --git a/migrate.h b/migrate.h
index 158241f3..32c0b3f4 100644
--- a/migrate.h
+++ b/migrate.h
@@ -6,9 +6,12 @@
 #ifndef MIGRATE_H
 #define MIGRATE_H
 
+struct migrate_version;
+
 /**
  * struct migrate_meta - Migration metadata
- * @v:			Chosen migration data version, host order
+ * @version:		Chosen migration data version, host order
+ * @v:			Migration version information
  * @bswap:		Source has opposite endianness
  * @peer_64b:		Source uses 64-bit void *
  * @time_64b:		Source uses 64-bit time_t
@@ -16,7 +19,8 @@
  * @flow_sidx_size:	Size of struct flow_sidx in source
  */
 struct migrate_meta {
-	uint32_t v;
+	uint32_t version;
+	const struct migrate_version *v;
 	bool bswap;
 	bool source_64b;
 	bool time_64b;
@@ -46,34 +50,42 @@ union migrate_header {
 	uint8_t unused[4096];
 } __attribute__((packed));
 
+struct migrate_stage;
+
 /**
- * struct migrate_data - Data sections for given source version
- * @v:			Source version this applies to, host order
- * @sections:		Array of data sections, NULL-terminated
+ * migrate_cb_t - Callback function to implement one migration stage
  */
-struct migrate_data {
-	uint32_t v;
-	struct iovec *sections;
-};
+typedef int (*migrate_cb_t)(struct ctx *c, struct migrate_meta *m,
+			    const struct migrate_stage *stage, int fd);
 
 /**
- * struct migrate_handler - Function to handle a specific data section
- * @fn:			Function pointer taking pointer to context and metadata
+ * struct migrate_stage - Callbacks and parameters for one stage of migration
+ * @name:	Stage name (for debugging)
+ * @source:	Callback to implement this stage on the source
+ * @target:	Callback to implement this stage on the target
+ * @v:		Source version this applies to, host order
+ * @sections:	Array of data sections, NULL-terminated
  */
-struct migrate_handler {
-	int (*fn)(struct ctx *c, struct migrate_meta *m);
+struct migrate_stage {
+	const char *name;
+	migrate_cb_t source;
+	migrate_cb_t target;
+	/* FIXME: rollback callbacks? */
+	union {
+		struct iovec iov;
+	};
 };
 
 /**
- * struct migrate_target_handlers - Versioned sets of migration target handlers
- * @v:			Source version this applies to, host order
- * @pre:		Set of functions to execute in target before data copy
- * @post:		Set of functions to execute in target after data copy
+ * struct migrate_version - Stages for a particular protocol version
+ * @version:	Version number this implements
+ * @stages:	Ordered array of stages
+ * @nstages:	Length of @stages
  */
-struct migrate_target_handlers {
-	uint32_t v;
-	struct migrate_handler *pre;
-	struct migrate_handler *post;
+struct migrate_version {
+	uint32_t version;
+	const struct migrate_stage *stages;
+	unsigned nstages;
 };
 
 void migrate_init(struct ctx *c);
-- 
@@ -6,9 +6,12 @@
 #ifndef MIGRATE_H
 #define MIGRATE_H
 
+struct migrate_version;
+
 /**
  * struct migrate_meta - Migration metadata
- * @v:			Chosen migration data version, host order
+ * @version:		Chosen migration data version, host order
+ * @v:			Migration version information
  * @bswap:		Source has opposite endianness
  * @peer_64b:		Source uses 64-bit void *
  * @time_64b:		Source uses 64-bit time_t
@@ -16,7 +19,8 @@
  * @flow_sidx_size:	Size of struct flow_sidx in source
  */
 struct migrate_meta {
-	uint32_t v;
+	uint32_t version;
+	const struct migrate_version *v;
 	bool bswap;
 	bool source_64b;
 	bool time_64b;
@@ -46,34 +50,42 @@ union migrate_header {
 	uint8_t unused[4096];
 } __attribute__((packed));
 
+struct migrate_stage;
+
 /**
- * struct migrate_data - Data sections for given source version
- * @v:			Source version this applies to, host order
- * @sections:		Array of data sections, NULL-terminated
+ * migrate_cb_t - Callback function to implement one migration stage
  */
-struct migrate_data {
-	uint32_t v;
-	struct iovec *sections;
-};
+typedef int (*migrate_cb_t)(struct ctx *c, struct migrate_meta *m,
+			    const struct migrate_stage *stage, int fd);
 
 /**
- * struct migrate_handler - Function to handle a specific data section
- * @fn:			Function pointer taking pointer to context and metadata
+ * struct migrate_stage - Callbacks and parameters for one stage of migration
+ * @name:	Stage name (for debugging)
+ * @source:	Callback to implement this stage on the source
+ * @target:	Callback to implement this stage on the target
+ * @v:		Source version this applies to, host order
+ * @sections:	Array of data sections, NULL-terminated
  */
-struct migrate_handler {
-	int (*fn)(struct ctx *c, struct migrate_meta *m);
+struct migrate_stage {
+	const char *name;
+	migrate_cb_t source;
+	migrate_cb_t target;
+	/* FIXME: rollback callbacks? */
+	union {
+		struct iovec iov;
+	};
 };
 
 /**
- * struct migrate_target_handlers - Versioned sets of migration target handlers
- * @v:			Source version this applies to, host order
- * @pre:		Set of functions to execute in target before data copy
- * @post:		Set of functions to execute in target after data copy
+ * struct migrate_version - Stages for a particular protocol version
+ * @version:	Version number this implements
+ * @stages:	Ordered array of stages
+ * @nstages:	Length of @stages
  */
-struct migrate_target_handlers {
-	uint32_t v;
-	struct migrate_handler *pre;
-	struct migrate_handler *post;
+struct migrate_version {
+	uint32_t version;
+	const struct migrate_stage *stages;
+	unsigned nstages;
 };
 
 void migrate_init(struct ctx *c);
-- 
2.48.1