LU-14802 nodemap: return proper error code

In nodemap_add_range_helper() it was always returning -ENOMEM when
there was an error inserting a new range into the existing nodemap.

    nodemap_add_range_helper()) cannot insert nodemap range: rc = -17
    mgs_iocontrol_nodemap()) MGS: OBD_IOC_NODEMAP command: rc = -12

This was confusing because the error returned by range_insert() was
typically -EEXIST (i.e. the entry being inserted already was in the
nodemap).  Do not print an error to the console in this common case.

Return the actual error to the caller so that this is more clear
to the end user.  Have l_ioctl() always set errno on error, in
addition to returning the error, since many callers depend on this.

Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
Change-Id: I2c80a11dfdf9e6e1c9a8235b8f74f5bcea68c08e
Reviewed-on: https://review.whamcloud.com/45626
Reviewed-by: Sebastien Buisson <sbuisson@ddn.com>
Tested-by: jenkins <devops@whamcloud.com>
Reviewed-by: Etienne AUJAMES <eaujames@ddn.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/libcfs/libcfs/util/l_ioctl.c b/libcfs/libcfs/util/l_ioctl.c
index 0a0d3fa..15e3d33 100644 (file)
--- a/libcfs/libcfs/util/l_ioctl.c
+++ b/libcfs/libcfs/util/l_ioctl.c
@@ -44,19 +44,25 @@ struct ioc_dev {
 
 static struct ioc_dev ioc_dev_list[10];
 
+#ifndef ARRAY_SIZE
+# define ARRAY_SIZE(a) ((sizeof(a)) / (sizeof((a)[0])))
+#endif /* !ARRAY_SIZE */
+
 static int
 open_ioc_dev(int dev_id)
 {
        const char *dev_name;
 
-       if (dev_id < 0 ||
-           dev_id >= sizeof(ioc_dev_list) / sizeof(ioc_dev_list[0]))
-               return -EINVAL;
+       if (dev_id < 0 || dev_id >= ARRAY_SIZE(ioc_dev_list)) {
+               errno = EINVAL;
+               return -errno;
+       }
 
        dev_name = ioc_dev_list[dev_id].dev_name;
        if (!dev_name) {
                fprintf(stderr, "unknown device id: %d\n", dev_id);
-               return -EINVAL;
+               errno = EINVAL;
+               return -errno;
        }
 
        if (ioc_dev_list[dev_id].dev_fd < 0) {
@@ -66,7 +72,7 @@ open_ioc_dev(int dev_id)
                        fprintf(stderr, "opening %s failed: %s\n"
                                "hint: the kernel modules may not be loaded\n",
                                dev_name, strerror(errno));
-                       return fd;
+                       return -errno;
                }
                ioc_dev_list[dev_id].dev_fd = fd;
        }
@@ -84,7 +90,7 @@ int l_ioctl(int dev_id, unsigned int opc, void *buf)
 
        rc = ioctl(fd, opc, buf);
 
-       return rc;
+       return rc < 0 ? -errno : rc;
 }
 
 /* register a device to send ioctls to. */

diff --git a/lustre/mgs/mgs_handler.c b/lustre/mgs/mgs_handler.c
index b6ec00d..688b538 100644 (file)
--- a/lustre/mgs/mgs_handler.c
+++ b/lustre/mgs/mgs_handler.c
@@ -872,10 +872,11 @@ static int mgs_iocontrol_nodemap(const struct lu_env *env,
                rc = -ENOTTY;
        }
 
-       if (rc != 0) {
-               CERROR("%s: OBD_IOC_NODEMAP command %X for %s: rc = %d\n",
-                      mgs->mgs_obd->obd_name, lcfg->lcfg_command,
-                      nodemap_name, rc);
+       if (rc) {
+               CDEBUG_LIMIT(rc == -EEXIST ? D_INFO : D_ERROR,
+                            "%s: OBD_IOC_NODEMAP command %X for %s: rc = %d\n",
+                            mgs->mgs_obd->obd_name, lcfg->lcfg_command,
+                            nodemap_name, rc);
                GOTO(out_lcfg, rc);
        }
 

diff --git a/lustre/ptlrpc/nodemap_handler.c b/lustre/ptlrpc/nodemap_handler.c
index c3a8103..d793fe5 100644 (file)
--- a/lustre/ptlrpc/nodemap_handler.c
+++ b/lustre/ptlrpc/nodemap_handler.c
@@ -815,13 +815,14 @@ int nodemap_add_range_helper(struct nodemap_config *config,
        }
 
        rc = range_insert(&config->nmc_range_tree, range);
-       if (rc != 0) {
-               CERROR("cannot insert nodemap range into '%s': rc = %d\n",
-                     nodemap->nm_name, rc);
+       if (rc) {
+               CDEBUG_LIMIT(rc == -EEXIST ? D_INFO : D_ERROR,
+                            "cannot insert nodemap range into '%s': rc = %d\n",
+                            nodemap->nm_name, rc);
                up_write(&config->nmc_range_tree_lock);
                list_del(&range->rn_list);
                range_destroy(range);
-               GOTO(out, rc = -ENOMEM);
+               GOTO(out, rc);
        }
 
        list_add(&range->rn_list, &nodemap->nm_ranges);
@@ -844,6 +845,7 @@ int nodemap_add_range_helper(struct nodemap_config *config,
 out:
        return rc;
 }
+
 int nodemap_add_range(const char *name, const lnet_nid_t nid[2])
 {
        struct lu_nodemap       *nodemap = NULL;

diff --git a/lustre/utils/liblustreapi.c b/lustre/utils/liblustreapi.c
index 1efc766..8ce5dc4 100644 (file)
--- a/lustre/utils/liblustreapi.c
+++ b/lustre/utils/liblustreapi.c
@@ -364,7 +364,7 @@ int llapi_ioctl_unpack(struct obd_ioctl_data *data, char *pbuf, int max_len)
        char *ptr;
 
        if (pbuf == NULL)
-               return 1;
+               return -EINVAL;
 
        overlay = (struct obd_ioctl_data *)pbuf;
 

diff --git a/lustre/utils/obd.c b/lustre/utils/obd.c
index c43a7cc..ce86ae1 100644 (file)
--- a/lustre/utils/obd.c
+++ b/lustre/utils/obd.c
@@ -4024,24 +4024,24 @@ static int nodemap_cmd(enum lcfg_command_type cmd, void *ret_data,
 
        memset(buf, 0, sizeof(rawbuf));
        rc = llapi_ioctl_pack(&data, &buf, sizeof(rawbuf));
-       if (rc != 0) {
+       if (rc) {
                fprintf(stderr,
-                       "error: invalid ioctl: %08x errno: %d with rc=%d\n",
-                       cmd, errno, rc);
+                       "error: invalid ioctl request: %08x errno: %d: %s\n",
+                       cmd, errno, strerror(-rc));
                goto out;
        }
 
        rc = l_ioctl(OBD_DEV_ID, OBD_IOC_NODEMAP, buf);
-       if (rc != 0) {
+       if (rc) {
                fprintf(stderr,
-                       "error: invalid ioctl: %08x errno: %d with rc=%d\n",
-                       cmd, errno, rc);
+                       "error: invalid ioctl: %08x errno: %d: %s\n",
+                       cmd, errno, strerror(errno));
                goto out;
        }
 
        if (ret_data) {
                rc = llapi_ioctl_unpack(&data, buf, sizeof(rawbuf));
-               if (rc != 0)
+               if (rc)
                        goto out;
 
                if (ret_size > data.ioc_plen1)
@@ -4242,7 +4242,7 @@ static int parse_nid_range(char *nodemap_range, char *nid_range, int range_len)
                fprintf(stderr,
                        "error: nodemap_xxx_range: can't parse nid range: %s\n",
                        nodemap_range);
-               return -1;
+               return -EINVAL;
        }
 
        rc = cfs_nidrange_find_min_max(&nidlist, &min_nid[0], &max_nid[0],
@@ -4272,7 +4272,7 @@ static int parse_nid_range(char *nodemap_range, char *nid_range, int range_len)
  * --name                      nodemap name
  * --range                     properly formatted nid range
  *
- * \retval                     0 on success
+ * \retval                     0 on success, -errno on error
  */
 int jt_nodemap_add_range(int argc, char **argv)
 {
@@ -4302,21 +4302,20 @@ int jt_nodemap_add_range(int argc, char **argv)
        if (!nodemap_name || !nodemap_range) {
                fprintf(stderr,
                        "usage: nodemap_add_range --name <name> --range <range>\n");
-               return -1;
+               return -EINVAL;
        }
 
        rc = parse_nid_range(nodemap_range, nid_range, sizeof(nid_range));
        if (rc) {
-               errno = -rc;
                return rc;
        }
        rc = nodemap_cmd(LCFG_NODEMAP_ADD_RANGE, NULL, 0, argv[0],
                         nodemap_name, nid_range, NULL);
-       if (rc != 0) {
-               errno = -rc;
+       if (rc) {
                fprintf(stderr,
-                       "error: %s: cannot add range '%s' to nodemap '%s': rc = %d\n",
-                       jt_cmdname(argv[0]), nodemap_range, nodemap_name, rc);
+                       "error: %s: cannot add range '%s' to nodemap '%s': %s\n",
+                       jt_cmdname(argv[0]), nodemap_range, nodemap_name,
+                       strerror(-rc));
        }
 
        return rc;
@@ -5594,7 +5593,7 @@ int jt_get_obj_version(int argc, char **argv)
        }
 
        rc = l_ioctl(OBD_DEV_ID, OBD_IOC_GET_OBJ_VERSION, buf);
-       if (rc == -1) {
+       if (rc) {
                fprintf(stderr, "error: %s: ioctl: %s\n",
                        jt_cmdname(argv[0]), strerror(errno));
                return -errno;