Whamcloud - gitweb
LU-17308 mgs: move pool_cmd check to the kernel 02/53202/12
authorEtienne AUJAMES <etienne.aujames@cea.fr>
Tue, 21 Nov 2023 19:01:43 +0000 (20:01 +0100)
committerOleg Drokin <green@whamcloud.com>
Sun, 4 Feb 2024 08:28:37 +0000 (08:28 +0000)
Several checks for pool_cmd need to be done before touching the MGS
configuration.

e.g: the following case should be denied before adding a destroy
record in the MGS configurations:
 - The pool does not exist
 - The pool is not empty (OSTs still in the pool)

This work is done in userspace (check_pool_cmd) by checking the client
lov parameters for pools. But nothing guarantees those parameters to
be in sync. So, only the MGS configuration should be trusted for that.

This patch move those checks in the kernel. There are several reasons
for this:
 - It guarantees the pool configurations consistency even if an
   external tool is used.
 - For standalone MGS, it limits the overhead of reading the
   configuration several times.

This patch add a "-n|--nowait" option for pool_cmd to skip waiting
for pool updates on the clients. This is useful when doing a lot of
pool_cmd in a raw. And this avoids cancelling clients CONFIG lock
each times (because of mgc_requeue_timeout_min).

e.g:
  lctl pool_destroy -n lustre.old
  lctl pool_new -n lustre.test
  lctl pool_add -n lustre.test OST0001
  ...
  lctl pool_add lustre.test OST0010

check_pool_cmd_result() is modified to compute the client wait delay
with mgc_requeue_timeout_min.

Add a regression test "ost-pools 2f".

Test-Parameters: testlist=ost-pools
Test-Parameters: testlist=ost-pools
Test-Parameters: testlist=ost-pools env=ONLY=2f,ONLY_REPEAT=50
Test-Parameters: testlist=ost-pools env=ONLY=2f,ONLY_REPEAT=50
Signed-off-by: Etienne AUJAMES <eaujames@ddn.com>
Change-Id: Ifbc49b5667bf17253716052a7480114936c65149
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/53202
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Guillaume Courrier <guillaume.courrier@cea.fr>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/doc/lctl-pool_add.8
lustre/doc/lctl-pool_new.8
lustre/mgs/mgs_llog.c
lustre/tests/ost-pools.sh
lustre/utils/lctl.c
lustre/utils/obd.c

index ed68033..fccfe05 100644 (file)
@@ -2,9 +2,11 @@
 .SH NAME
 lctl-pool_add \- add OSTs to a named pool
 .SH SYNOPSIS
-.B lctl pool_add \fI<fsname>.<poolname>\fR \fI<ost_index> ...\fR
+.BR "lctl pool_add" " [" --nowait | -n "] "
+.IR <fsname> . <pool> " "  <ost_index> " " ...
 .br
-.B lctl pool_add \fI<fsname>.<poolname>\fR \fI<ost_range> ...\fR
+.BR "lctl pool_add" " [" --nowait | -n "] "
+.IR <fsname> . <pool> " " <ost_range> " "...
 
 .SH DESCRIPTION
 Add one or more OSTs to the pool named
@@ -26,13 +28,16 @@ to
 .I end
 (inclusive), optionally skipping every
 .I step
-index values.  The
-.B lctl pool_add
-command must be run on the MGS node and can only be used by the
-root user.  If the MGS is on a separate node from the MDS, a
-Lustre client must be mounted while the
+index values.
+
+.BR NOTE:
+After updating the MGS configuration, this command tries to wait and
+check if pools are updated on a client.
+If the MGS is on a separate node from the MDS, a Lustre client must
+be mounted on the MGS node while the
 .B lctl
-commands are being run.
+commands are being run for this. Otherwise, the client check is
+skipped.
 
 The OST pool can be used by
 .BR lfs-setstripe (1)
@@ -48,6 +53,16 @@ OSTs that are in the current pool.  As well,
 .BR lfs-df (1)
 can show only the free space or inodes in a named pool.
 
+.SH OPTIONS
+.TP
+.BR -n ", " --nowait
+Do not wait and check if pool is updated on a client. This is useful
+when calling a lot of "
+.B lctl
+pool_*" in a row. This avoids revoking the clients "CONFIG" lock for each
+command (by default clients retake their lock and update their configurations
+in a delay between 5-10s).
+
 .SH EXAMPLES
 .TP
 .B # lfs pool_new testfs.local
index d9dd914..7fef593 100644 (file)
@@ -2,7 +2,8 @@
 .SH NAME
 lctl-pool_new \- create a new OST pool
 .SH SYNOPSIS
-.B lctl pool_new \fI<fsname>.<poolname>\fR
+.BR "lctl pool_new" " [" --nowait | -n "] "
+.IR <fsname> . <pool>
 
 .SH DESCRIPTION
 Create a list of OSTs with the name
@@ -12,10 +13,16 @@ in the filesystem named
 The
 .B lctl pool_new
 command must be run on the MGS node and can only be used by the
-root user.  If the MGS is on a separate node from the MDS, a
-Lustre client must be mounted while the
+root user.
+
+.BR NOTE:
+After updating the MGS configuration, this command tries to wait and
+check if pools are updated on a client.
+If the MGS is on a separate node from the MDS, a Lustre client must
+be mounted on the MGS node while the
 .B lctl
-commands are being run.
+commands are being run for this. Otherwise, the client check is
+skipped.
 
 This named list of OSTs can be used by
 .BR lfs-setstripe (1)
@@ -25,6 +32,16 @@ to locate files that were created on the specified pool.  As well,
 .BR lfs-df (1)
 can show only the free space or inodes in a named pool.
 
+.SH OPTIONS
+.TP
+.BR -n ", " --nowait
+Do not wait and check if pool is updated on a client. This is useful
+when calling a lot of "
+.B lctl
+pool_*" in a row. This avoids revoking the clients "CONFIG" lock for each
+command (by default clients retake their lock and update their configurations
+in a delay between 5-10s).
+
 .SH EXAMPLES
 .TP
 .B # lfs pool_new testfs.local
index f2db9dc..3fca59e 100644 (file)
@@ -748,14 +748,31 @@ struct mgs_modify_lookup {
        int             mml_modified;
 };
 
-static int mgs_check_record_match(const struct lu_env *env,
-                               struct llog_handle *llh,
-                               struct llog_rec_hdr *rec, void *data)
+enum mgs_search_pool_status {
+       POOL_STATUS_NONE = 0,
+       POOL_STATUS_EXIST,
+       POOL_STATUS_OST_EXIST,
+};
+
+struct mgs_search_pool_data {
+       char                            *msp_tgt;
+       char                            *msp_fs;
+       char                            *msp_pool;
+       char                            *msp_ost;
+       enum mgs_search_pool_status     msp_status;
+       bool                            msp_skip;
+};
+
+static int mgs_search_pool_cb(const struct lu_env *env,
+                             struct llog_handle *llh,
+                             struct llog_rec_hdr *rec, void *data)
 {
-       struct cfg_marker *mc_marker = data;
-       struct cfg_marker *marker;
+       struct mgs_search_pool_data *d = data;
        struct lustre_cfg *lcfg = REC_DATA(rec);
        int cfg_len = REC_DATA_LEN(rec);
+       char *fsname;
+       char *poolname;
+       char *ostname = NULL;
        int rc;
        ENTRY;
 
@@ -770,48 +787,97 @@ static int mgs_check_record_match(const struct lu_env *env,
                RETURN(rc);
        }
 
-       /* We only care about markers */
-       if (lcfg->lcfg_command != LCFG_MARKER)
+       /* check if section is skipped */
+       if (lcfg->lcfg_command == LCFG_MARKER) {
+               struct cfg_marker *marker = lustre_cfg_buf(lcfg, 1);
+
+               if (!(marker->cm_flags & CM_END))
+                       RETURN(0);
+
+               d->msp_skip = (marker->cm_flags & CM_SKIP) ||
+                       strcmp(d->msp_tgt, marker->cm_tgtname) != 0;
+
                RETURN(0);
+       }
 
-       marker = lustre_cfg_buf(lcfg, 1);
+       if (d->msp_skip)
+               RETURN(0);
 
-       if (marker->cm_flags & CM_SKIP)
+       switch (lcfg->lcfg_command) {
+       case LCFG_POOL_ADD:
+       case LCFG_POOL_REM:
+               ostname = lustre_cfg_string(lcfg, 3);
+               fallthrough;
+       case LCFG_POOL_NEW:
+       case LCFG_POOL_DEL:
+               fsname = lustre_cfg_string(lcfg, 1);
+               poolname = lustre_cfg_string(lcfg, 2);
+               break;
+       default:
                RETURN(0);
+       }
 
-       if ((strcmp(mc_marker->cm_comment, marker->cm_comment) == 0) &&
-               (strcmp(mc_marker->cm_tgtname, marker->cm_tgtname) == 0)) {
-               /* Found a non-skipped marker match */
-               CDEBUG(D_MGS, "Matched rec %u marker %d flag %x %s %s\n",
-                       rec->lrh_index, marker->cm_step,
-                       marker->cm_flags, marker->cm_tgtname,
-                       marker->cm_comment);
-               rc = LLOG_PROC_BREAK;
+       if (strcmp(d->msp_fs, fsname) != 0)
+               RETURN(0);
+       if (strcmp(d->msp_pool, poolname) != 0)
+               RETURN(0);
+       if (ostname && d->msp_ost && (strcmp(d->msp_ost, ostname) != 0))
+               RETURN(0);
+
+       /* Found a non-skipped marker match */
+       CDEBUG(D_MGS, "Matched pool rec %u cmd:0x%x %s.%s %s\n",
+              rec->lrh_index, lcfg->lcfg_command, fsname, poolname,
+              ostname ? ostname : "");
+
+       switch (lcfg->lcfg_command) {
+       case LCFG_POOL_ADD:
+               d->msp_status = POOL_STATUS_OST_EXIST;
+               RETURN(LLOG_PROC_BREAK);
+       case LCFG_POOL_REM:
+               d->msp_status = POOL_STATUS_EXIST;
+               RETURN(LLOG_PROC_BREAK);
+       case LCFG_POOL_NEW:
+               d->msp_status = POOL_STATUS_EXIST;
+               RETURN(LLOG_PROC_BREAK);
+       case LCFG_POOL_DEL:
+               d->msp_status = POOL_STATUS_NONE;
+               RETURN(LLOG_PROC_BREAK);
+       default:
+               break;
        }
 
-       RETURN(rc);
+       RETURN(0);
 }
 
 /**
- * Check an existing config log record with matching comment and device
+ * Search a pool in a MGS configuration.
  * Return code:
- * 0 - checked successfully,
- * LLOG_PROC_BREAK - record matches
+ * positive - return the status of the pool,
  * negative - error
  */
-static int mgs_check_marker(const struct lu_env *env, struct mgs_device *mgs,
-               struct fs_db *fsdb, struct mgs_target_info *mti,
-               char *logname, char *devname, char *comment)
+static
+int mgs_search_pool(const struct lu_env *env, struct mgs_device *mgs,
+                   struct fs_db *fsdb, struct mgs_target_info *mti,
+                   char *logname, char *devname, char *fsname, char *poolname,
+                   char *ostname)
 {
        struct llog_handle *loghandle;
        struct llog_ctxt *ctxt;
-       struct cfg_marker *mc_marker;
+       struct mgs_search_pool_data d;
+       int status = POOL_STATUS_NONE;
        int rc;
 
        ENTRY;
 
        LASSERT(mutex_is_locked(&fsdb->fsdb_mutex));
-       CDEBUG(D_MGS, "mgs check %s/%s/%s\n", logname, devname, comment);
+
+
+       d.msp_tgt = devname;
+       d.msp_fs = fsname;
+       d.msp_pool = poolname;
+       d.msp_ost = ostname;
+       d.msp_status = POOL_STATUS_NONE;
+       d.msp_skip = false;
 
        ctxt = llog_get_context(mgs->mgs_obd, LLOG_CONFIG_ORIG_CTXT);
        LASSERT(ctxt != NULL);
@@ -829,32 +895,16 @@ static int mgs_check_marker(const struct lu_env *env, struct mgs_device *mgs,
        if (llog_get_size(loghandle) <= 1)
                GOTO(out_close, rc = 0);
 
-       OBD_ALLOC_PTR(mc_marker);
-       if (!mc_marker)
-               GOTO(out_close, rc = -ENOMEM);
-       if (strlcpy(mc_marker->cm_comment, comment,
-               sizeof(mc_marker->cm_comment)) >=
-               sizeof(mc_marker->cm_comment))
-               GOTO(out_free, rc = -E2BIG);
-       if (strlcpy(mc_marker->cm_tgtname, devname,
-               sizeof(mc_marker->cm_tgtname)) >=
-               sizeof(mc_marker->cm_tgtname))
-               GOTO(out_free, rc = -E2BIG);
-
-       rc = llog_process(env, loghandle, mgs_check_record_match,
-                       (void *)mc_marker, NULL);
-
-out_free:
-       OBD_FREE_PTR(mc_marker);
+       rc = llog_reverse_process(env, loghandle, mgs_search_pool_cb, &d, NULL);
+       if (rc == LLOG_PROC_BREAK)
+               status = d.msp_status;
 
 out_close:
        llog_close(env, loghandle);
 out_pop:
-       if (rc && rc != LLOG_PROC_BREAK)
-               CDEBUG(D_ERROR, "%s: mgs check %s/%s failed: rc = %d\n",
-                       mgs->mgs_obd->obd_name, mti->mti_svname, comment, rc);
        llog_ctxt_put(ctxt);
-       RETURN(rc);
+
+       RETURN(rc < 0 ? rc : status);
 }
 
 static int mgs_modify_handler(const struct lu_env *env,
@@ -5814,6 +5864,75 @@ int mgs_nodemap_cmd(const struct lu_env *env, struct mgs_device *mgs,
        RETURN(rc);
 }
 
+static inline
+int mgs_pool_check_ostname(struct fs_db *fsdb, char *fsname, char *ostname)
+{
+       char *ptr;
+       unsigned int index;
+
+       /* check if ostname match fsname */
+       ptr = strrchr(ostname, '-');
+       if (!ptr || (strncmp(fsname, ostname, ptr - ostname) != 0))
+               RETURN(-EINVAL);
+
+       ptr++;
+       if (sscanf(ptr, "OST%04x_UUID", &index) != 1)
+               return -EINVAL;
+       if (index > INDEX_MAP_MAX_VALUE)
+               return -ERANGE;
+       if (!test_bit(index, fsdb->fsdb_ost_index_map))
+               return -ENODEV;
+
+       return 0;
+}
+
+static
+int mgs_pool_sanity(const struct lu_env *env, struct mgs_device *mgs,
+                   struct fs_db *fsdb, struct mgs_target_info *mti,
+                   char *logname, char *devname, enum lcfg_command_type cmd,
+                   char *fsname, char *poolname, char *ostname)
+{
+       char *lov = fsdb->fsdb_clilov;
+       int status;
+       int rc = 0;
+
+       status = mgs_search_pool(env, mgs, fsdb, mti, logname, lov,
+                                fsname, poolname, ostname);
+       if (status < 0)
+               return status;
+
+       switch (cmd) {
+       case LCFG_POOL_NEW:
+               if (status >= POOL_STATUS_EXIST)
+                       rc = -EEXIST;
+               break;
+       case LCFG_POOL_ADD:
+               if (status == POOL_STATUS_NONE)
+                       rc = -ENOMEDIUM;
+               else if (status == POOL_STATUS_OST_EXIST)
+                       rc = -EEXIST;
+               break;
+       case LCFG_POOL_REM:
+               if (status == POOL_STATUS_NONE)
+                       rc = -ENOMEDIUM;
+               if (status != POOL_STATUS_OST_EXIST)
+                       rc = -ENOENT;
+               break;
+       case LCFG_POOL_DEL:
+               if (status == POOL_STATUS_NONE)
+                       rc = -ENOENT;
+               if (status == POOL_STATUS_OST_EXIST)
+                       rc = -ENOTEMPTY;
+               break;
+       default:
+               rc = -EINVAL;
+               break;
+       }
+
+       return rc;
+}
+
+
 int mgs_pool_cmd(const struct lu_env *env, struct mgs_device *mgs,
                 enum lcfg_command_type cmd, char *fsname,
                 char *poolname, char *ostname)
@@ -5821,15 +5940,18 @@ int mgs_pool_cmd(const struct lu_env *env, struct mgs_device *mgs,
        struct fs_db *fsdb;
        char *lovname;
        char *logname;
-       char *label = NULL, *canceled_label = NULL;
+       char *label = NULL;
+       char *canceled_label = NULL;
        int label_sz;
        struct mgs_target_info *mti = NULL;
-       bool checked = false;
-       bool locked = false;
-       bool free = false;
        int rc, i;
        ENTRY;
 
+       if ((cmd == LCFG_POOL_REM || cmd == LCFG_POOL_ADD) && !ostname)
+               RETURN(-EINVAL);
+       if ((cmd == LCFG_POOL_DEL || cmd == LCFG_POOL_NEW) && ostname)
+               ostname = NULL;
+
        rc = mgs_find_or_make_fsdb(env, mgs, fsname, &fsdb);
        if (rc) {
                CERROR("Can't get db for %s\n", fsname);
@@ -5837,20 +5959,15 @@ int mgs_pool_cmd(const struct lu_env *env, struct mgs_device *mgs,
        }
        if (test_bit(FSDB_LOG_EMPTY, &fsdb->fsdb_flags)) {
                CERROR("%s is not defined\n", fsname);
-               free = true;
+               mgs_unlink_fsdb(mgs, fsdb);
                GOTO(out_fsdb, rc = -EINVAL);
        }
 
        label_sz = 10 + strlen(fsname) + strlen(poolname);
-
-       /* check if ostname match fsname */
-       if (ostname != NULL) {
-               char *ptr;
-
-               ptr = strrchr(ostname, '-');
-               if ((ptr == NULL) ||
-                   (strncmp(fsname, ostname, ptr-ostname) != 0))
-                       RETURN(-EINVAL);
+       if (ostname) {
+               rc =  mgs_pool_check_ostname(fsdb, fsname, ostname);
+               if (rc)
+                       GOTO(out_fsdb, rc);
                label_sz += strlen(ostname);
        }
 
@@ -5877,6 +5994,7 @@ int mgs_pool_cmd(const struct lu_env *env, struct mgs_device *mgs,
                OBD_ALLOC(canceled_label, label_sz);
                if (canceled_label == NULL)
                        GOTO(out_label, rc = -ENOMEM);
+
                sprintf(label, "del %s.%s", fsname, poolname);
                sprintf(canceled_label, "new %s.%s", fsname, poolname);
                break;
@@ -5885,94 +6003,77 @@ int mgs_pool_cmd(const struct lu_env *env, struct mgs_device *mgs,
        }
 
        OBD_ALLOC_PTR(mti);
-       if (mti == NULL)
+       if (!mti)
                GOTO(out_cancel, rc = -ENOMEM);
-       strncpy(mti->mti_svname, "lov pool", sizeof(mti->mti_svname));
+       strscpy(mti->mti_svname, "lov pool", sizeof(mti->mti_svname));
 
        mutex_lock(&fsdb->fsdb_mutex);
-       locked = true;
-       /* write pool def to all MDT logs */
-       for (i = 0; i < INDEX_MAP_SIZE * 8; i++) {
-               if (test_bit(i,  fsdb->fsdb_mdt_index_map)) {
-                       rc = name_create_mdt_and_lov(&logname, &lovname,
-                                                    fsdb, i);
-                       if (rc)
-                               GOTO(out_mti, rc);
-
-                       if (!checked && (canceled_label == NULL)) {
-                               rc = mgs_check_marker(env, mgs, fsdb, mti,
-                                                     logname, lovname, label);
-                               if (rc) {
-                                       name_destroy(&logname);
-                                       name_destroy(&lovname);
-                                       GOTO(out_mti,
-                                            rc = (rc == LLOG_PROC_BREAK ?
-                                                  -EEXIST : rc));
-                               }
-                               checked = true;
-                       }
-                       if (canceled_label != NULL)
-                               rc = mgs_modify(env, mgs, fsdb, mti, logname,
-                                               lovname, canceled_label,
-                                               CM_SKIP);
-
-                       if (rc >= 0)
-                               rc = mgs_write_log_pool(env, mgs, logname,
-                                                       fsdb, lovname, cmd,
-                                                       fsname, poolname,
-                                                       ostname, label);
-                       name_destroy(&logname);
-                       name_destroy(&lovname);
-                       if (rc)
-                               GOTO(out_mti, rc);
-               }
-       }
 
        rc = name_create(&logname, fsname, "-client");
        if (rc)
-               GOTO(out_mti, rc);
+               GOTO(out_unlock, rc);
 
-       if (!checked && (canceled_label == NULL)) {
-               rc = mgs_check_marker(env, mgs, fsdb, mti, logname,
-                                     fsdb->fsdb_clilov, label);
-               if (rc) {
-                       name_destroy(&logname);
-                       GOTO(out_mti, rc = (rc == LLOG_PROC_BREAK ?
-                                           -EEXIST : rc));
-               }
-       }
-       if (canceled_label != NULL) {
+       rc = mgs_pool_sanity(env, mgs, fsdb, mti, logname, fsdb->fsdb_clilov,
+                            cmd, fsname, poolname, ostname);
+       if (rc < 0)
+               GOTO(out_logname, rc);
+
+       if (canceled_label)
                rc = mgs_modify(env, mgs, fsdb, mti, logname,
                                fsdb->fsdb_clilov, canceled_label, CM_SKIP);
-               if (rc < 0) {
-                       name_destroy(&logname);
-                       GOTO(out_mti, rc);
-               }
-       }
+
+       if (rc < 0)
+               GOTO(out_logname, rc);
 
        rc = mgs_write_log_pool(env, mgs, logname, fsdb, fsdb->fsdb_clilov,
                                cmd, fsname, poolname, ostname, label);
-       mutex_unlock(&fsdb->fsdb_mutex);
-       locked = false;
+       if (rc < 0)
+               GOTO(out_logname, rc);
+
        name_destroy(&logname);
+
+       /* write pool def to all MDT logs */
+       for_each_set_bit(i, fsdb->fsdb_mdt_index_map, INDEX_MAP_SIZE) {
+               rc = name_create_mdt_and_lov(&logname, &lovname, fsdb, i);
+               if (rc)
+                       GOTO(out_unlock, rc);
+
+               if (canceled_label)
+                       rc = mgs_modify(env, mgs, fsdb, mti, logname, lovname,
+                                       canceled_label, CM_SKIP);
+
+               if (rc < 0)
+                       GOTO(out_names, rc);
+
+               rc = mgs_write_log_pool(env, mgs, logname, fsdb, lovname, cmd,
+                                       fsname, poolname, ostname, label);
+               if (rc)
+                       GOTO(out_names, rc);
+
+               name_destroy(&logname);
+               name_destroy(&lovname);
+       }
+       mutex_unlock(&fsdb->fsdb_mutex);
+
        /* request for update */
        mgs_revoke_lock(mgs, fsdb, MGS_CFG_T_CONFIG);
 
        GOTO(out_mti, rc);
 
+out_names:
+       name_destroy(&lovname);
+out_logname:
+       name_destroy(&logname);
+out_unlock:
+       mutex_unlock(&fsdb->fsdb_mutex);
 out_mti:
-       if (locked)
-               mutex_unlock(&fsdb->fsdb_mutex);
-       if (mti != NULL)
-               OBD_FREE_PTR(mti);
+       OBD_FREE_PTR(mti);
 out_cancel:
-       if (canceled_label != NULL)
+       if (canceled_label)
                OBD_FREE(canceled_label, label_sz);
 out_label:
        OBD_FREE(label, label_sz);
 out_fsdb:
-       if (free)
-               mgs_unlink_fsdb(mgs, fsdb);
        mgs_put_fsdb(mgs, fsdb);
 
        return rc;
index eebdc22..e275403 100755 (executable)
@@ -432,6 +432,45 @@ test_2e() {
 }
 run_test 2e "pool_add: OST already in a pool should be rejected"
 
+test_2f() {
+       local mgc_timeout_path=/sys/module/mgc/parameters/mgc_requeue_timeout_min
+       local mgc_timeout
+       local tgt="$FSNAME-OST0000_UUID"
+       local rc
+
+       (( MGS_VERSION >= $(version_code 2.15.60) )) ||
+               skip "Need MGS version at least 2.15.60"
+
+       mgc_timeout=$(cat $mgc_timeout_path)
+       echo 5 > $mgc_timeout_path
+       stack_trap "echo $mgc_timeout > $mgc_timeout_path"
+
+       do_facet mgs timeout 4 $LCTL pool_new -n $FSNAME.$POOL || rc=$?
+       ((rc != 124)) || error "lctl pool_new -n $FSNAME.$POOL timeout"
+       ((rc == 0)) || error "lctl pool_new -n $FSNAME.$POOL failed"
+
+       do_facet mgs timeout 4 $LCTL pool_add -n $FSNAME.$POOL $tgt || rc=$?
+       ((rc != 124)) || error "lctl pool_new -n $FSNAME.$POOL timeout"
+       ((rc == 0)) || error "lctl pool_new -n $FSNAME.$POOL failed"
+
+       wait_update_facet mds1 \
+               "lctl pool_list $FSNAME.$POOL 2> /dev/null | sed '1d'" "$tgt" ||
+               error "Add $tgt to $FSNAME.$POOL failed"
+
+       do_facet mgs timeout 4 $LCTL pool_remove -n $FSNAME.$POOL $tgt || rc=$?
+       ((rc != 124)) || error "lctl pool_remove -n $FSNAME.$POOL timeout"
+       ((rc == 0)) || error "lctl pool_remove -n $FSNAME.$POOL failed"
+
+       do_facet mgs timeout 4 $LCTL pool_destroy -n $FSNAME.$POOL || rc=$?
+       ((rc != 124)) || error "lctl pool_destroy -n $FSNAME.$POOL timeout"
+       ((rc == 0)) || error "lctl pool_destroy -n $FSNAME.$POOL failed"
+
+       wait_update_facet mds1 \
+               "lctl pool_list $FSNAME.$POOL &> /dev/null || echo destroy" "destroy" ||
+               error "destroy $FSNAME.$POOL failed"
+}
+run_test 2f "check -n|--nowait option"
+
 test_3a() {
        lctl get_param -n lov.$FSNAME-*.pools.$POOL 2>/dev/null
        [[ $? -ne 0 ]] || destroy_pool $POOL
index 825a2be..e62fc65 100644 (file)
@@ -262,16 +262,16 @@ command_t cmdlist[] = {
        {"===  Pools ==", NULL, 0, "pool management"},
        {"pool_new", jt_pool_cmd, 0,
         "add a new pool\n"
-        "usage: pool_new <fsname>.<poolname>"},
+        "usage: pool_new [-n|--nowait] <fsname>.<poolname>"},
        {"pool_add", jt_pool_cmd, 0,
         "add the named OSTs to the pool\n"
-        "usage: pool_add <fsname>.<poolname> <ostname indexed list>"},
+        "usage: pool_add [-n|--nowait] <fsname>.<poolname> <ostname indexed list>"},
        {"pool_remove", jt_pool_cmd, 0,
         "remove the named OST from the pool\n"
-        "usage: pool_remove <fsname>.<poolname> <ostname indexed list>"},
+        "usage: pool_remove [-n|--nowait] <fsname>.<poolname> <ostname indexed list>"},
        {"pool_destroy", jt_pool_cmd, 0,
         "destroy a pool\n"
-        "usage: pool_destroy <fsname>.<poolname>"},
+        "usage: pool_destroy [-n|--nowait] <fsname>.<poolname>"},
        {"pool_list", jt_pool_cmd, 0,
         "list pools and pools members\n"
         "usage: pool_list  <fsname>[.<poolname>] | <pathname>"},
index ebf1cee..c478905 100644 (file)
@@ -3272,94 +3272,6 @@ static char *get_event_filter(__u32 cmd)
        return NULL;
 }
 
-/**
- * Callback to search ostname in llog
- * - { index: 23, event: attach, device: lustre-OST0000-osc, type: osc,
- *     UUID: lustre-clilov_UUID }
- * - { index: 24, event: setup, device: lustre-OST0000-osc,
- *     UUID: lustre-OST0000_UUID, node: 192.168.0.120@tcp }
- * - { index: 25, event: add_osc, device: lustre-clilov,
- *     ost: lustre-OST0000_UUID, index: 0, gen: 1 }
- *
- * \param record[in]   pointer to llog record
- * \param data[in]     pointer to ostname
- *
- * \retval             1 if ostname is found
- *                     0 if ostname is not found
- *                     -ENOENT if ostname is deleted
- *                     -ENOMEM if Error allocating memory
- */
-static int llog_search_ost_cb(const char *record, void *data)
-{
-       char *ostname = data;
-       char ost_filter[MAX_STRING_SIZE] = {'\0'};
-       char *add_osc, *del_osc, *setup, *cleanup;
-       int rc = -ENOMEM;
-
-       add_osc = get_event_filter(LCFG_LOV_ADD_OBD);
-       if (!add_osc)
-               goto out;
-
-       del_osc = get_event_filter(LCFG_LOV_DEL_OBD);
-       if (!del_osc)
-               goto out;
-
-       setup = get_event_filter(LCFG_SETUP);
-       if (!setup)
-               goto out;
-
-       cleanup = get_event_filter(LCFG_CLEANUP);
-       if (!cleanup)
-               goto out;
-
-       if (ostname && ostname[0])
-               snprintf(ost_filter, sizeof(ost_filter), " %s,", ostname);
-
-       rc = 0;
-       if (strstr(record, ost_filter)) {
-               if (strstr(record, add_osc) || strstr(record, setup)) {
-                       rc = 1;
-                       goto out_setup;
-               }
-               if (strstr(record, del_osc) || strstr(record, cleanup)) {
-                       rc = -ENOENT;
-                       goto out_setup;
-               }
-       }
-out_setup:
-       free(cleanup);
-       free(setup);
-       free(del_osc);
-       free(add_osc);
-out:
-       return rc;
-}
-
-/**
- * Search ost in llog
- *
- * \param logname[in]          pointer to config log name
- * \param last_index[in]       the index of the last llog record
- * \param ostname[in]          pointer to ost name
- *
- * \retval                     1 if ostname is found
- *                             0 if ostname is not found
- */
-static int llog_search_ost(char *logname, long last_index, char *ostname)
-{
-       long start, end, inc = MAX_IOC_BUFLEN / 128;
-       int rc = 0;
-
-       for (end = last_index; end > 1; end -= inc) {
-               start = end - inc > 0 ? end - inc : 1;
-               rc = jt_llog_print_iter(logname, start, end, llog_search_ost_cb,
-                                       ostname, true, false);
-               if (rc)
-                       break;
-       }
-
-       return (rc == 1 ? 1 : 0);
-}
 
 struct llog_del_ost_priv {
        char *logname;
@@ -3468,158 +3380,6 @@ static int llog_del_ost(char *logname, long last_index, char *ostname,
        return rc;
 }
 
-struct llog_pool_data {
-       char lpd_fsname[LUSTRE_MAXFSNAME + 1];
-       char lpd_poolname[LOV_MAXPOOLNAME + 1];
-       char lpd_ostname[MAX_OBD_NAME + 1];
-       enum lcfg_command_type lpd_cmd_type;
-       bool lpd_pool_exists;
-       int lpd_ost_num;
-};
-
-/**
- * Called for each formatted line in the config log (within range).
- *
- * - { index: 74, event: new_pool, device: tfs-clilov, fsname: tfs, pool: tmp }
- * - { index: 77, event: add_pool, device: tfs-clilov, fsname: tfs, pool: tmp,
- *     ost: tfs-OST0000_UUID }
- * - { index: 224, event: remove_pool, device: tfs-clilov, fsname: tfs,
- *     pool: tmp, ost: tfs-OST0003_UUID }
- * - { index: 227, event: del_pool, device: tfs-clilov, fsname: tfs, pool: tmp }
- *
- * \param record[in]   pointer to llog record
- * \param data[in]     pointer to llog_pool_data
- *
- * \retval             1 if pool or OST is found
- *                     0 if pool or OST is not found
- *                     -ENOENT if pool or OST is removed
- */
-static int llog_search_pool_cb(const char *record, void *data)
-{
-       struct llog_pool_data *lpd = data;
-       char pool_filter[MAX_STRING_SIZE] = "";
-       char *new_pool, *del_pool, *add_pool, *rem_pool;
-       char *found = NULL;
-       int fs_pool_len = 0, rc = 0;
-
-       new_pool = get_event_filter(LCFG_POOL_NEW);
-       del_pool = get_event_filter(LCFG_POOL_DEL);
-       add_pool = get_event_filter(LCFG_POOL_ADD);
-       rem_pool = get_event_filter(LCFG_POOL_REM);
-       if (!new_pool || !del_pool || !add_pool || !rem_pool) {
-               rc = -ENOMEM;
-               goto out;
-       }
-
-       fs_pool_len = 16 + strlen(lpd->lpd_fsname) + strlen(lpd->lpd_poolname);
-       snprintf(pool_filter, fs_pool_len + 1, "fsname: %s, pool: %s",
-                lpd->lpd_fsname, lpd->lpd_poolname);
-
-       /* search poolname */
-       found = strstr(record, pool_filter);
-       if (found &&
-           (found[fs_pool_len] == ' ' || found[fs_pool_len] == ',')) {
-               if (strstr(record, new_pool)) {
-                       lpd->lpd_pool_exists = true;
-                       rc = 1;
-                       goto out;
-               }
-               if (strstr(record, del_pool)) {
-                       lpd->lpd_pool_exists = false;
-                       rc = -ENOENT;
-                       goto out;
-               }
-
-               if (lpd->lpd_cmd_type == LCFG_POOL_NEW ||
-                   lpd->lpd_cmd_type == LCFG_POOL_DEL) {
-                       /* In function mgs_pool_cmd(), a pool's pool_new/add
-                        * record will be marked as "SKIP" if its pool_destroy/
-                        * remove record is logged later. That means the "SKIP"
-                        * record won't be printed here and thus lpd_ost_num
-                        * doesn't need to be decreased as well.
-                        */
-                       if (strstr(record, add_pool))
-                               lpd->lpd_ost_num++;
-               } else if (lpd->lpd_ostname[0] != '\0') {
-                       if (strstr(record, lpd->lpd_ostname)) {
-                               lpd->lpd_pool_exists = true;
-                               if (strstr(record, add_pool)) {
-                                       lpd->lpd_ost_num = 1;
-                                       rc = 1;
-                                       goto out;
-                               }
-                               if (strstr(record, rem_pool)) {
-                                       lpd->lpd_ost_num = 0;
-                                       rc = -ENOENT;
-                                       goto out;
-                               }
-                       }
-               }
-       }
-out:
-       if (new_pool)
-               free(new_pool);
-       if (del_pool)
-               free(del_pool);
-       if (add_pool)
-               free(add_pool);
-       if (rem_pool)
-               free(rem_pool);
-
-       return rc;
-}
-
-/* Search pool and its ost in llog
- *
- * \param logname[in]          pointer to config log name
- * \param last_index[in]       the index of the last llog record
- * \param fsname[in]           pointer to filesystem name
- * \param poolname[in]         pointer pool name
- * \param ostname[in]          pointer to OST name(OSTnnnn-UUID)
- * \param cmd[in]              pool command type
- *
- * \retval                     < 0 on error
- *                             0 if pool is empty or OST is not found
- *                             1 if pool is not empty or OST is found
- */
-static int llog_search_pool(char *logname, long last_index, char *fsname,
-                           char *poolname, char *ostname,
-                           enum lcfg_command_type cmd)
-{
-       struct llog_pool_data lpd;
-       long start, end, inc = MAX_IOC_BUFLEN / 128;
-       int rc = 0;
-
-       memset(&lpd, 0, sizeof(lpd));
-       lpd.lpd_cmd_type = cmd;
-       lpd.lpd_pool_exists = false;
-       lpd.lpd_ost_num = 0;
-       strncpy(lpd.lpd_fsname, fsname, sizeof(lpd.lpd_fsname) - 1);
-       if (poolname && poolname[0])
-               strncpy(lpd.lpd_poolname, poolname,
-                       sizeof(lpd.lpd_poolname) - 1);
-       if (ostname && ostname[0])
-               strncpy(lpd.lpd_ostname, ostname, sizeof(lpd.lpd_ostname) - 1);
-
-       for (end = last_index; end > 1; end -= inc) {
-               start = end - inc > 0 ? end - inc : 1;
-               rc = jt_llog_print_iter(logname, start, end,
-                                       llog_search_pool_cb, &lpd, true, false);
-               if (rc) {
-                       if (rc == 1 && lpd.lpd_pool_exists)
-                               rc = lpd.lpd_ost_num ? 1 : 0;
-                       else if (rc == -ENOENT && lpd.lpd_pool_exists &&
-                                !lpd.lpd_ost_num)
-                               rc = 0;
-                       goto out;
-               }
-       }
-
-       rc = -ENOENT;
-out:
-       return rc;
-}
-
 static bool combined_mgs_mds(char *fsname)
 {
        glob_t path;
@@ -3635,103 +3395,85 @@ static bool combined_mgs_mds(char *fsname)
        return false;
 }
 
-/*
- * if pool is NULL, search ostname in target_obd
- * if pool is not NULL:
- *  - if pool not found returns errno < 0
- *  - if ostname is NULL, returns 1 if pool is not empty and 0 if pool empty
- *  - if ostname is not NULL, returns 1 if OST is in pool and 0 if not
- */
-int lctl_search_ost(char *fsname, char *poolname, char *ostname,
-                   enum lcfg_command_type cmd)
+static
+void pool_cmd_interpret_err(enum lcfg_command_type cmd, char *cmdname,
+                           char *fullpool, char *fsname, char *ostname, int rc)
 {
-       char logname[MAX_OBD_NAME] = {'\0'};
-       long last_index;
-
-       if (fsname && fsname[0] == '\0')
-               fsname = NULL;
-       if (!fsname)
-               return -EINVAL;
-
-       if (combined_mgs_mds(fsname))
-               return llapi_search_ost(fsname, poolname, ostname);
-
-       /* fetch the last_index of llog record */
-       snprintf(logname, sizeof(logname), "%s-client", fsname);
-       last_index = llog_last_index(logname);
-       if (last_index < 0)
-               return last_index;
-
-       /* if pool is NULL, search ostname in target_obd */
-       if (!poolname && ostname)
-               return llog_search_ost(logname, last_index, ostname);
+       switch (rc) {
+       case -ENAMETOOLONG:
+               fprintf(stderr,
+                       "%s: either the pool or file system name is too long (max pool name len is %d and file system name is %d)\n",
+                       jt_cmdname(cmdname), LOV_MAXPOOLNAME, LUSTRE_MAXFSNAME);
+               return;
+       case -EINVAL:
+               fprintf(stderr,
+                       "%s: can contain only alphanumeric characters, underscores, and dashes besides the required '.'\n",
+                       jt_cmdname(cmdname));
+               return;
+       default:
+               break;
+       }
 
-       return llog_search_pool(logname, last_index, fsname, poolname,
-                               ostname, cmd);
+       switch (cmd) {
+       case LCFG_POOL_NEW:
+       case LCFG_POOL_DEL:
+               if (rc == -EEXIST)
+                       fprintf(stderr, "%s: pool %s already exists\n",
+                               jt_cmdname(cmdname), fullpool);
+               else if (rc == -ENOENT)
+                       fprintf(stderr, "%s: pool %s not found\n",
+                               jt_cmdname(cmdname), fullpool);
+               else if (rc == -ENOTEMPTY)
+                       fprintf(stderr,
+                               "%s: pool %s not empty, please remove all members\n",
+                               jt_cmdname(cmdname), fullpool);
+               else if (rc)
+                       fprintf(stderr, "%s %s: %s\n",
+                               jt_cmdname(cmdname), fullpool, strerror(-rc));
+               break;
+       case LCFG_POOL_ADD:
+       case LCFG_POOL_REM:
+               if (rc == -ENOMEDIUM)
+                       fprintf(stderr, "%s: pool %s not found\n",
+                               jt_cmdname(cmdname), fullpool);
+               else if (rc == -EEXIST)
+                       fprintf(stderr, "%s: %s is already in pool %s\n",
+                               jt_cmdname(cmdname), ostname, fullpool);
+               else if (rc == -ENODEV)
+                       fprintf(stderr, "%s: %s is not part of the '%s' fs.\n",
+                               jt_cmdname(cmdname), ostname, fsname);
+               else if (rc == -ENOENT)
+                       fprintf(stderr, "%s: %s not found in pool %s\n",
+                               jt_cmdname(cmdname), ostname, fullpool);
+               else if (rc)
+                       fprintf(stderr, "%s %s %s: %s\n",
+                               jt_cmdname(cmdname), fullpool, ostname,
+                               strerror(-rc));
+               break;
+       default:
+               break;
+       }
 }
 
-static int check_pool_cmd(enum lcfg_command_type cmd, char *fsname,
-                         char *poolname, char *ostname)
+static int get_mgc_requeue_timeout_min(void)
 {
-       int rc;
+       const char *path = "/sys/module/mgc/parameters/mgc_requeue_timeout_min";
+       FILE *fp;
+       char buf[PATH_MAX] = { 0 };
+       int val = 5;
 
-       rc = lctl_search_ost(fsname, poolname, ostname, cmd);
-       if (rc < 0 && (cmd != LCFG_POOL_NEW)) {
-               fprintf(stderr, "Pool %s.%s not found\n",
-                       fsname, poolname);
-               return rc;
-       }
+       fp = fopen(path, "r");
+       if (!fp)
+               return val;
 
-       switch (cmd) {
-       case LCFG_POOL_NEW: {
-               if (ostname)
-                       return -EINVAL;
+       if (!fgets(buf, sizeof(buf), fp))
+               goto out;
 
-               if (rc >= 0) {
-                       fprintf(stderr, "Pool %s.%s already exists\n",
-                               fsname, poolname);
-                       return -EEXIST;
-               }
-               return 0;
-       }
-       case LCFG_POOL_DEL: {
-               if (ostname)
-                       return -EINVAL;
+       val = atoi(buf);
+out:
+       fclose(fp);
 
-               if (rc == 1) {
-                       fprintf(stderr,
-                               "Pool %s.%s not empty, please remove all members\n",
-                               fsname, poolname);
-                       return -ENOTEMPTY;
-               }
-               return 0;
-       }
-       case LCFG_POOL_ADD: {
-               if (rc == 1) {
-                       fprintf(stderr, "OST %s is already in pool %s.%s\n",
-                               ostname, fsname, poolname);
-                       return -EEXIST;
-               }
-               rc = lctl_search_ost(fsname, NULL, ostname, cmd);
-               if (rc == 0) {
-                       fprintf(stderr, "OST %s is not part of the '%s' fs.\n",
-                               ostname, fsname);
-                       return -ENOENT;
-               }
-               return 0;
-       }
-       case LCFG_POOL_REM: {
-               if (rc == 0) {
-                       fprintf(stderr, "OST %s not found in pool %s.%s\n",
-                               ostname, fsname, poolname);
-                       return -ENOENT;
-               }
-               return 0;
-       }
-       default:
-               break;
-       } /* switch */
-       return -EINVAL;
+       return val;
 }
 
 /*
@@ -3743,13 +3485,20 @@ static int check_pool_cmd(enum lcfg_command_type cmd, char *fsname,
 static int check_pool_cmd_result(enum lcfg_command_type cmd, char *fsname,
                                 char *poolname, char *ostname)
 {
-       int cpt = 10;
+       int cpt;
        int rc = 0;
 
+       /* mgs is standalone -> no client to wait */
+       if (!combined_mgs_mds(fsname))
+               return 0;
+
+       /* max time to wait a client */
+       cpt = 2 * get_mgc_requeue_timeout_min() + 2;
+
        switch (cmd) {
        case LCFG_POOL_NEW: {
                do {
-                       rc = lctl_search_ost(fsname, poolname, NULL, cmd);
+                       rc = llapi_search_ost(fsname, poolname, NULL);
                        if (rc == -ENODEV)
                                return rc;
                        if (rc < 0)
@@ -3768,7 +3517,7 @@ static int check_pool_cmd_result(enum lcfg_command_type cmd, char *fsname,
        }
        case LCFG_POOL_DEL: {
                do {
-                       rc = lctl_search_ost(fsname, poolname, NULL, cmd);
+                       rc = llapi_search_ost(fsname, poolname, NULL);
                        if (rc == -ENODEV)
                                return rc;
                        if (rc >= 0)
@@ -3787,7 +3536,7 @@ static int check_pool_cmd_result(enum lcfg_command_type cmd, char *fsname,
        }
        case LCFG_POOL_ADD: {
                do {
-                       rc = lctl_search_ost(fsname, poolname, ostname, cmd);
+                       rc = llapi_search_ost(fsname, poolname, ostname);
                        if (rc == -ENODEV)
                                return rc;
                        if (rc != 1)
@@ -3805,7 +3554,7 @@ static int check_pool_cmd_result(enum lcfg_command_type cmd, char *fsname,
        }
        case LCFG_POOL_REM: {
                do {
-                       rc = lctl_search_ost(fsname, poolname, ostname, cmd);
+                       rc = llapi_search_ost(fsname, poolname, ostname);
                        if (rc == -ENODEV)
                                return rc;
                        if (rc == 1)
@@ -3829,24 +3578,27 @@ static int check_pool_cmd_result(enum lcfg_command_type cmd, char *fsname,
 
 static int check_and_complete_ostname(char *fsname, char *ostname)
 {
+       char real_ostname[UUID_MAX];
        char *ptr;
-       char real_ostname[MAX_OBD_NAME + 1];
+       int len;
        char i;
 
+       if (strlen(ostname) >= sizeof(real_ostname))
+               return -ENAMETOOLONG;
+
        /* if OST name does not start with fsname, we add it */
        /* if not check if the fsname is the right one */
        ptr = strchr(ostname, '-');
        if (!ptr) {
-               snprintf(real_ostname, MAX_OBD_NAME + 1, "%s-%s", fsname,
-                        ostname);
+               len = snprintf(real_ostname, sizeof(real_ostname), "%s-%s",
+                              fsname, ostname);
+               if (len < 0 || len >= sizeof(real_ostname))
+                       return -ENAMETOOLONG;
        } else if (strncmp(ostname, fsname, strlen(fsname)) != 0) {
                fprintf(stderr, "%s does not start with fsname %s\n",
                        ostname, fsname);
                return -EINVAL;
        } else {
-               if (strlen(ostname) > sizeof(real_ostname) - 1)
-                       return -E2BIG;
-
                strncpy(real_ostname, ostname, sizeof(real_ostname));
        }
 
@@ -3871,7 +3623,11 @@ static int check_and_complete_ostname(char *fsname, char *ostname)
        /* real_ostname is fsname-OSTXXXX????? */
        /* if OST name does not end with _UUID, we add it */
        if (*ptr == '\0') {
-               strcat(real_ostname, "_UUID");
+               len = sizeof(real_ostname) - strlen(real_ostname) - 1;
+               if (sizeof("_UUID") - 1 > len)
+                       return -ENAMETOOLONG;
+
+               strncat(real_ostname, "_UUID", len);
        } else if (strcmp(ptr, "_UUID") != 0) {
                fprintf(stderr,
                        "ostname %s does not end with _UUID\n", ostname);
@@ -3884,8 +3640,7 @@ static int check_and_complete_ostname(char *fsname, char *ostname)
 
 /* returns 0 or -errno */
 static int pool_cmd(enum lcfg_command_type cmd, char *cmdname,
-                   char *fullpoolname, char *fsname, char *poolname,
-                   char *ostname)
+                   char *fullpoolname, char *fsname, char *ostname)
 {
        int rc = 0;
        struct obd_ioctl_data data;
@@ -3893,13 +3648,6 @@ static int pool_cmd(enum lcfg_command_type cmd, char *cmdname,
        struct lustre_cfg *lcfg;
        char rawbuf[MAX_IOC_BUFLEN], *buf = rawbuf;
 
-       rc = check_pool_cmd(cmd, fsname, poolname, ostname);
-       if (rc == -ENODEV)
-               fprintf(stderr,
-                       "Can't verify pool command since there is no local MDT or client, proceeding anyhow...\n");
-       else if (rc)
-               return rc;
-
        lustre_cfg_bufs_reset(&bufs, NULL);
        lustre_cfg_bufs_set_string(&bufs, 0, cmdname);
        lustre_cfg_bufs_set_string(&bufs, 1, fullpoolname);
@@ -3931,22 +3679,9 @@ static int pool_cmd(enum lcfg_command_type cmd, char *cmdname,
        }
        rc = l_ioctl(OBD_DEV_ID, OBD_IOC_POOL, buf);
 out:
-       if (rc)
-               rc = -errno;
-       switch (rc) {
-       case -ENAMETOOLONG:
-               fprintf(stderr,
-                       "error: %s: either the pool or file system name is too long (max pool name len is %d and file system name is %d)\n",
-                       jt_cmdname(cmdname), LOV_MAXPOOLNAME, LUSTRE_MAXFSNAME);
-               break;
-       case -EINVAL:
-               fprintf(stderr,
-                       "error: %s can contain only alphanumeric characters, underscores, and dashes besides the required '.'\n",
-                       jt_cmdname(cmdname));
-       default:
-               break;
-       }
        free(lcfg);
+       pool_cmd_interpret_err(cmd, cmdname, fullpoolname, fsname, ostname, rc);
+
        return rc;
 }
 
@@ -5175,182 +4910,256 @@ static bool get_pools_path(char *fsname)
        return (rc == 0);
 }
 
-static int extract_fsname_poolname(char **argv, char *fsname, char *poolname)
+#define POOL_LIST 0
+static
+int parse_pool_cmd_args(int argc, char **argv, bool *wait,
+                       enum lcfg_command_type *cmd, char **fullpool,
+                       char *fsname, char *poolname,
+                       char ***ostargv, int *ostargc)
 {
-       char *cmd = argv[0], *param = argv[1];
-       char *ptr;
-       int rc, fsname_len;
+       char *cmdname, *param, *ptr;
+       int fsname_len;
 
-       ptr = strchr(param, '.');
-       if (!ptr) {
-               if (strcmp(cmd, "pool_list") == 0) {
-                       snprintf(fsname, LUSTRE_MAXFSNAME + 1, "%s", param);
-                       poolname[0] = '\0';
-                       goto out;
-               }
-               fprintf(stderr, ". is missing in %s\n", param);
-               rc = -EINVAL;
-               goto err;
+       *wait = true;
+       *cmd = POOL_LIST;
+       *fullpool = NULL;
+       fsname[0] = '\0';
+       poolname[0] = '\0';
+       *ostargv = NULL;
+       *ostargc = 0;
+
+       if (argc < 1)
+               return CMD_NONE;
+
+       cmdname = *argv;
+       if (argc < 2)
+               return CMD_HELP;
+
+       argc--;
+       argv++;
+       if (strcmp(*argv, "--help") == 0 || strcmp(*argv, "-h") == 0)
+               return CMD_HELP;
+
+       if (strcmp(*argv, "--nowait") == 0 || strcmp(*argv, "-n") == 0) {
+               *wait = false;
+               argc--;
+               argv++;
        }
 
-       fsname_len = ptr - param;
-       if (fsname_len == 0) {
-               fprintf(stderr, "fsname is empty\n");
-               rc = -EINVAL;
-               goto err;
-       } else if (fsname_len > LUSTRE_MAXFSNAME) {
-               fprintf(stderr, "fsname is too long\n");
-               rc = -EINVAL;
-               goto err;
+       if (!argc)
+               return CMD_HELP;
+
+       param = *argv;
+       *fullpool = param;
+       argc--;
+       argv++;
+       if (argc) {
+               *ostargv = argv;
+               *ostargc = argc;
+       }
+
+       if (strcmp("pool_new", cmdname) == 0)
+               *cmd = LCFG_POOL_NEW;
+       else if (strcmp("pool_destroy", cmdname) == 0)
+               *cmd = LCFG_POOL_DEL;
+       else if (strcmp("pool_remove", cmdname) == 0)
+               *cmd = LCFG_POOL_REM;
+       else if (strcmp("pool_add", cmdname) == 0)
+               *cmd = LCFG_POOL_ADD;
+       else if (strcmp("pool_list", cmdname) == 0)
+               *cmd = POOL_LIST;
+       else
+               return -EINVAL;
+
+       if ((*cmd == LCFG_POOL_REM || *cmd == LCFG_POOL_ADD) && !*ostargc)
+               return CMD_HELP;
+
+       ptr = strchr(param, '.');
+       if (!ptr && *cmd == POOL_LIST)
+               fsname_len = strlen(param);
+       else if (ptr)
+               fsname_len = ptr - param;
+       else
+               return -EINVAL;
+
+       if (fsname_len == 0)
+               return -EINVAL;
+       if (fsname_len > LUSTRE_MAXFSNAME) {
+               fprintf(stderr, "%s: fsname is too long\n", cmdname);
+               return -ENAMETOOLONG;
        }
 
        strncpy(fsname, param, fsname_len);
        fsname[fsname_len] = '\0';
+       if (!ptr)
+               return 0;
+
        ++ptr;
+       if (ptr[0] == '\0')
+               return -EINVAL;
 
-       if (ptr[0] == '\0') {
-               fprintf(stderr, "poolname is empty\n");
-               rc = -EINVAL;
-               goto err;
+       if (strlen(ptr) > LOV_MAXPOOLNAME) {
+               fprintf(stderr, "%s: poolname is too long\n", cmdname);
+               return -ENAMETOOLONG;
        }
 
-       snprintf(poolname, LOV_MAXPOOLNAME + 1, "%s", ptr);
+       strncpy(poolname, ptr, LOV_MAXPOOLNAME + 1);
        if (lov_pool_is_reserved(poolname)) {
-               fprintf(stderr, "poolname cannot be '%s'\n", poolname);
+               fprintf(stderr, "%s: poolname cannot be '%s'\n",
+                       cmdname, poolname);
                return -EINVAL;
        }
-out:
-       return 0;
 
-err:
-       fprintf(stderr, "argument %s must be <fsname>.<poolname>\n", param);
-       return rc;
+       if (*wait && (*cmd != POOL_LIST) && !combined_mgs_mds(fsname)) {
+               int min_wait = get_mgc_requeue_timeout_min();
+
+               *wait = false;
+               fprintf(stderr,
+                       "Warning, standalone MGS for \"%s\", unable to check pool updates on clients.\n"
+                       "Verify if pools are updated on a client (client sync delay: %d-%ds).\n\n",
+                       fsname, min_wait, 2 * min_wait);
+       }
+
+       return 0;
 }
 
-int jt_pool_cmd(int argc, char **argv)
+struct pool_ost_cmd {
+       int     rc;
+       char    ostname[UUID_MAX];
+};
+
+static
+int extract_ost_list(int argc, char **argv, char *fsname,
+                    struct pool_ost_cmd **out_osts)
 {
-       enum lcfg_command_type cmd;
-       char fsname[LUSTRE_MAXFSNAME + 1];
-       char poolname[LOV_MAXPOOLNAME + 1];
-       int i, rc;
+       char format[2 * UUID_MAX];
+       int i;
        int *array = NULL, array_sz;
-       struct {
-               int     rc;
-               char    ostname[MAX_OBD_NAME + 1];
-       } *cmds = NULL;
-       int cmds_nr = 0, cmd_start = 0;
-
-       switch (argc) {
-       case 0:
-       case 1: { rc = CMD_HELP; goto out_cmd; }
-       case 2: {
-               rc = extract_fsname_poolname(argv, fsname, poolname);
-               if (rc)
-                       break;
+       struct pool_ost_cmd *ostlist = NULL;
+       int ostcnt = 0;
+       int rc;
 
-               if (strcmp("pool_new", argv[0]) == 0) {
-                       cmd = LCFG_POOL_NEW;
-               } else if (strcmp("pool_destroy", argv[0]) == 0) {
-                       cmd = LCFG_POOL_DEL;
-               } else if (strcmp("pool_list", argv[0]) == 0) {
-                       if (get_pools_path(fsname))
-                               return llapi_poollist(argv[1]);
-                       if (get_mgs_device() > 0)
-                               return llog_poollist(fsname, poolname);
-                       fprintf(stderr,
-                               "Cannot run pool_list command since there is no local MGS/MDT or client\n");
-                       rc = CMD_HELP;
-                       goto out_cmd;
-               } else {
-                       rc = CMD_HELP;
-                       goto out_cmd;
+       if (argc < 1)
+               return -EINVAL;
+
+       /* generate full list of OSTs */
+       for (i = 0; i < argc; i++) {
+               int j, start;
+               struct pool_ost_cmd *tmp;
+
+               if (strlen(argv[i]) >= sizeof(format)) {
+                       rc = -EINVAL;
+                       goto err_freelist;
                }
 
-               rc = pool_cmd(cmd, argv[0], argv[1], fsname, poolname, NULL);
-               if (rc)
-                       break;
+               array_sz = get_array_idx(argv[i], format, &array);
+               if (array_sz == 0) {
+                       rc = -EINVAL;
+                       goto err_freelist;
+               }
 
-               check_pool_cmd_result(cmd, fsname, poolname, NULL);
-               break;
-       }
-       default: {
-               char format[2 * MAX_OBD_NAME];
+               start = ostcnt;
+               ostcnt += array_sz;
+               tmp = realloc(ostlist, ostcnt * sizeof(*ostlist));
+               if (!tmp) {
+                       rc = -ENOMEM;
+                       goto err_freearr;
+               }
 
-               if (strcmp("pool_remove", argv[0]) == 0) {
-                       cmd = LCFG_POOL_REM;
-               } else if (strcmp("pool_add", argv[0]) == 0) {
-                       cmd = LCFG_POOL_ADD;
-               } else {
-                       rc = CMD_HELP;
-                       goto out_cmd;
+               ostlist = tmp;
+               for (j = 0; j < array_sz; j++) {
+                       char tmp[UUID_MAX];
+
+                       rc = snprintf(tmp, sizeof(tmp), format, array[j]);
+                       if (rc < 0 || rc >= sizeof(tmp)) {
+                               rc = -ENAMETOOLONG;
+                               goto err_freearr;
+                       }
+
+                       rc = check_and_complete_ostname(fsname, tmp);
+                       if (rc)
+                               goto err_freearr;
+
+                       strncpy(ostlist[start + j].ostname, tmp, UUID_MAX);
                }
 
-               rc = extract_fsname_poolname(argv, fsname, poolname);
-               if (rc)
-                       break;
+               free(array);
+       }
+       *out_osts = ostlist;
 
-               /* generate full list of OSTs */
-               for (i = 2; i < argc; i++) {
-                       int j;
+       return ostcnt;
 
-                       array_sz = get_array_idx(argv[i], format, &array);
-                       if (array_sz == 0)
-                               goto out;
+err_freearr:
+       free(array);
+err_freelist:
+       free(ostlist);
+       *out_osts = NULL;
+       fprintf(stderr, "Wrong format for OST list\n");
 
-                       cmd_start = cmds_nr;
-                       cmds_nr += array_sz;
-                       cmds = realloc(cmds, cmds_nr * sizeof(cmds[0]));
-                       if (cmds == NULL) {
-                               rc = -ENOMEM;
-                               goto out;
-                       }
+       return rc;
+}
 
-                       for (j = 0; j < array_sz; j++) {
-                               char *ostname = cmds[cmd_start + j].ostname;
-                               int rc2;
+int jt_pool_cmd(int argc, char **argv)
+{
+       enum lcfg_command_type cmd;
+       bool wait_client;
+       char *fullpool;
+       char fsname[LUSTRE_MAXFSNAME + 1];
+       char poolname[LOV_MAXPOOLNAME + 1];
+       int i, rc;
+       int ostargc = 0;
+       char **ostargv = NULL;
+       struct pool_ost_cmd *cmds = NULL;
+       int cmds_nr = 0;
 
-                               snprintf(ostname, MAX_OBD_NAME, format,
-                                        array[j]);
-                               ostname[MAX_OBD_NAME] = '\0';
+       rc = parse_pool_cmd_args(argc, argv, &wait_client, &cmd, &fullpool,
+                                fsname, poolname, &ostargv, &ostargc);
+       if (rc)
+               goto out;
 
-                               rc2 = check_and_complete_ostname(fsname,
-                                                               ostname);
-                               if (rc2) {
-                                       rc = rc ? rc : rc2;
-                                       goto out;
-                               }
-                       } /* for(j) */
-               } /* for(i) */
-               /* submit all commands */
-               for (i = 0; i < cmds_nr; i++) {
-                       cmds[i].rc = pool_cmd(cmd, argv[0], argv[1], fsname,
-                                             poolname, cmds[i].ostname);
-                       /* Return an err if any of the add/dels fail */
-                       if (!rc)
-                               rc = cmds[i].rc;
+       switch (cmd) {
+       case LCFG_POOL_NEW:
+       case LCFG_POOL_DEL:
+               rc = pool_cmd(cmd, argv[0], fullpool, fsname, NULL);
+               if (!rc && wait_client)
+                       check_pool_cmd_result(cmd, fsname, poolname, NULL);
+               break;
+       case LCFG_POOL_ADD:
+       case LCFG_POOL_REM:
+               rc = extract_ost_list(ostargc, ostargv, fsname, &cmds);
+               if (rc < 0)
+                       goto out;
+
+               cmds_nr = rc;
+               rc = 0;
+               for (i = 0; i < cmds_nr && rc != -EFAULT; i++) {
+                       cmds[i].rc = pool_cmd(cmd, argv[0], fullpool, fsname,
+                                             cmds[i].ostname);
+                       rc = cmds[i].rc ? cmds[i].rc : rc;
                }
+
                /* check results */
-               for (i = 0; i < cmds_nr; i++) {
-                       if (!cmds[i].rc) {
+               for (i = 0; i < cmds_nr && rc != -EFAULT; i++) {
+                       if (!rc && wait_client)
                                check_pool_cmd_result(cmd, fsname, poolname,
                                                      cmds[i].ostname);
-                       }
                }
-               fallthrough;
+
+               free(cmds);
+               break;
+       default:
+               if (get_pools_path(fsname))
+                       rc = llapi_poollist(fullpool);
+               else if (get_mgs_device() > 0)
+                       rc = llog_poollist(fsname, poolname);
        }
-       } /* switch */
+
+       return rc;
 
 out:
-       if (array)
-               free(array);
-       if (cmds)
-               free(cmds);
+       if (rc < 0)
+               fprintf(stderr, "%s: %s\n", argv[0], strerror(-rc));
 
-       if (rc != 0) {
-               errno = -rc;
-               perror(argv[0]);
-       }
-out_cmd:
        return rc;
 }