mi: make return values + errno consistent across error paths

This change unifies and documents the error return values for the MI
implementation. This gives us the following semantics:

 - zero on success

 - -1 for errors in the MI communication with an endpoint, with errno
   set accordingly

 - positive values where the MI communication succeeded, but we received
   an error response from the endpoint. The return value will be that
   from the MI response status field, and should correspond to one of
   the nvme_mi_resp_status values.

We add these semantics to the file-level kdoc comments in mi.h.

Most of the changes here are replacing the negative-errno returns:

    return -EIO;

with:

    errno = EIO;
    return -1;

but there are a few slightly-more-involved changes where we need to
preserve errno across a cleanup/log that might clobber it.

For the dbus code in mi-mctp.c, we need to convert the sd_bus convention of
negative-errno values into errno; we can do most of these through the
dbus_err() helper.

Fixes: #417

Signed-off-by: Jeremy Kerr <[email protected]>

Author: jk-ozlabs

src/nvme/mi-mctp.c

@@ -210,17 +210,21 @@ static int nvme_mi_mctp_submit(struct nvme_mi_ep *ep,
 	struct iovec req_iov[3], resp_iov[3];
 	struct msghdr req_msg, resp_msg;
 	struct sockaddr_mctp addr;
+	int i, rc, errno_save;
 	ssize_t len;
 	__le32 mic;
-	int i, rc;
 	__u8 tag;
 
-	if (ep->transport != &nvme_mi_transport_mctp)
-		return -EINVAL;
+	if (ep->transport != &nvme_mi_transport_mctp) {
+		errno = EINVAL;
+		return -1;
+	}
 
 	/* we need enough space for at least a generic (/error) response */
-	if (resp->hdr_len < sizeof(struct nvme_mi_msg_resp))
-		return -EINVAL;
+	if (resp->hdr_len < sizeof(struct nvme_mi_msg_resp)) {
+		errno = EINVAL;
+		return -1;
+	}
 
 	mctp = ep->transport_data;
 	tag = nvme_mi_mctp_tag_alloc(ep);
@@ -256,9 +260,11 @@ static int nvme_mi_mctp_submit(struct nvme_mi_ep *ep,
 
 	len = ops.sendmsg(mctp->sd, &req_msg, 0);
 	if (len < 0) {
+		errno_save = errno;
 		nvme_msg(ep->root, LOG_ERR,
 			 "Failure sending MCTP message: %m\n");
-		rc = len;
+		errno = errno_save;
+		rc = -1;
 		goto out;
 	}
 
@@ -282,14 +288,17 @@ static int nvme_mi_mctp_submit(struct nvme_mi_ep *ep,
 	len = ops.recvmsg(mctp->sd, &resp_msg, 0);
 
 	if (len < 0) {
+		errno_save = errno;
 		nvme_msg(ep->root, LOG_ERR,
 			 "Failure receiving MCTP message: %m\n");
+		errno = errno_save;
 		goto out;
 	}
 
 
 	if (len == 0) {
 		nvme_msg(ep->root, LOG_WARNING, "No data from MCTP endpoint\n");
+		errno = EIO;
 		goto out;
 	}
 
@@ -304,6 +313,7 @@ static int nvme_mi_mctp_submit(struct nvme_mi_ep *ep,
 		nvme_msg(ep->root, LOG_ERR,
 			 "Invalid MCTP response: too short (%zd bytes, needed %zd)\n",
 			 len, 8 + sizeof(mic));
+		errno = EPROTO;
 		goto out;
 	}
 
@@ -312,6 +322,7 @@ static int nvme_mi_mctp_submit(struct nvme_mi_ep *ep,
 		nvme_msg(ep->root, LOG_WARNING,
 			 "Response message has unaligned length (%zd)!\n",
 			 len);
+		errno = EPROTO;
 		goto out;
 	}
 
@@ -379,8 +390,10 @@ static int nvme_mi_mctp_desc_ep(struct nvme_mi_ep *ep, char *buf, size_t len)
 {
 	struct nvme_mi_transport_mctp *mctp;
 
-	if (ep->transport != &nvme_mi_transport_mctp)
+	if (ep->transport != &nvme_mi_transport_mctp) {
+		errno = EINVAL;
 		return -1;
+	}
 
 	mctp = ep->transport_data;
 
@@ -401,6 +414,7 @@ nvme_mi_ep_t nvme_mi_open_mctp(nvme_root_t root, unsigned int netid, __u8 eid)
 {
 	struct nvme_mi_transport_mctp *mctp;
 	struct nvme_mi_ep *ep;
+	int errno_save;
 
 	ep = nvme_mi_init_ep(root);
 	if (!ep)
@@ -423,15 +437,22 @@ nvme_mi_ep_t nvme_mi_open_mctp(nvme_root_t root, unsigned int netid, __u8 eid)
 	return ep;
 
 err_free_ep:
+	errno_save = errno;
 	free(ep);
+	errno = errno_save;
 	return NULL;
 }
 
 #ifdef CONFIG_LIBSYSTEMD
 
-static void _dbus_err(nvme_root_t root, int rc, int line) {
+/* helper for handling dbus errors: D-Bus API returns a negtive errno on
+ * failure; set errno and log.
+ */
+static void _dbus_err(nvme_root_t root, int rc, int line)
+{
 	nvme_msg(root, LOG_ERR, "MCTP D-Bus failed line %d: %s %d\n",
 		line, strerror(-rc), rc);
+	errno = -rc;
 }
 
 #define dbus_err(r, rc) _dbus_err(r, rc, __LINE__)
@@ -452,9 +473,8 @@ static int nvme_mi_mctp_add(nvme_root_t root, unsigned int netid, __u8 eid)
 	}
 
 	ep = nvme_mi_open_mctp(root, netid, eid);
-	if (!ep) {
-		return -ENOMEM;
-	}
+	if (!ep)
+		return -1;
 
 	return 0;
 }
@@ -480,7 +500,7 @@ static int handle_mctp_endpoint(nvme_root_t root, const char* objpath,
 		rc = sd_bus_message_enter_container(m, 'a', "{sv}");
 		if (rc < 0) {
 			dbus_err(root, rc);
-			return rc;
+			return -1;
 		}
 
 		while (!container_end(m)) {
@@ -491,13 +511,13 @@ static int handle_mctp_endpoint(nvme_root_t root, const char* objpath,
 			rc = sd_bus_message_enter_container(m, 'e', "sv");
 			if (rc < 0) {
 				dbus_err(root, rc);
-				return rc;
+				return -1;
 			}
 
 			rc = sd_bus_message_read(m, "s", &propname);
 			if (rc < 0) {
 				dbus_err(root, rc);
-				return rc;
+				return -1;
 			}
 
 			if (strcmp(propname, "EID") == 0) {
@@ -520,36 +540,38 @@ static int handle_mctp_endpoint(nvme_root_t root, const char* objpath,
 
 			if (rc < 0) {
 				dbus_err(root, rc);
-				return rc;
+				return -1;
 			}
 
 			/* Exit prop item */
 			rc = sd_bus_message_exit_container(m);
 			if (rc < 0) {
 				dbus_err(root, rc);
-				return rc;
+				return -1;
 			}
 		}
 
 		/* Exit property dict */
 		rc = sd_bus_message_exit_container(m);
 		if (rc < 0) {
 			dbus_err(root, rc);
-			return rc;
+			return -1;
 		}
 	}
 
 	if (have_nvmemi) {
 		if (!(have_eid && have_net)) {
 			nvme_msg(root, LOG_ERR,
 				 "Missing property for %s\n", objpath);
-			return -ENOENT;
+			errno = ENOENT;
+			return -1;
 		}
 		rc = nvme_mi_mctp_add(root, net, eid);
 		if (rc < 0) {
+			int errno_save = errno;
 			nvme_msg(root, LOG_ERR,
-				 "Error adding net %d eid %d: %s\n",
-				net, eid, strerror(-rc));
+				 "Error adding net %d eid %d: %m\n", net, eid);
+			errno = errno_save;
 		}
 	} else {
 		/* Ignore other endpoints */
@@ -567,15 +589,15 @@ static int handle_mctp_obj(nvme_root_t root, sd_bus_message *m)
 	rc = sd_bus_message_read(m, "o", &objpath);
 	if (rc < 0) {
 		dbus_err(root, rc);
-		return rc;
+		return -1;
 	}
 
 	/* Enter response object: our array of (string, property dict)
 	 * values */
 	rc = sd_bus_message_enter_container(m, 'a', "{sa{sv}}");
 	if (rc < 0) {
 		dbus_err(root, rc);
-		return rc;
+		return -1;
 	}
 
 
@@ -585,13 +607,13 @@ static int handle_mctp_obj(nvme_root_t root, sd_bus_message *m)
 		rc = sd_bus_message_enter_container(m, 'e', "sa{sv}");
 		if (rc < 0) {
 			dbus_err(root, rc);
-			return rc;
+			return -1;
 		}
 
 		rc = sd_bus_message_read(m, "s", &ifname);
 		if (rc < 0) {
 			dbus_err(root, rc);
-			return rc;
+			return -1;
 		}
 
 		if (!strcmp(ifname, MCTP_DBUS_IFACE_ENDPOINT)) {
@@ -605,23 +627,23 @@ static int handle_mctp_obj(nvme_root_t root, sd_bus_message *m)
 			rc = sd_bus_message_skip(m, "a{sv}");
 			if (rc < 0) {
 				dbus_err(root, rc);
-				return rc;
+				return -1;
 			}
 		}
 
 		/* Exit interface item */
 		rc = sd_bus_message_exit_container(m);
 		if (rc < 0) {
 			dbus_err(root, rc);
-			return rc;
+			return -1;
 		}
 	}
 
 	/* Exit response object */
 	rc = sd_bus_message_exit_container(m);
 	if (rc < 0) {
 		dbus_err(root, rc);
-		return rc;
+		return -1;
 	}
 
 	return 0;
@@ -632,19 +654,22 @@ nvme_root_t nvme_mi_scan_mctp(void)
 	sd_bus *bus = NULL;
 	sd_bus_message *resp = NULL;
 	sd_bus_error berr = SD_BUS_ERROR_NULL;
+	int rc, errno_save;
 	nvme_root_t root;
-	int rc;
 
 	root = nvme_mi_create_root(NULL, DEFAULT_LOGLEVEL);
 	if (!root) {
-		rc = -ENOMEM;
+		errno = ENOMEM;
+		rc = -1;
 		goto out;
 	}
 
 	rc = sd_bus_default_system(&bus);
 	if (rc < 0) {
 		nvme_msg(root, LOG_ERR, "Failed opening D-Bus: %s\n",
 			 strerror(-rc));
+		errno = -rc;
+		rc = -1;
 		goto out;
 	}
 
@@ -659,14 +684,17 @@ nvme_root_t nvme_mi_scan_mctp(void)
 	if (rc < 0) {
 		nvme_msg(root, LOG_ERR, "Failed querying MCTP D-Bus: %s (%s)\n",
 			 berr.message, berr.name);
+		errno = -rc;
+		rc = -1;
 		goto out;
 	}
 
 	rc = sd_bus_message_enter_container(resp, 'a', "{oa{sa{sv}}}");
 	if (rc != 1) {
 		dbus_err(root, rc);
 		if (rc == 0)
-			rc = -EPROTO;
+			errno = EPROTO;
+		rc = -1;
 		goto out;
 	}
 
@@ -675,6 +703,7 @@ nvme_root_t nvme_mi_scan_mctp(void)
 		rc = sd_bus_message_enter_container(resp, 'e', "oa{sa{sv}}");
 		if (rc < 0) {
 			dbus_err(root, rc);
+			rc = -1;
 			goto out;
 		}
 
@@ -683,18 +712,21 @@ nvme_root_t nvme_mi_scan_mctp(void)
 		rc = sd_bus_message_exit_container(resp);
 		if (rc < 0) {
 			dbus_err(root, rc);
+			rc = -1;
 			goto out;
 		}
 	}
 
 	rc = sd_bus_message_exit_container(resp);
 	if (rc < 0) {
 		dbus_err(root, rc);
+		rc = -1;
 		goto out;
 	}
 	rc = 0;
 
 out:
+	errno_save = errno;
 	sd_bus_error_free(&berr);
 	sd_bus_message_unref(resp);
 	sd_bus_unref(bus);
@@ -703,6 +735,7 @@ nvme_root_t nvme_mi_scan_mctp(void)
 		if (root) {
 			nvme_mi_free_root(root);
 		}
+		errno = errno_save;
 		root = NULL;
 	}
 	return root;