git: edfb339d3868 - main - bhyve/snapshot: switch to nvlist for snapshot requests

From: Robert Wing <rew_at_FreeBSD.org>
Date: Wed, 09 Feb 2022 17:22:14 UTC
The branch main has been updated by rew:

URL: https://cgit.FreeBSD.org/src/commit/?id=edfb339d3868bbd36393efb2738287e170767784

commit edfb339d3868bbd36393efb2738287e170767784
Author:     Robert Wing <rew@FreeBSD.org>
AuthorDate: 2022-02-09 17:11:57 +0000
Commit:     Robert Wing <rew@FreeBSD.org>
CommitDate: 2022-02-09 17:11:57 +0000

    bhyve/snapshot: switch to nvlist for snapshot requests
    
    Switch to using an nvlist with nvlist_send()/nvlist_recv() to
    communicate from bhyvectl(8) to bhyve(8).
    
    The idea is that a bhyve process receives a command with with a set of
    arguments. The nvlist here is structured to reflect that premise.
    
    For example, to snapshot the vm, the expected nvlist looks like:
    
        { cmd=START_CHECKPOINT, filename="filename" }
    
    Reviewed by:    markj
    Differential Revision:  https://reviews.freebsd.org/D33977
---
 usr.sbin/bhyve/snapshot.c    | 42 +++++++++++++++++++++---------------------
 usr.sbin/bhyve/snapshot.h    | 19 -------------------
 usr.sbin/bhyvectl/Makefile   |  1 +
 usr.sbin/bhyvectl/bhyvectl.c | 39 +++++++++++++++++++--------------------
 4 files changed, 41 insertions(+), 60 deletions(-)

diff --git a/usr.sbin/bhyve/snapshot.c b/usr.sbin/bhyve/snapshot.c
index 4c2cbed2c143..66f05ed68bcd 100644
--- a/usr.sbin/bhyve/snapshot.c
+++ b/usr.sbin/bhyve/snapshot.c
@@ -1325,7 +1325,7 @@ vm_vcpu_resume(struct vmctx *ctx)
 }
 
 static int
-vm_checkpoint(struct vmctx *ctx, char *checkpoint_file, bool stop_vm)
+vm_checkpoint(struct vmctx *ctx, const char *checkpoint_file, bool stop_vm)
 {
 	int fd_checkpoint = 0, kdata_fd = 0;
 	int ret = 0;
@@ -1440,17 +1440,23 @@ done:
 	return (error);
 }
 
-int
-handle_message(struct ipc_message *imsg, struct vmctx *ctx)
+static int
+handle_message(struct vmctx *ctx, nvlist_t *nvl)
 {
-	int err;
+	int err, cmd;
 
-	switch (imsg->code) {
-		case START_CHECKPOINT:
-			err = vm_checkpoint(ctx, imsg->data.op.snapshot_filename, false);
-			break;
+	if (!nvlist_exists_number(nvl, "cmd"))
+		return (-1);
+
+	cmd = nvlist_get_number(nvl, "cmd");
+	switch (cmd) {
 		case START_SUSPEND:
-			err = vm_checkpoint(ctx, imsg->data.op.snapshot_filename, true);
+		case START_CHECKPOINT:
+			if (!nvlist_exists_string(nvl, "filename"))
+				err = -1;
+			else
+				err = vm_checkpoint(ctx, nvlist_get_string(nvl, "filename"),
+				    cmd == START_SUSPEND ? true : false);
 			break;
 		default:
 			EPRINTLN("Unrecognized checkpoint operation\n");
@@ -1460,6 +1466,7 @@ handle_message(struct ipc_message *imsg, struct vmctx *ctx)
 	if (err != 0)
 		EPRINTLN("Unable to perform the requested operation\n");
 
+	nvlist_destroy(nvl);
 	return (err);
 }
 
@@ -1469,25 +1476,18 @@ handle_message(struct ipc_message *imsg, struct vmctx *ctx)
 void *
 checkpoint_thread(void *param)
 {
-	struct ipc_message imsg;
 	struct checkpoint_thread_info *thread_info;
-	ssize_t n;
+	nvlist_t *nvl;
 
 	pthread_set_name_np(pthread_self(), "checkpoint thread");
 	thread_info = (struct checkpoint_thread_info *)param;
 
 	for (;;) {
-		n = recvfrom(thread_info->socket_fd, &imsg, sizeof(imsg), 0, NULL, 0);
-
-		/*
-		 * slight sanity check: see if there's enough data to at
-		 * least determine the type of message.
-		 */
-		if (n >= sizeof(imsg.code))
-			handle_message(&imsg, thread_info->ctx);
+		nvl = nvlist_recv(thread_info->socket_fd, 0);
+		if (nvl != NULL)
+			handle_message(thread_info->ctx, nvl);
 		else
-			EPRINTLN("Failed to receive message: %s\n",
-			    n == -1 ? strerror(errno) : "unknown error");
+			EPRINTLN("nvlist_recv() failed: %s", strerror(errno));
 	}
 
 	return (NULL);
diff --git a/usr.sbin/bhyve/snapshot.h b/usr.sbin/bhyve/snapshot.h
index 3ffd42fbeabc..ddf23b8c0619 100644
--- a/usr.sbin/bhyve/snapshot.h
+++ b/usr.sbin/bhyve/snapshot.h
@@ -60,31 +60,12 @@ struct restore_state {
 	ucl_object_t *meta_root_obj;
 };
 
-/* Filename that will be used for save/restore */
-struct checkpoint_op {
-	char snapshot_filename[MAX_SNAPSHOT_FILENAME];
-};
-
 /* Messages that a bhyve process understands. */
 enum ipc_opcode {
 	START_CHECKPOINT,
 	START_SUSPEND,
 };
 
-/*
- * The type of message and associated data to
- * send to a bhyve process.
- */
-struct ipc_message {
-        enum ipc_opcode code;
-        union {
-                /*
-                 * message specific structures
-                 */
-                struct checkpoint_op op;
-        } data;
-};
-
 struct checkpoint_thread_info {
 	struct vmctx *ctx;
 	int socket_fd;
diff --git a/usr.sbin/bhyvectl/Makefile b/usr.sbin/bhyvectl/Makefile
index 12fdb706cadb..c004d603c8db 100644
--- a/usr.sbin/bhyvectl/Makefile
+++ b/usr.sbin/bhyvectl/Makefile
@@ -17,6 +17,7 @@ WARNS?=	3
 CFLAGS+= -I${SRCTOP}/sys/amd64/vmm
 
 .if ${MK_BHYVE_SNAPSHOT} != "no"
+LIBADD+= nv
 CFLAGS+= -DBHYVE_SNAPSHOT
 
 # usr.sbin/bhyve/snapshot.h needs ucl header
diff --git a/usr.sbin/bhyvectl/bhyvectl.c b/usr.sbin/bhyvectl/bhyvectl.c
index 017427db4d4f..560a3a3eb443 100644
--- a/usr.sbin/bhyvectl/bhyvectl.c
+++ b/usr.sbin/bhyvectl/bhyvectl.c
@@ -33,10 +33,13 @@ __FBSDID("$FreeBSD$");
 
 #include <sys/param.h>
 #include <sys/types.h>
-#include <sys/sysctl.h>
+#include <sys/cpuset.h>
 #include <sys/errno.h>
 #include <sys/mman.h>
-#include <sys/cpuset.h>
+#include <sys/nv.h>
+#include <sys/socket.h>
+#include <sys/sysctl.h>
+#include <sys/un.h>
 
 #include <stdio.h>
 #include <stdlib.h>
@@ -57,9 +60,6 @@ __FBSDID("$FreeBSD$");
 #include <machine/vmm_dev.h>
 #include <vmmapi.h>
 
-#include <sys/socket.h>
-#include <sys/un.h>
-
 #include "amd/vmcb.h"
 #include "intel/vmcs.h"
 
@@ -1684,10 +1684,9 @@ show_memseg(struct vmctx *ctx)
 
 #ifdef BHYVE_SNAPSHOT
 static int
-send_message(struct vmctx *ctx, void *data, size_t len)
+send_message(struct vmctx *ctx, nvlist_t *nvl)
 {
 	struct sockaddr_un addr;
-	ssize_t len_sent;
 	int err, socket_fd;
 	char vmname_buf[MAX_VMNAME];
 
@@ -1709,14 +1708,16 @@ send_message(struct vmctx *ctx, void *data, size_t len)
 
 	snprintf(addr.sun_path, sizeof(addr.sun_path), "%s%s", BHYVE_RUN_DIR, vmname_buf);
 
-	len_sent = sendto(socket_fd, data, len, 0,
-	    (struct sockaddr *)&addr, sizeof(struct sockaddr_un));
-
-	if (len_sent < 0) {
-		perror("Failed to send message to bhyve vm");
-		err = -1;
+	if (connect(socket_fd, (struct sockaddr *)&addr, SUN_LEN(&addr)) != 0) {
+		perror("connect() failed");
+		err = errno;
+		goto done;
 	}
 
+	if (nvlist_send(socket_fd, nvl) < 0)
+		perror("nvlist_send() failed");
+	nvlist_destroy(nvl);
+
 done:
 	if (socket_fd > 0)
 		close(socket_fd);
@@ -1726,15 +1727,13 @@ done:
 static int
 snapshot_request(struct vmctx *ctx, const char *file, enum ipc_opcode code)
 {
-	struct ipc_message imsg;
-	size_t length;
-
-	imsg.code = code;
-	strlcpy(imsg.data.op.snapshot_filename, file, MAX_SNAPSHOT_FILENAME);
+	nvlist_t *nvl;
 
-	length = offsetof(struct ipc_message, data) + sizeof(imsg.data.op);
+	nvl = nvlist_create(0);
+	nvlist_add_number(nvl, "cmd", code);
+	nvlist_add_string(nvl, "filename", file);
 
-	return (send_message(ctx, (void *)&imsg, length));
+	return (send_message(ctx, nvl));
 }
 #endif