Author: sephe
Date: Fri Nov 11 08:04:24 2016
New Revision: 308516
URL: https://svnweb.freebsd.org/changeset/base/308516

Log:
  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

Modified:
  stable/10/contrib/hyperv/tools/hv_kvp_daemon.c
  stable/10/sys/dev/hyperv/utilities/hv_kvp.c
  stable/10/sys/dev/hyperv/utilities/hv_kvp.h
  stable/10/sys/dev/hyperv/utilities/hv_utilreg.h
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/contrib/hyperv/tools/hv_kvp_daemon.c
==============================================================================
--- stable/10/contrib/hyperv/tools/hv_kvp_daemon.c      Fri Nov 11 07:52:29 
2016        (r308515)
+++ stable/10/contrib/hyperv/tools/hv_kvp_daemon.c      Fri Nov 11 08:04:24 
2016        (r308516)
@@ -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, str
         */
        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_nam
 
        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_nam
                        /*
                         * 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, 
        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
 
        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
 
        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_m
 {
        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_m
 {
        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_m
        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_m
        }
 
        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_m
 
        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_m
                    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_m
                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);
+                       }
                }
 
                /*

Modified: stable/10/sys/dev/hyperv/utilities/hv_kvp.c
==============================================================================
--- stable/10/sys/dev/hyperv/utilities/hv_kvp.c Fri Nov 11 07:52:29 2016        
(r308515)
+++ stable/10/sys/dev/hyperv/utilities/hv_kvp.c Fri Nov 11 08:04:24 2016        
(r308516)
@@ -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
        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
                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
                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
  * 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, in
                        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, in
                 */
                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 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
                }
                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 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);
 

Modified: stable/10/sys/dev/hyperv/utilities/hv_kvp.h
==============================================================================
--- stable/10/sys/dev/hyperv/utilities/hv_kvp.h Fri Nov 11 07:52:29 2016        
(r308515)
+++ stable/10/sys/dev/hyperv/utilities/hv_kvp.h Fri Nov 11 08:04:24 2016        
(r308516)
@@ -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

Modified: stable/10/sys/dev/hyperv/utilities/hv_utilreg.h
==============================================================================
--- stable/10/sys/dev/hyperv/utilities/hv_utilreg.h     Fri Nov 11 07:52:29 
2016        (r308515)
+++ stable/10/sys/dev/hyperv/utilities/hv_utilreg.h     Fri Nov 11 08:04:24 
2016        (r308516)
@@ -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
_______________________________________________
svn-src-stable-10@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-stable-10
To unsubscribe, send any mail to "svn-src-stable-10-unsubscr...@freebsd.org"

Reply via email to