Don't run 'zpool iostat -c CMD' command on all vdevs, if vdevs specified

zpool iostat allows you to specify only certain vdevs to display.
Currently, if you run 'zpool iostat -c CMD vdev1 vdev2 ...'
on specific vdevs, it will actually run the command on *all* vdevs,
and just display the results for the vdevs you specify.  This patch
corrects the behavior to only run the command on the specified vdevs,
and also enables the zpool_iostat_005_pos.ksh tests.
    
Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Closes #5443

cmd/zpool/zpool_iter.c

@@ -360,7 +360,7 @@ for_each_vdev_run_cb(zpool_handle_t *zhp, nvlist_t *nv, void *cb_vcdl)
 	vdev_cmd_data_list_t *vcdl = cb_vcdl;
 	vdev_cmd_data_t *data;
 	char *path = NULL;
-	int i;
+	int i, match = 0;
 
 	if (nvlist_lookup_string(nv, ZPOOL_CONFIG_PATH, &path) != 0)
 		return (1);
@@ -374,6 +374,19 @@ for_each_vdev_run_cb(zpool_handle_t *zhp, nvlist_t *nv, void *cb_vcdl)
 		}
 	}
 
+	/* Check for whitelisted vdevs here, if any */
+	for (i = 0; i < vcdl->vdev_names_count; i++) {
+		if (strcmp(vcdl->vdev_names[i], zpool_vdev_name(g_zfs, zhp, nv,
+		    vcdl->cb_name_flags)) == 0) {
+			match = 1;
+			break; /* match */
+		}
+	}
+
+	/* If we whitelisted vdevs, and this isn't one of them, then bail out */
+	if (!match && vcdl->vdev_names_count)
+		return (0);
+
 	/*
 	 * Resize our array and add in the new element.
 	 */
@@ -437,19 +450,28 @@ all_pools_for_each_vdev_run_vcdl(vdev_cmd_data_list_t *vcdl)
 }
 
 /*
- * Run command 'cmd' on all vdevs in all pools.  Saves the first line of output
+ * Run command 'cmd' on all vdevs in all pools in argv.  Saves the first line of
- * from the command in vcdk->data[].line for all vdevs.
+ * output from the command in vcdk->data[].line for all vdevs.  If you want
+ * to run the command on only certain vdevs, fill in g_zfs, vdev_names,
+ * vdev_names_count, and cb_name_flags.  Otherwise leave them as zero.
  *
  * Returns a vdev_cmd_data_list_t that must be freed with
  * free_vdev_cmd_data_list();
  */
 vdev_cmd_data_list_t *
-all_pools_for_each_vdev_run(int argc, char **argv, char *cmd)
+all_pools_for_each_vdev_run(int argc, char **argv, char *cmd,
+    libzfs_handle_t *g_zfs, char **vdev_names, int vdev_names_count,
+    int cb_name_flags)
 {
 	vdev_cmd_data_list_t *vcdl;
 	vcdl = safe_malloc(sizeof (vdev_cmd_data_list_t));
 	vcdl->cmd = cmd;
 
+	vcdl->vdev_names = vdev_names;
+	vcdl->vdev_names_count = vdev_names_count;
+	vcdl->cb_name_flags = cb_name_flags;
+	vcdl->g_zfs = g_zfs;
+
 	/* Gather our list of all vdevs in all pools */
 	for_each_pool(argc, argv, B_TRUE, NULL,
 	    all_pools_for_each_vdev_gather_cb, vcdl);

cmd/zpool/zpool_main.c

@@ -312,7 +312,7 @@ get_usage(zpool_help_t idx) {
 		    "[-R root] [-F [-n]]\n"
 		    "\t    <pool | id> [newpool]\n"));
 	case HELP_IOSTAT:
-		return (gettext("\tiostat [-T d | u] [-ghHLpPvy] "
+		return (gettext("\tiostat [-c CMD] [-T d | u] [-ghHLpPvy] "
 		    "[[-lq]|[-r|-w]]\n"
 		    "\t    [[pool ...]|[pool vdev ...]|[vdev ...]] "
 		    "[interval [count]]\n"));
@@ -335,8 +335,8 @@ get_usage(zpool_help_t idx) {
 	case HELP_SCRUB:
 		return (gettext("\tscrub [-s] <pool> ...\n"));
 	case HELP_STATUS:
-		return (gettext("\tstatus [-gLPvxD] [-T d|u] [pool] ... "
+		return (gettext("\tstatus [-c CMD] [-gLPvxD] [-T d|u] [pool]"
-		    "[interval [count]]\n"));
+		    " ... [interval [count]]\n"));
 	case HELP_UPGRADE:
 		return (gettext("\tupgrade\n"
 		    "\tupgrade -v\n"
@@ -4055,8 +4055,13 @@ zpool_do_iostat(int argc, char **argv)
 			usage(B_FALSE);
 			break;
 		case '?':
-			(void) fprintf(stderr, gettext("invalid option '%c'\n"),
+			if (optopt == 'c') {
-			    optopt);
+				fprintf(stderr,
+				    gettext("Missing CMD for -c\n"));
+			} else {
+				fprintf(stderr,
+				    gettext("invalid option '%c'\n"), optopt);
+			}
 			usage(B_FALSE);
 		}
 	}
@@ -4256,9 +4261,10 @@ zpool_do_iostat(int argc, char **argv)
 				continue;
 			}
 
-			if (cmd != NULL)
+			if (cmd != NULL && cb.cb_verbose)
 				cb.vcdl = all_pools_for_each_vdev_run(argc,
-				    argv, cmd);
+				    argv, cmd, g_zfs, cb.cb_vdev_names,
+				    cb.cb_vdev_names_count, cb.cb_name_flags);
 
 			pool_list_iter(list, B_FALSE, print_iostat, &cb);
 
@@ -6113,8 +6119,13 @@ zpool_do_status(int argc, char **argv)
 			get_timestamp_arg(*optarg);
 			break;
 		case '?':
-			(void) fprintf(stderr, gettext("invalid option '%c'\n"),
+			if (optopt == 'c') {
-			    optopt);
+				fprintf(stderr,
+				    gettext("Missing CMD for -c\n"));
+			} else {
+				fprintf(stderr,
+				    gettext("invalid option '%c'\n"), optopt);
+			}
 			usage(B_FALSE);
 		}
 	}
@@ -6135,7 +6146,8 @@ zpool_do_status(int argc, char **argv)
 			print_timestamp(timestamp_fmt);
 
 		if (cmd != NULL)
-			cb.vcdl = all_pools_for_each_vdev_run(argc, argv, cmd);
+			cb.vcdl = all_pools_for_each_vdev_run(argc, argv, cmd,
+			    NULL, NULL, 0, 0);
 
 		ret = for_each_pool(argc, argv, B_TRUE, NULL,
 		    status_callback, &cb);

cmd/zpool/zpool_util.h

@@ -86,11 +86,21 @@ typedef struct vdev_cmd_data_list
 {
 	char *cmd;		/* Command to run */
 	unsigned int count;	/* Number of vdev_cmd_data items (vdevs) */
+
+	/* vars to whitelist only certain vdevs, if requested */
+	libzfs_handle_t *g_zfs;
+	char **vdev_names;
+	int vdev_names_count;
+	int cb_name_flags;
+
 	vdev_cmd_data_t *data;	/* Array of vdevs */
+
 } vdev_cmd_data_list_t;
 
 vdev_cmd_data_list_t *all_pools_for_each_vdev_run(int argc, char **argv,
-    char *cmd);
+    char *cmd, libzfs_handle_t *g_zfs, char **vdev_names, int vdev_names_count,
+    int cb_name_flags);
+
 void free_vdev_cmd_data_list(vdev_cmd_data_list_t *vcdl);
 
 #ifdef	__cplusplus

tests/runfiles/linux.run

@@ -362,7 +362,8 @@ user =
 
 [tests/functional/cli_user/zpool_iostat]
 tests = ['zpool_iostat_001_neg', 'zpool_iostat_002_pos',
-    'zpool_iostat_003_neg', 'zpool_iostat_004_pos']
+    'zpool_iostat_003_neg', 'zpool_iostat_004_pos',
+    'zpool_iostat_005_pos']
 user =
 
 [tests/functional/cli_user/zpool_list]

tests/zfs-tests/tests/functional/cli_user/zpool_iostat/zpool_iostat_005_pos.ksh

@@ -54,10 +54,27 @@ fi
 # a '/' when we specify the path (-P) flag. We check for "{}" to see if one
 # of the VDEV variables isn't set.
 #
-C1=$($ZPOOL iostat -Pv | $GREP -E '^\s+/' | $WC -l)
+C1=$($ZPOOL iostat -Pv $testpool | $GREP -E '^\s+/' | $WC -l)
-C2=$($ZPOOL iostat -Pv -c 'echo vdev_test{$VDEV_PATH}{$VDEV_UPATH}' | $GREP -E '^\s+/' | $GREP -v '{}' | $WC -l)
+C2=$($ZPOOL iostat -Pv -c 'echo vdev_test{$VDEV_PATH}{$VDEV_UPATH}' $testpool \
+    | $GREP -E '^\s+/' | $GREP -v '{}' | $WC -l)
 if [ "$C1" != "$C2" ] ; then
 	log_fail "zpool iostat -c failed, expected $C1 vdevs, got $C2"
 else
-	log_pass "zpool iostat -c passed, expected $C1 vdevs, got $C2"
+	log_note "zpool iostat -c passed, expected $C1 vdevs, got $C2"
+fi
+
+# Call iostat on only a specific vdev, and verify that the command only gets
+# run on the vdev.  We write the command results to a temp file to verify that
+# the command actually gets run, rather than just verifying that the results
+# are *displayed* for the specific vdev.
+TMP=$($MKTEMP)
+FIRST_VDEV=$($ZPOOL iostat -Pv $testpool | $GREP -Eo '^\s+/[^ ]+' | $HEAD -n 1)
+log_must $ZPOOL iostat -Pv -c "echo \$VDEV_PATH >> $TMP" $testpool \
+    $FIRST_VDEV > /dev/null
+C2=$($WC -w < $TMP)
+$RM $TMP
+if [ "$C2" != "1" ] ; then
+	log_fail "zpool iostat -c <VDEV> failed, expected 1 vdev, got $C2"
+else
+	log_note "zpool iostat -c <VDEV> passed, expected 1 vdev, got $C2"
 fi