git: e96f9fa135a2 - stable/13 - bhyve/snapshot: use SOCK_DGRAM instead of SOCK_STREAM

From: John Baldwin <jhb_at_FreeBSD.org>
Date: Thu, 26 Jan 2023 18:54:20 UTC
The branch stable/13 has been updated by jhb:

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

commit e96f9fa135a24befbc7ea611f38848d83bff9a4d
Author:     Robert Wing <rew@FreeBSD.org>
AuthorDate: 2021-03-08 00:23:29 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2023-01-26 18:46:01 +0000

    bhyve/snapshot: use SOCK_DGRAM instead of SOCK_STREAM
    
    The save/restore feature uses a unix domain socket to send messages
    from bhyvectl(8) to a bhyve(8) process. A datagram socket will suffice
    for this.
    
    An added benefit of using a datagram socket is simplified code. For
    bhyve, the listen/accept calls are dropped; and for bhyvectl, the
    connect() call is dropped.
    
    EPRINTLN handles raw mode for bhyve(8), use it to print error messages.
    
    Reviewed by:    jhb
    Differential Revision:  https://reviews.freebsd.org/D28983
    
    (cherry picked from commit 38dfb0626fd35c64b2e2d5faae2c90e7981a3307)
---
 usr.sbin/bhyve/snapshot.c    | 65 +++++++++++++++++---------------------------
 usr.sbin/bhyvectl/bhyvectl.c | 22 ++++-----------
 2 files changed, 31 insertions(+), 56 deletions(-)

diff --git a/usr.sbin/bhyve/snapshot.c b/usr.sbin/bhyve/snapshot.c
index 2938a0e84c88..061ba11096e9 100644
--- a/usr.sbin/bhyve/snapshot.c
+++ b/usr.sbin/bhyve/snapshot.c
@@ -79,6 +79,7 @@ __FBSDID("$FreeBSD$");
 #include "bhyverun.h"
 #include "acpi.h"
 #include "atkbdc.h"
+#include "debug.h"
 #include "inout.h"
 #include "fwctl.h"
 #include "ioapic.h"
@@ -117,8 +118,6 @@ static sig_t old_winch_handler;
 
 #define	MAX_VMNAME 100
 
-#define	MAX_MSG_SIZE 1024
-
 #define	SNAPSHOT_BUFFER_SIZE (20 * MB)
 
 #define	JSON_STRUCT_ARR_KEY		"structs"
@@ -1442,24 +1441,10 @@ done:
 }
 
 int
-get_checkpoint_msg(int conn_fd, struct vmctx *ctx)
+handle_message(struct checkpoint_op *checkpoint_op, struct vmctx *ctx)
 {
-	unsigned char buf[MAX_MSG_SIZE];
-	struct checkpoint_op *checkpoint_op;
-	int len, recv_len, total_recv = 0;
-	int err = 0;
-
-	len = sizeof(struct checkpoint_op); /* expected length */
-	while ((recv_len = recv(conn_fd, buf + total_recv, len - total_recv, 0)) > 0) {
-		total_recv += recv_len;
-	}
-	if (recv_len < 0) {
-		perror("Error while receiving data from bhyvectl");
-		err = -1;
-		goto done;
-	}
+	int err;
 
-	checkpoint_op = (struct checkpoint_op *)buf;
 	switch (checkpoint_op->op) {
 		case START_CHECKPOINT:
 			err = vm_checkpoint(ctx, checkpoint_op->snapshot_filename, false);
@@ -1468,12 +1453,13 @@ get_checkpoint_msg(int conn_fd, struct vmctx *ctx)
 			err = vm_checkpoint(ctx, checkpoint_op->snapshot_filename, true);
 			break;
 		default:
-			fprintf(stderr, "Unrecognized checkpoint operation.\n");
+			EPRINTLN("Unrecognized checkpoint operation\n");
 			err = -1;
 	}
 
-done:
-	close(conn_fd);
+	if (err != 0)
+		EPRINTLN("Unable to perform the requested operation\n");
+
 	return (err);
 }
 
@@ -1483,21 +1469,25 @@ done:
 void *
 checkpoint_thread(void *param)
 {
+	struct checkpoint_op op;
 	struct checkpoint_thread_info *thread_info;
-	int conn_fd, ret;
+	ssize_t n;
 
 	pthread_set_name_np(pthread_self(), "checkpoint thread");
 	thread_info = (struct checkpoint_thread_info *)param;
 
-	while ((conn_fd = accept(thread_info->socket_fd, NULL, NULL)) > -1) {
-		ret = get_checkpoint_msg(conn_fd, thread_info->ctx);
-		if (ret != 0) {
-			fprintf(stderr, "Failed to read message on checkpoint "
-					"socket. Retrying.\n");
-		}
-	}
-	if (conn_fd < -1) {
-		perror("Failed to accept connection");
+	for (;;) {
+		n = recvfrom(thread_info->socket_fd, &op, sizeof(op), 0, NULL, 0);
+
+		/*
+		 * slight sanity check: see if there's enough data to at
+		 * least determine the type of message.
+		 */
+		if (n >= sizeof(op.op))
+			handle_message(&op, thread_info->ctx);
+		else
+			EPRINTLN("Failed to receive message: %s\n",
+			    n == -1 ? strerror(errno) : "unknown error");
 	}
 
 	return (NULL);
@@ -1527,9 +1517,9 @@ init_checkpoint_thread(struct vmctx *ctx)
 	if (err != 0)
 		errc(1, err, "checkpoint cv init");
 
-	socket_fd = socket(PF_UNIX, SOCK_STREAM, 0);
+	socket_fd = socket(PF_UNIX, SOCK_DGRAM, 0);
 	if (socket_fd < 0) {
-		perror("Socket creation failed (IPC with bhyvectl");
+		EPRINTLN("Socket creation failed: %s", strerror(errno));
 		err = -1;
 		goto fail;
 	}
@@ -1548,13 +1538,8 @@ init_checkpoint_thread(struct vmctx *ctx)
 	unlink(addr.sun_path);
 
 	if (bind(socket_fd, (struct sockaddr *)&addr, addr.sun_len) != 0) {
-		perror("Failed to bind socket (IPC with bhyvectl)");
-		err = -1;
-		goto fail;
-	}
-
-	if (listen(socket_fd, 10) < 0) {
-		perror("Failed to listen on socket (IPC with bhyvectl)");
+		EPRINTLN("Failed to bind socket \"%s\": %s\n",
+		    addr.sun_path, strerror(errno));
 		err = -1;
 		goto fail;
 	}
diff --git a/usr.sbin/bhyvectl/bhyvectl.c b/usr.sbin/bhyvectl/bhyvectl.c
index 0f7b9533fe4b..df02f7caf345 100644
--- a/usr.sbin/bhyvectl/bhyvectl.c
+++ b/usr.sbin/bhyvectl/bhyvectl.c
@@ -1687,11 +1687,11 @@ static int
 send_checkpoint_op_req(struct vmctx *ctx, struct checkpoint_op *op)
 {
 	struct sockaddr_un addr;
-	int socket_fd, len, len_sent, total_sent;
-	int err = 0;
+	ssize_t len_sent;
+	int err, socket_fd;
 	char vmname_buf[MAX_VMNAME];
 
-	socket_fd = socket(PF_UNIX, SOCK_STREAM, 0);
+	socket_fd = socket(PF_UNIX, SOCK_DGRAM, 0);
 	if (socket_fd < 0) {
 		perror("Error creating bhyvectl socket");
 		err = -1;
@@ -1709,21 +1709,11 @@ send_checkpoint_op_req(struct vmctx *ctx, struct checkpoint_op *op)
 
 	snprintf(addr.sun_path, sizeof(addr.sun_path), "%s%s", BHYVE_RUN_DIR, vmname_buf);
 
-	if (connect(socket_fd, (struct sockaddr *)&addr,
-			sizeof(struct sockaddr_un)) != 0) {
-		perror("Connect to VM socket failed");
-		err = -1;
-		goto done;
-	}
-
-	len = sizeof(*op);
-	total_sent = 0;
-	while ((len_sent = send(socket_fd, (char *)op + total_sent, len - total_sent, 0)) > 0) {
-		total_sent += len_sent;
-	}
+	len_sent = sendto(socket_fd, op, sizeof(*op), 0,
+	    (struct sockaddr *)&addr, sizeof(struct sockaddr_un));
 
 	if (len_sent < 0) {
-		perror("Failed to send checkpoint operation request");
+		perror("Failed to send message to bhyve vm");
 		err = -1;
 	}