[PATCH 0/2] btrfs-progs: fixes for the cli test group

[PATCH 0/2] btrfs-progs: fixes for the cli test group

[Qu Wenruo]

For the current devel branch, there are two failures for the cli test group:

- cli/009
  This is caused by a very recent (only in devel branch) refactor for
  btrfstune, which removes the ability to customize the return value.

  Fix it by adding a new @ret argument for usage() helper.

- cli/017
  This exists for a while, and it's caused by a recent kernel change.

  Fix the test case to handle it better.

Qu Wenruo (2):
  btrfs-progs: make usage() call to properly return an exit value
  btrfs-progs: tests: cli: fix 017 test case failure

 check/main.c                                | 4 ++--
 cmds/device.c                               | 4 ++--
 cmds/filesystem.c                           | 2 +-
 cmds/qgroup.c                               | 2 +-
 cmds/quota.c                                | 4 ++--
 cmds/receive.c                              | 4 ++--
 cmds/restore.c                              | 4 ++--
 cmds/subvolume-list.c                       | 2 +-
 cmds/subvolume.c                            | 2 +-
 common/help.c                               | 6 +++---
 common/help.h                               | 2 +-
 image/main.c                                | 3 +--
 mkfs/main.c                                 | 3 +--
 tests/cli-tests/017-fi-show-missing/test.sh | 2 +-
 tune/main.c                                 | 3 +--
 15 files changed, 22 insertions(+), 25 deletions(-)

-- 
2.39.1

[PATCH 1/2] btrfs-progs: make usage() call to properly return an exit value

[Qu Wenruo]

[BUG]
Currently cli/009 test case failed with different exit number:

  ====== RUN CHECK /home/adam/btrfs-progs/btrfstune --help
  usage: btrfstune [options] device
  [...]
  failed: /home/adam/btrfs-progs/btrfstune --help
  test failed for case 009-btrfstune

[CAUSE]
In tune/main.c, we have the following call on usage():

  static void print_usage(int ret)
  {
	usage(&tune_cmd);
	exit(ret);
  }

However usage() itself would always call exit(1):

  void usage(const struct cmd_struct *cmd)
  {
	usage_command_usagestr(cmd->usagestr, NULL, 0, true, true);
	exit(1);
  }

This makes prevents any caller of usage() to modify its exit number.

[FIX]
Add a new argument @ret for print_usage(), so we can properly return 0
for -h/--help usage.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c          | 4 ++--
 cmds/device.c         | 4 ++--
 cmds/filesystem.c     | 2 +-
 cmds/qgroup.c         | 2 +-
 cmds/quota.c          | 4 ++--
 cmds/receive.c        | 4 ++--
 cmds/restore.c        | 4 ++--
 cmds/subvolume-list.c | 2 +-
 cmds/subvolume.c      | 2 +-
 common/help.c         | 6 +++---
 common/help.h         | 2 +-
 image/main.c          | 3 +--
 mkfs/main.c           | 3 +--
 tune/main.c           | 3 +--
 14 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/check/main.c b/check/main.c
index 899d8bd88e2a..38e44eae6ef6 100644
--- a/check/main.c
+++ b/check/main.c
@@ -10075,7 +10075,7 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
 				break;
 			case '?':
 			case 'h':
-				usage(cmd);
+				usage(cmd, 0);
 			case GETOPT_VAL_REPAIR:
 				printf("enabling repair mode\n");
 				opt_check_repair = 1;
@@ -10130,7 +10130,7 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
 	}
 
 	if (check_argc_exact(argc - optind, 1))
-		usage(cmd);
+		usage(cmd, 1);
 
 	if (g_task_ctx.progress_enabled) {
 		g_task_ctx.tp = TASK_NOTHING;
diff --git a/cmds/device.c b/cmds/device.c
index 69fe3fb3b9a8..555ee13724d5 100644
--- a/cmds/device.c
+++ b/cmds/device.c
@@ -440,10 +440,10 @@ static int cmd_device_scan(const struct cmd_struct *cmd, int argc, char **argv)
 	devstart = optind;
 
 	if (all && forget)
-		usage(cmd);
+		usage(cmd, 1);
 
 	if (all && check_argc_max(argc - optind, 1))
-		usage(cmd);
+		usage(cmd, 1);
 
 	if (all || argc - optind == 0) {
 		if (forget) {
diff --git a/cmds/filesystem.c b/cmds/filesystem.c
index b189d7cb706d..bfcf7d1bcaf0 100644
--- a/cmds/filesystem.c
+++ b/cmds/filesystem.c
@@ -752,7 +752,7 @@ static int cmd_filesystem_show(const struct cmd_struct *cmd,
 	if (argc > optind) {
 		search = argv[optind];
 		if (*search == 0)
-			usage(cmd);
+			usage(cmd, 1);
 		type = check_arg_type(search);
 
 		/*
diff --git a/cmds/qgroup.c b/cmds/qgroup.c
index d58d9a91cb25..7389b386fc05 100644
--- a/cmds/qgroup.c
+++ b/cmds/qgroup.c
@@ -2136,7 +2136,7 @@ static int cmd_qgroup_limit(const struct cmd_struct *cmd, int argc, char **argv)
 		args.qgroupid = parse_qgroupid_or_path(argv[optind + 1]);
 		path = argv[optind + 2];
 	} else
-		usage(cmd);
+		usage(cmd, 1);
 
 	fd = btrfs_open_dir(path, &dirstream, 1);
 	if (fd < 0)
diff --git a/cmds/quota.c b/cmds/quota.c
index d11b0aaf8cbd..9ae614ea1231 100644
--- a/cmds/quota.c
+++ b/cmds/quota.c
@@ -79,7 +79,7 @@ static int cmd_quota_enable(const struct cmd_struct *cmd, int argc, char **argv)
 	ret = quota_ctl(BTRFS_QUOTA_CTL_ENABLE, argc, argv);
 
 	if (ret < 0)
-		usage(cmd);
+		usage(cmd, 1);
 	return ret;
 }
 static DEFINE_SIMPLE_COMMAND(quota_enable, "enable");
@@ -100,7 +100,7 @@ static int cmd_quota_disable(const struct cmd_struct *cmd,
 	ret = quota_ctl(BTRFS_QUOTA_CTL_DISABLE, argc, argv);
 
 	if (ret < 0)
-		usage(cmd);
+		usage(cmd, 1);
 	return ret;
 }
 static DEFINE_SIMPLE_COMMAND(quota_disable, "disable");
diff --git a/cmds/receive.c b/cmds/receive.c
index 1d623ae3ce90..8d5f8721850e 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -1762,9 +1762,9 @@ static int cmd_receive(const struct cmd_struct *cmd, int argc, char **argv)
 	}
 
 	if (dump && check_argc_exact(argc - optind, 0))
-		usage(cmd);
+		usage(cmd, 1);
 	if (!dump && check_argc_exact(argc - optind, 1))
-		usage(cmd);
+		usage(cmd, 1);
 
 	tomnt = argv[optind];
 
diff --git a/cmds/restore.c b/cmds/restore.c
index 19df6be2f539..85b98d502ac3 100644
--- a/cmds/restore.c
+++ b/cmds/restore.c
@@ -1481,9 +1481,9 @@ static int cmd_restore(const struct cmd_struct *cmd, int argc, char **argv)
 	}
 
 	if (!list_roots && check_argc_min(argc - optind, 2))
-		usage(cmd);
+		usage(cmd, 1);
 	else if (list_roots && check_argc_min(argc - optind, 1))
-		usage(cmd);
+		usage(cmd, 1);
 
 	if (fs_location && root_objectid) {
 		error("can't use -f and -r at the same time");
diff --git a/cmds/subvolume-list.c b/cmds/subvolume-list.c
index 3de167f95685..c94245070540 100644
--- a/cmds/subvolume-list.c
+++ b/cmds/subvolume-list.c
@@ -1645,7 +1645,7 @@ out:
 	if (comparer_set)
 		free(comparer_set);
 	if (uerr)
-		usage(cmd);
+		usage(cmd, 1);
 	return !!ret;
 }
 DEFINE_SIMPLE_COMMAND(subvolume_list, "list");
diff --git a/cmds/subvolume.c b/cmds/subvolume.c
index 50aac4034db4..e194ded54b3b 100644
--- a/cmds/subvolume.c
+++ b/cmds/subvolume.c
@@ -1299,7 +1299,7 @@ static int cmd_subvolume_show(const struct cmd_struct *cmd, int argc, char **arg
 	if (by_rootid && by_uuid) {
 		error(
 		"options --rootid and --uuid cannot be used at the same time");
-		usage(cmd);
+		usage(cmd, 1);
 	}
 
 	fullpath = realpath(argv[optind], NULL);
diff --git a/common/help.c b/common/help.c
index 8145df9e5781..0e50ba1dad53 100644
--- a/common/help.c
+++ b/common/help.c
@@ -105,7 +105,7 @@ void clean_args_no_options(const struct cmd_struct *cmd, int argc, char *argv[])
 		switch (c) {
 		default:
 			if (cmd->usagestr)
-				usage(cmd);
+				usage(cmd, 1);
 		}
 	}
 }
@@ -380,10 +380,10 @@ void usage_unknown_option(const struct cmd_struct *cmd, char **argv)
 }
 
 __attribute__((noreturn))
-void usage(const struct cmd_struct *cmd)
+void usage(const struct cmd_struct *cmd, int ret)
 {
 	usage_command_usagestr(cmd->usagestr, NULL, 0, true, true);
-	exit(1);
+	exit(ret);
 }
 
 static void usage_command_group_internal(const struct cmd_group *grp, bool full,
diff --git a/common/help.h b/common/help.h
index 02286847e13a..9375ab6e32b1 100644
--- a/common/help.h
+++ b/common/help.h
@@ -103,7 +103,7 @@ __attribute__((noreturn))
 void usage_unknown_option(const struct cmd_struct *cmd, char **argv);
 
 __attribute__((noreturn))
-void usage(const struct cmd_struct *cmd);
+void usage(const struct cmd_struct *cmd, int ret);
 void usage_command(const struct cmd_struct *cmd, bool full, bool err);
 void usage_command_group(const struct cmd_group *grp, bool all, bool err);
 void usage_command_group_short(const struct cmd_group *grp);
diff --git a/image/main.c b/image/main.c
index 65aa3b30b182..3e7a34533716 100644
--- a/image/main.c
+++ b/image/main.c
@@ -3053,8 +3053,7 @@ static const struct cmd_struct image_cmd = {
 
 static void print_usage(int ret)
 {
-	usage(&image_cmd);
-	exit(ret);
+	usage(&image_cmd, ret);
 }
 
 int BOX_MAIN(image)(int argc, char *argv[])
diff --git a/mkfs/main.c b/mkfs/main.c
index 341ba4089484..da123ee4455d 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -448,8 +448,7 @@ static const struct cmd_struct mkfs_cmd = {
 
 static void print_usage(int ret)
 {
-	usage(&mkfs_cmd);
-	exit(ret);
+	usage(&mkfs_cmd, ret);
 }
 
 static int zero_output_file(int out_fd, u64 size)
diff --git a/tune/main.c b/tune/main.c
index acfc126286d0..79b676972b50 100644
--- a/tune/main.c
+++ b/tune/main.c
@@ -95,8 +95,7 @@ static const struct cmd_struct tune_cmd = {
 
 static void print_usage(int ret)
 {
-	usage(&tune_cmd);
-	exit(ret);
+	usage(&tune_cmd, ret);
 }
 
 int BOX_MAIN(btrfstune)(int argc, char *argv[])
-- 
2.39.1

[PATCH 2/2] btrfs-progs: tests: cli: fix 017 test case failure

[Qu Wenruo]

[BUG]
Test case cli/017 fails with the following errors:

    [TEST]   cli-tests.sh
    [TEST/cli]   017-fi-show-missing
  didn't find exact missing device
  test failed for case 017-fi-show-missing

[CAUSE]
After kernel commit cb3e217bdb39 ("btrfs: use btrfs_dev_name() helper to
handle missing devices better"), all dev info ioctl call on missing
device would only return "<missing disk>" for its path.

Thus "btrfs filesystem show" would never report detailed device path for
missing disks.

[FIX]
Instead of relying on the device path, change the check to rely on devid
instead.

Now cli/017 can properly pass.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/cli-tests/017-fi-show-missing/test.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/cli-tests/017-fi-show-missing/test.sh b/tests/cli-tests/017-fi-show-missing/test.sh
index 25e4c60a70e0..67757e20a898 100755
--- a/tests/cli-tests/017-fi-show-missing/test.sh
+++ b/tests/cli-tests/017-fi-show-missing/test.sh
@@ -23,7 +23,7 @@ run_check $SUDO_HELPER mv "$dev2" /dev/loop-non-existent
 run_check $SUDO_HELPER mount -o degraded $dev1 $TEST_MNT
 
 if ! run_check_stdout $SUDO_HELPER "$TOP/btrfs" filesystem show "$TEST_MNT" | \
-	grep -q "$dev2 MISSING"; then
+	grep -q -e "devid[[:space:]]\+2.*MISSING"; then
 
 	_fail "didn't find exact missing device"
 fi
-- 
2.39.1

Re: [PATCH 0/2] btrfs-progs: fixes for the cli test group

[David Sterba]

On Mon, Feb 13, 2023 at 01:26:31PM +0800, Qu Wenruo wrote:
> For the current devel branch, there are two failures for the cli test group:
> 
> - cli/009
>   This is caused by a very recent (only in devel branch) refactor for
>   btrfstune, which removes the ability to customize the return value.
> 
>   Fix it by adding a new @ret argument for usage() helper.
> 
> - cli/017
>   This exists for a while, and it's caused by a recent kernel change.
> 
>   Fix the test case to handle it better.
> 
> Qu Wenruo (2):
>   btrfs-progs: make usage() call to properly return an exit value
>   btrfs-progs: tests: cli: fix 017 test case failure

Added to devel, thanks.