MFC 308201

    hyperv/kvp: Don't mix message status codes and function return values.

    While I'm here, move message status codes to hv_utilreg.h, since they
    will be used by the upcoming VSS stuffs.

    Submitted by:   Hongjiang Zhang <honzhan microsoft com>
    Sponsored by:   Microsoft
    Differential Revision:  https://reviews.freebsd.org/D8391

4 files changed, 71 insertions, 69 deletions

diff --git a/contrib/hyperv/tools/hv_kvp_daemon.c b/contrib/hyperv/tools/hv_kvp_daemon.c
index 8897957..baff21c 100644
--- a/contrib/hyperv/tools/hv_kvp_daemon.c
+++ b/contrib/hyperv/tools/hv_kvp_daemon.c
@@ -52,9 +52,10 @@
 #include <string.h>
 #include <syslog.h>
 #include <unistd.h>
+#include <assert.h>
 
 #include "hv_kvp.h"
-
+#include "hv_utilreg.h"
 typedef uint8_t		__u8;
 typedef uint16_t	__u16;
 typedef uint32_t	__u32;
@@ -684,18 +685,16 @@ kvp_get_ipconfig_info(char *if_name, struct hv_kvp_ipaddr_value *buffer)
 	 */
 	kvp_process_ipconfig_file(cmd, (char *)buffer->gate_way,
 	    (MAX_GATEWAY_SIZE * 2), INET_ADDRSTRLEN, 0);
-
 	/*
 	 * Retrieve the IPV6 address of default gateway.
 	 */
-	snprintf(cmd, sizeof(cmd), "netstat -rn inet6 | grep %s | awk '/default/ {print $2 }", if_name);
+	snprintf(cmd, sizeof(cmd), "netstat -rn inet6 | grep %s | awk '/default/ {print $2 }'", if_name);
 
 	/*
 	 * Execute the command to gather gateway IPV6 info.
 	 */
 	kvp_process_ipconfig_file(cmd, (char *)buffer->gate_way,
 	    (MAX_GATEWAY_SIZE * 2), INET6_ADDRSTRLEN, 1);
-
 	/*
 	 * we just invoke an external script to get the DNS info.
 	 *
@@ -782,11 +781,11 @@ kvp_process_ip_address(void *addrp,
 	}
 
 	if ((length - *offset) < addr_length + 1) {
-		return (HV_KVP_E_FAIL);
+		return (EINVAL);
 	}
 	if (str == NULL) {
 		strlcpy(buffer, "inet_ntop failed\n", length);
-		return (HV_KVP_E_FAIL);
+		return (errno);
 	}
 	if (*offset == 0) {
 		strlcpy(buffer, tmp, length);
@@ -832,7 +831,7 @@ kvp_get_ip_info(int family, char *if_name, int op,
 
 	if (getifaddrs(&ifap)) {
 		strlcpy(buffer, "getifaddrs failed\n", buffer_length);
-		return (HV_KVP_E_FAIL);
+		return (errno);
 	}
 
 	curp = ifap;
@@ -924,7 +923,6 @@ kvp_get_ip_info(int family, char *if_name, int op,
 			/*
 			 * Collect other ip configuration info.
 			 */
-
 			kvp_get_ipconfig_info(if_name, ip_buffer);
 		}
 
@@ -954,7 +952,7 @@ kvp_write_file(FILE *f, const char *s1, const char *s2, const char *s3)
 	ret = fprintf(f, "%s%s%s%s\n", s1, s2, "=", s3);
 
 	if (ret < 0) {
-		return (HV_KVP_E_FAIL);
+		return (EIO);
 	}
 
 	return (0);
@@ -979,7 +977,7 @@ kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
 
 	if (file == NULL) {
 		KVP_LOG(LOG_ERR, "FreeBSD Failed to open config file\n");
-		return (HV_KVP_E_FAIL);
+		return (errno);
 	}
 
 	/*
@@ -988,7 +986,7 @@ kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
 
 	mac_addr = kvp_if_name_to_mac(if_name);
 	if (mac_addr == NULL) {
-		error = HV_KVP_E_FAIL;
+		error = EINVAL;
 		goto kvp_set_ip_info_error;
 	}
 	/* MAC Address */
@@ -1091,28 +1089,30 @@ kvp_op_getipinfo(struct hv_kvp_msg *op_msg, void *data __unused)
 {
 	struct hv_kvp_ipaddr_value *ip_val;
 	char *if_name;
+	int error = 0;
 
 	assert(op_msg != NULL);
 	KVP_LOG(LOG_DEBUG, "In kvp_op_getipinfo.\n");
 
 	ip_val = &op_msg->body.kvp_ip_val;
-	op_msg->hdr.error = HV_KVP_S_OK;
+	op_msg->hdr.error = HV_S_OK;
 
 	if_name = kvp_mac_to_if_name((char *)ip_val->adapter_id);
 
 	if (if_name == NULL) {
 		/* No interface found with the mac address. */
-		op_msg->hdr.error = HV_KVP_E_FAIL;
+		op_msg->hdr.error = HV_E_FAIL;
 		goto kvp_op_getipinfo_done;
 	}
 
-	op_msg->hdr.error = kvp_get_ip_info(0, if_name,
+	error = kvp_get_ip_info(0, if_name,
 	    HV_KVP_OP_GET_IP_INFO, ip_val, (MAX_IP_ADDR_SIZE * 2));
-
+	if (error)
+		op_msg->hdr.error = HV_E_FAIL;
 	free(if_name);
 
 kvp_op_getipinfo_done:
-	return(op_msg->hdr.error);
+	return (error);
 }
 
 
@@ -1121,25 +1121,27 @@ kvp_op_setipinfo(struct hv_kvp_msg *op_msg, void *data __unused)
 {
 	struct hv_kvp_ipaddr_value *ip_val;
 	char *if_name;
+	int error = 0;
 
 	assert(op_msg != NULL);
 	KVP_LOG(LOG_DEBUG, "In kvp_op_setipinfo.\n");
 
 	ip_val = &op_msg->body.kvp_ip_val;
-	op_msg->hdr.error = HV_KVP_S_OK;
+	op_msg->hdr.error = HV_S_OK;
 
 	if_name = (char *)ip_val->adapter_id;
 
 	if (if_name == NULL) {
 		/* No adapter provided. */
-		op_msg->hdr.error = HV_KVP_GUID_NOTFOUND;
+		op_msg->hdr.error = HV_GUID_NOTFOUND;
 		goto kvp_op_setipinfo_done;
 	}
 
-	op_msg->hdr.error = kvp_set_ip_info(if_name, ip_val);
-
+	error = kvp_set_ip_info(if_name, ip_val);
+	if (error)
+		op_msg->hdr.error = HV_E_FAIL;
 kvp_op_setipinfo_done:
-	return(op_msg->hdr.error);
+	return (error);
 }
 
 
@@ -1154,7 +1156,7 @@ kvp_op_setgetdel(struct hv_kvp_msg *op_msg, void *data)
 	assert(op_hdlr != NULL);
 
 	op_pool = op_msg->hdr.kvp_hdr.pool;
-	op_msg->hdr.error = HV_KVP_S_OK;
+	op_msg->hdr.error = HV_S_OK;
 
 	switch(op_hdlr->kvp_op_key) {
 	case HV_KVP_OP_SET:
@@ -1198,8 +1200,7 @@ kvp_op_setgetdel(struct hv_kvp_msg *op_msg, void *data)
 	}
 
 	if (error != 0)
-		op_msg->hdr.error = HV_KVP_S_CONT;
-
+		op_msg->hdr.error = HV_S_CONT;
 	return(error);
 }
 
@@ -1216,7 +1217,7 @@ kvp_op_enumerate(struct hv_kvp_msg *op_msg, void *data __unused)
 
 	op = op_msg->hdr.kvp_hdr.operation;
 	op_pool = op_msg->hdr.kvp_hdr.pool;
-	op_msg->hdr.error = HV_KVP_S_OK;
+	op_msg->hdr.error = HV_S_OK;
 
 	/*
 	 * If the pool is not HV_KVP_POOL_AUTO, read from the appropriate
@@ -1229,7 +1230,7 @@ kvp_op_enumerate(struct hv_kvp_msg *op_msg, void *data __unused)
 		    HV_KVP_EXCHANGE_MAX_KEY_SIZE,
 		    op_msg->body.kvp_enum_data.data.msg_value.value,
 		    HV_KVP_EXCHANGE_MAX_VALUE_SIZE)) {
-			op_msg->hdr.error = HV_KVP_S_CONT;
+			op_msg->hdr.error = HV_S_CONT;
 			error = -1;
 		}
 		goto kvp_op_enumerate_done;
@@ -1298,12 +1299,14 @@ kvp_op_enumerate(struct hv_kvp_msg *op_msg, void *data __unused)
 		KVP_LOG(LOG_ERR, "Auto pool Index %d not found.\n",
 		    op_msg->body.kvp_enum_data.index);
 #endif
-		op_msg->hdr.error = HV_KVP_S_CONT;
+		op_msg->hdr.error = HV_S_CONT;
 		error = -1;
 		break;
 	}
 
 kvp_op_enumerate_done:
+	if (error != 0)
+		op_msg->hdr.error = HV_S_CONT;
 	return(error);
 }
 
@@ -1496,10 +1499,13 @@ main(int argc, char *argv[])
 			 */
 			error = kvp_op_hdlrs[op].kvp_op_exec(hv_msg,
 			    (void *)&kvp_op_hdlrs[op]);
-			if (error != 0 && hv_msg->hdr.error != HV_KVP_S_CONT)
-				KVP_LOG(LOG_WARNING,
-				    "Operation failed OP = %d, error = 0x%x\n",
-				    op, error);
+			if (error != 0) {
+				assert(hv_msg->hdr.error != HV_S_OK);
+				if (hv_msg->hdr.error != HV_S_CONT)
+					KVP_LOG(LOG_WARNING,
+					    "Operation failed OP = %d, error = 0x%x\n",
+					    op, error);
+			}
 		}
 
 		/*
diff --git a/sys/dev/hyperv/utilities/hv_kvp.c b/sys/dev/hyperv/utilities/hv_kvp.c
index 5a83577..3753c2f 100644
--- a/sys/dev/hyperv/utilities/hv_kvp.c
+++ b/sys/dev/hyperv/utilities/hv_kvp.c
@@ -72,8 +72,6 @@ __FBSDID("$FreeBSD$");
 
 /* hv_kvp defines */
 #define BUFFERSIZE	sizeof(struct hv_kvp_msg)
-#define KVP_SUCCESS	0
-#define KVP_ERROR	1
 #define kvp_hdr		hdr.kvp_hdr
 
 #define KVP_FWVER_MAJOR		3
@@ -480,7 +478,7 @@ hv_kvp_convert_usermsg_to_hostmsg(struct hv_kvp_msg *umsg, struct hv_kvp_msg *hm
 	case HV_KVP_OP_SET_IP_INFO:
 	case HV_KVP_OP_SET:
 	case HV_KVP_OP_DELETE:
-		return (KVP_SUCCESS);
+		return (0);
 
 	case HV_KVP_OP_ENUMERATE:
 		host_exchg_data = &hmsg->body.kvp_enum_data.data;
@@ -501,9 +499,9 @@ hv_kvp_convert_usermsg_to_hostmsg(struct hv_kvp_msg *umsg, struct hv_kvp_msg *hm
 		host_exchg_data->value_type = HV_REG_SZ;
 
 		if ((hkey_len < 0) || (hvalue_len < 0))
-			return (HV_KVP_E_FAIL);
+			return (EINVAL);
 
-		return (KVP_SUCCESS);
+		return (0);
 
 	case HV_KVP_OP_GET:
 		host_exchg_data = &hmsg->body.kvp_get.data;
@@ -519,12 +517,12 @@ hv_kvp_convert_usermsg_to_hostmsg(struct hv_kvp_msg *umsg, struct hv_kvp_msg *hm
 		host_exchg_data->value_type = HV_REG_SZ;
 
 		if ((hkey_len < 0) || (hvalue_len < 0))
-			return (HV_KVP_E_FAIL);
+			return (EINVAL);
 
-		return (KVP_SUCCESS);
+		return (0);
 
 	default:
-		return (HV_KVP_E_FAIL);
+		return (EINVAL);
 	}
 }
 
@@ -533,16 +531,13 @@ hv_kvp_convert_usermsg_to_hostmsg(struct hv_kvp_msg *umsg, struct hv_kvp_msg *hm
  * Send the response back to the host.
  */
 static void
-hv_kvp_respond_host(hv_kvp_sc *sc, int error)
+hv_kvp_respond_host(hv_kvp_sc *sc, uint32_t error)
 {
 	struct hv_vmbus_icmsg_hdr *hv_icmsg_hdrp;
 
 	hv_icmsg_hdrp = (struct hv_vmbus_icmsg_hdr *)
 	    &sc->rcv_buf[sizeof(struct hv_vmbus_pipe_hdr)];
 
-	if (error)
-		error = HV_KVP_E_FAIL;
-
 	hv_icmsg_hdrp->status = error;
 	hv_icmsg_hdrp->icflags = HV_ICMSGHDRFLAG_TRANSACTION |
 	    HV_ICMSGHDRFLAG_RESPONSE;
@@ -612,8 +607,10 @@ hv_kvp_process_request(void *context, int pending)
 			error = vmbus_ic_negomsg(&sc->util_sc,
 			    kvp_buf, &recvlen, KVP_FWVER, KVP_MSGVER);
 			/* XXX handle vmbus_ic_negomsg failure. */
-			hv_kvp_respond_host(sc, error);
-
+			if (!error)
+				hv_kvp_respond_host(sc, HV_S_OK);
+			else
+				hv_kvp_respond_host(sc, HV_E_FAIL);
 			/*
 			 * It is ok to not acquire the mutex before setting
 			 * req_in_progress here because negotiation is the
@@ -657,7 +654,7 @@ hv_kvp_process_request(void *context, int pending)
 		 */
 		if (hv_kvp_req_in_progress(sc)) {
 			hv_kvp_log_info("%s: request was still active after wait so failing\n", __func__);
-			hv_kvp_respond_host(sc, HV_KVP_E_FAIL);
+			hv_kvp_respond_host(sc, HV_E_FAIL);
 			sc->req_in_progress = false;
 		}
 
@@ -737,9 +734,9 @@ hv_kvp_dev_daemon_read(struct cdev *dev, struct uio *uio, int ioflag __unused)
 	struct hv_kvp_msg *hv_kvp_dev_buf;
 	hv_kvp_sc *sc = (hv_kvp_sc*)dev->si_drv1;
 
-	/* Check hv_kvp daemon registration status*/
+	/* Read is not allowed util registering is done. */
 	if (!sc->register_done)
-		return (KVP_ERROR);
+		return (EPERM);
 
 	sema_wait(&sc->dev_sema);
 
@@ -789,7 +786,7 @@ hv_kvp_dev_daemon_write(struct cdev *dev, struct uio *uio, int ioflag __unused)
 		}
 		else {
 			hv_kvp_log_info("%s, KVP Registration Failed\n", __func__);
-			return (KVP_ERROR);
+			return (EINVAL);
 		}
 	} else {
 
@@ -799,10 +796,15 @@ hv_kvp_dev_daemon_write(struct cdev *dev, struct uio *uio, int ioflag __unused)
 			struct hv_kvp_msg *hmsg = sc->host_kvp_msg;
 			struct hv_kvp_msg *umsg = &sc->daemon_kvp_msg;
 
-			hv_kvp_convert_usermsg_to_hostmsg(umsg, hmsg);
-			hv_kvp_respond_host(sc, KVP_SUCCESS);
+			error = hv_kvp_convert_usermsg_to_hostmsg(umsg, hmsg);
+			hv_kvp_respond_host(sc, umsg->hdr.error);
 			wakeup(sc);
 			sc->req_in_progress = false;
+			if (umsg->hdr.error != HV_S_OK)
+				hv_kvp_log_info("%s, Error 0x%x from daemon\n",
+				    __func__, umsg->hdr.error);
+			if (error)
+				hv_kvp_log_info("%s, Error from convert\n", __func__);
 		}
 
 		sc->daemon_busy = false;
@@ -865,7 +867,7 @@ hv_kvp_attach(device_t dev)
 	child = SYSCTL_CHILDREN(device_get_sysctl_tree(dev));
 
 	SYSCTL_ADD_INT(ctx, child, OID_AUTO, "hv_kvp_log",
-	    CTLFLAG_RW, &hv_kvp_log, 0, "Hyperv KVP service log level");
+	    CTLFLAG_RWTUN, &hv_kvp_log, 0, "Hyperv KVP service log level");
 
 	TASK_INIT(&sc->task, 0, hv_kvp_process_request, sc);
 
diff --git a/sys/dev/hyperv/utilities/hv_kvp.h b/sys/dev/hyperv/utilities/hv_kvp.h
index c391da0..a94e671 100644
--- a/sys/dev/hyperv/utilities/hv_kvp.h
+++ b/sys/dev/hyperv/utilities/hv_kvp.h
@@ -144,19 +144,6 @@ enum hv_kvp_exchg_pool {
 	HV_KVP_POOL_COUNT /* Number of pools, must be last. */
 };
 
-
-/*
- * Some Hyper-V status codes.
- */
-#define HV_KVP_S_OK                      0x00000000
-#define HV_KVP_E_FAIL                    0x80004005
-#define HV_KVP_S_CONT                    0x80070103
-#define HV_ERROR_NOT_SUPPORTED           0x80070032
-#define HV_ERROR_MACHINE_LOCKED          0x800704F7
-#define HV_ERROR_DEVICE_NOT_CONNECTED    0x8007048F
-#define HV_INVALIDARG                    0x80070057
-#define HV_KVP_GUID_NOTFOUND             0x80041002
-
 #define ADDR_FAMILY_NONE                 0x00
 #define ADDR_FAMILY_IPV4                 0x01
 #define ADDR_FAMILY_IPV6                 0x02
diff --git a/sys/dev/hyperv/utilities/hv_utilreg.h b/sys/dev/hyperv/utilities/hv_utilreg.h
index 9358776..b29c0f9 100644
--- a/sys/dev/hyperv/utilities/hv_utilreg.h
+++ b/sys/dev/hyperv/utilities/hv_utilreg.h
@@ -29,10 +29,17 @@
 #ifndef _HV_UTILREG_H_
 #define _HV_UTILREG_H_
 
-#define HV_S_OK			0x00000000
-#define HV_E_FAIL		0x80004005
-#define HV_ERROR_NOT_SUPPORTED	0x80070032
-#define HV_ERROR_MACHINE_LOCKED	0x800704F7
+/*
+ * Some Hyper-V status codes.
+ */
+#define HV_S_OK				0x00000000
+#define HV_E_FAIL			0x80004005
+#define HV_S_CONT			0x80070103
+#define HV_ERROR_NOT_SUPPORTED		0x80070032
+#define HV_ERROR_MACHINE_LOCKED		0x800704F7
+#define HV_ERROR_DEVICE_NOT_CONNECTED	0x8007048F
+#define HV_INVALIDARG			0x80070057
+#define HV_GUID_NOTFOUND		0x80041002
 
 /*
  * Common defines for Hyper-V ICs