[PATCH 1/2] blkpr: rename operation to command

[PATCH 1/2] blkpr: rename operation to command

[zhenwei pi]

Suggested by Karel, rename operation to command.
Thus make this more clear.

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
 sys-utils/blkpr.8.adoc |  8 ++++----
 sys-utils/blkpr.c      | 44 +++++++++++++++++++++---------------------
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/sys-utils/blkpr.8.adoc b/sys-utils/blkpr.8.adoc
index 482bdec18..2955643fc 100644
--- a/sys-utils/blkpr.8.adoc
+++ b/sys-utils/blkpr.8.adoc
@@ -22,15 +22,15 @@ The _device_ argument is the pathname of the block device.
 
 == OPTIONS
 
-*-o*, *--operation* _operation_::
-The operation of persistent reservations, supported operations are *register*, *reserve*, *release*, *preempt*,
+*-c*, *--command* _command_::
+The command of persistent reservations, supported commands are *register*, *reserve*, *release*, *preempt*,
 *preempt-abort*, and *clear*.
 
 *-k*, *--key* _key_::
-The key the operation should operate on.
+The key the command should operate on.
 
 *-K*, *--oldkey* _oldkey_::
-The old key the operation should operate on.
+The old key the command should operate on.
 
 *-f*, *--flag* _flag_::
 Supported flag is *ignore-key*.
diff --git a/sys-utils/blkpr.c b/sys-utils/blkpr.c
index d46eb19c1..a5132c003 100644
--- a/sys-utils/blkpr.c
+++ b/sys-utils/blkpr.c
@@ -1,7 +1,7 @@
 /*
  * blkpr.c -- persistent reservations on a block device.
  *
- * Copyright (C) 2021 zhenwei pi <pizhenwei@bytedance.com>
+ * Copyright (C) 2021-2022 zhenwei pi <pizhenwei@bytedance.com>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -16,8 +16,8 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
- * This program uses IOC_PR_XXX ioctl to do persistent reservations
- * operation on a block device if the device supports it.
+ * This program uses IOC_PR_XXX ioctl to run persistent reservations
+ * command on a block device if the device supports it.
  */
 #include <string.h>
 #include <unistd.h>
@@ -49,7 +49,7 @@ static struct type_string pr_type[] = {
 	{PR_EXCLUSIVE_ACCESS_ALL_REGS, "exclusive-access-all-regs"}
 };
 
-static struct type_string pr_operation[] = {
+static struct type_string pr_command[] = {
 	{IOC_PR_REGISTER,      "register"},
 	{IOC_PR_RESERVE,       "reserve"},
 	{IOC_PR_RELEASE,       "release"},
@@ -96,11 +96,11 @@ static int parse_type_by_str(struct type_string *ts, int nmem, char *pattern)
 	{ return parse_type_by_str(XX, ARRAY_SIZE(XX), pattern); }
 
 PRINT_SUPPORTED(pr_type)
-PRINT_SUPPORTED(pr_operation)
+PRINT_SUPPORTED(pr_command)
 PRINT_SUPPORTED(pr_flag)
 
 PARSE(pr_type)
-PARSE(pr_operation)
+PARSE(pr_command)
 PARSE(pr_flag)
 
 static int do_pr(char *path, uint64_t key, uint64_t oldkey, int op, int type, int flag)
@@ -144,7 +144,7 @@ static int do_pr(char *path, uint64_t key, uint64_t oldkey, int op, int type, in
 		break;
 	default:
 		errno = EINVAL;
-		err(EXIT_FAILURE, _("unknown operation"));
+		err(EXIT_FAILURE, _("unknown command"));
 	}
 
 	close(fd);
@@ -168,26 +168,26 @@ static void __attribute__((__noreturn__)) usage(void)
 	fputs(_("Persistent reservations on a device.\n"), out);
 
 	fputs(USAGE_OPTIONS, out);
-	fputs(_(" -o, --operation <oper>   operation of persistent reservations\n"), out);
+	fputs(_(" -c, --command <cmd>      command of persistent reservations\n"), out);
 	fputs(_(" -k, --key <num>          key to operate\n"), out);
 	fputs(_(" -K, --oldkey <num>       old key to operate\n"), out);
-	fputs(_(" -f, --flag <flag>        operation flag\n"), out);
-	fputs(_(" -t, --type <type>        operation type\n"), out);
+	fputs(_(" -f, --flag <flag>        command flag\n"), out);
+	fputs(_(" -t, --type <type>        command type\n"), out);
 
 	fputs(USAGE_SEPARATOR, out);
 	printf(USAGE_HELP_OPTIONS(26));
 
 	fputs(USAGE_ARGUMENTS, out);
 
-	fputs(_(" <oper> is an operation, available operations:\n"), out);
+	fputs(_(" <cmd> is an command, available command:\n"), out);
 	fputs("        ", out);
-	print_pr_operation(out);
+	print_pr_command(out);
 
-	fputs(_(" <flag> is an operation flag, available flags:\n"), out);
+	fputs(_(" <flag> is a command flag, available flags:\n"), out);
 	fputs("        ", out);
 	print_pr_flag(out);
 
-	fputs(_(" <type> is an operation type, available types:\n"), out);
+	fputs(_(" <type> is a command type, available types:\n"), out);
 	fputs("        ", out);
 	print_pr_type(out);
 
@@ -200,12 +200,12 @@ int main(int argc, char **argv)
 	char c;
 	char *path;
 	uint64_t key = 0, oldkey = 0;
-	int operation = -1, type = -1, flag = 0;
+	int command = -1, type = -1, flag = 0;
 
 	static const struct option longopts[] = {
 	    { "help",            no_argument,       NULL, 'h' },
 	    { "version",         no_argument,       NULL, 'V' },
-	    { "operation",       required_argument, NULL, 'o' },
+	    { "command",         required_argument, NULL, 'c' },
 	    { "key",             required_argument, NULL, 'k' },
 	    { "oldkey",          required_argument, NULL, 'K' },
 	    { "flag",            required_argument, NULL, 'f' },
@@ -219,7 +219,7 @@ int main(int argc, char **argv)
 	close_stdout_atexit();
 
 	errno = EINVAL;
-	while ((c = getopt_long(argc, argv, "hVo:k:K:f:t:", longopts, NULL)) != -1) {
+	while ((c = getopt_long(argc, argv, "hVc:k:K:f:t:", longopts, NULL)) != -1) {
 		switch(c) {
 		case 'k':
 			key = strtosize_or_err(optarg,
@@ -229,10 +229,10 @@ int main(int argc, char **argv)
 			oldkey = strtosize_or_err(optarg,
 					_("failed to parse old key"));
 			break;
-		case 'o':
-			operation = parse_pr_operation(optarg);
-			if (operation < 0)
-				err(EXIT_FAILURE, _("unknown operation"));
+		case 'c':
+			command = parse_pr_command(optarg);
+			if (command < 0)
+				err(EXIT_FAILURE, _("unknown command"));
 			break;
 		case 't':
 			type = parse_pr_type(optarg);
@@ -263,7 +263,7 @@ int main(int argc, char **argv)
 		errtryhelp(EXIT_FAILURE);
 	}
 
-	do_pr(path, key, oldkey, operation, type, flag);
+	do_pr(path, key, oldkey, command, type, flag);
 
 	return EXIT_SUCCESS;
 }
-- 
2.25.1

[PATCH 2/2] blkpr: add descriptions for arguments

[zhenwei pi]

Karel pointed out that the arguments of this command is not obvious
enough for end-users. So add the detailed descriptions.

The description comes from linux/Documentation/block/pr.rst, and
modify a bit.

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
 sys-utils/blkpr.c | 81 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 62 insertions(+), 19 deletions(-)

diff --git a/sys-utils/blkpr.c b/sys-utils/blkpr.c
index a5132c003..906fa7883 100644
--- a/sys-utils/blkpr.c
+++ b/sys-utils/blkpr.c
@@ -37,29 +37,77 @@
 struct type_string {
 	int type;
 	char *str;
+	char *desc;
 };
 
 /* This array should keep align with enum pr_type of linux/types.h */
 static struct type_string pr_type[] = {
-	{PR_WRITE_EXCLUSIVE,           "write-exclusive"},
-	{PR_EXCLUSIVE_ACCESS,          "exclusive-access"},
-	{PR_WRITE_EXCLUSIVE_REG_ONLY,  "write-exclusive-reg-only"},
-	{PR_EXCLUSIVE_ACCESS_REG_ONLY, "exclusive-access-reg-only"},
-	{PR_WRITE_EXCLUSIVE_ALL_REGS,  "write-exclusive-all-regs"},
-	{PR_EXCLUSIVE_ACCESS_ALL_REGS, "exclusive-access-all-regs"}
+	{PR_WRITE_EXCLUSIVE,           "write-exclusive",
+	"Only the initiator that owns the reservation can write to the\n"
+	"\t\tdevice. Any initiator can read from the device.\n"},
+
+	{PR_EXCLUSIVE_ACCESS,          "exclusive-access",
+	"Only the initiator that owns the reservation can access the device.\n"},
+
+	{PR_WRITE_EXCLUSIVE_REG_ONLY,  "write-exclusive-reg-only",
+	"Only initiators with a registered key can write to the device,\n"
+	"\t\tAny initiator can read from the device.\n"},
+
+	{PR_EXCLUSIVE_ACCESS_REG_ONLY, "exclusive-access-reg-only",
+	"Only initiators with a registered key can access the device.\n"},
+
+	{PR_WRITE_EXCLUSIVE_ALL_REGS,  "write-exclusive-all-regs",
+	"Only initiators with a registered key can write to the device.\n"
+	"\t\tAny initiator can read from the device.\n"
+	"\t\tAll initiators with a registered key are considered reservation holders.\n"
+	"\t\tPlease reference the SPC spec on the meaning of a reservation\n"
+	"\t\tholder if you want to use this type.\n"},
+
+	{PR_EXCLUSIVE_ACCESS_ALL_REGS, "exclusive-access-all-regs",
+	"Only initiators with a registered key can access the device.\n"
+	"\t\tAll initiators with a registered key are considered reservation holders.\n"
+	"\t\tPlease reference the SPC spec on the meaning of a reservation\n"
+	"\t\tholder if you want to use this type.\n"}
 };
 
 static struct type_string pr_command[] = {
-	{IOC_PR_REGISTER,      "register"},
-	{IOC_PR_RESERVE,       "reserve"},
-	{IOC_PR_RELEASE,       "release"},
-	{IOC_PR_PREEMPT,       "preempt"},
-	{IOC_PR_PREEMPT_ABORT, "preempt-abort"},
-	{IOC_PR_CLEAR,         "clear"},
+	{IOC_PR_REGISTER,      "register",
+	"This command registers a new reservation if the key argument\n"
+	"\t\tis non-null. If no existing reservation exists oldkey must be zero,\n"
+	"\t\tif an existing reservation should be replaced oldkey must contain\n"
+	"\t\tthe old reservation key.\n"
+	"\t\tIf the key argument is 0 it unregisters the existing reservation passed\n"
+	"\t\tin oldkey.\n"},
+
+	{IOC_PR_RESERVE,       "reserve",
+	"This command reserves the device and thus restricts access for other\n"
+	"\t\tdevices based on the type argument.  The key argument must be the existing\n"
+	"\t\treservation key for the device as acquired by the register, preempt,\n"
+	"\t\tpreempt-abort commands.\n"},
+
+	{IOC_PR_RELEASE,       "release",
+	"This command releases the reservation specified by key and flags\n"
+	"\t\tand thus removes any access restriction implied by it.\n"},
+
+	{IOC_PR_PREEMPT,       "preempt",
+	"This command releases the existing reservation referred to by\n"
+	"\t\told_key and replaces it with a new reservation of type for the\n"
+	"\t\treservation key key.\n"},
+
+	{IOC_PR_PREEMPT_ABORT, "preempt-abort",
+	"This command works like preempt except that it also aborts\n"
+	"\t\tany outstanding command sent over a connection identified by oldkey.\n"},
+
+	{IOC_PR_CLEAR,         "clear",
+	"This command unregisters both key and any other reservation key\n"
+	"\t\tregistered with the device and drops any existing reservation.\n"},
 };
 
 static struct type_string pr_flag[] = {
-	{PR_FL_IGNORE_KEY, "ignore-key"}
+	{PR_FL_IGNORE_KEY, "ignore-key",
+	"Ignore the existing reservation key.  This is commonly supported for\n"
+	"\t\tregister command, and some implementation may support the flag for\n"
+	"\t\treserve command.\n"}
 };
 
 static void print_type(FILE *out, struct type_string *ts, size_t nmem)
@@ -67,9 +115,7 @@ static void print_type(FILE *out, struct type_string *ts, size_t nmem)
 	size_t i;
 
 	for (i = 0; i < nmem; i++) {
-		fprintf(out, "%s", ts[i].str);
-		fputs(i + 2 < nmem ? ", " :
-		      i + 1 < nmem ? _(", and ") : "\n", out);
+		fprintf(out, "\t%s: %s\n", ts[i].str, ts[i].desc);
 	}
 }
 
@@ -180,15 +226,12 @@ static void __attribute__((__noreturn__)) usage(void)
 	fputs(USAGE_ARGUMENTS, out);
 
 	fputs(_(" <cmd> is an command, available command:\n"), out);
-	fputs("        ", out);
 	print_pr_command(out);
 
 	fputs(_(" <flag> is a command flag, available flags:\n"), out);
-	fputs("        ", out);
 	print_pr_flag(out);
 
 	fputs(_(" <type> is a command type, available types:\n"), out);
-	fputs("        ", out);
 	print_pr_type(out);
 
 	printf(USAGE_MAN_TAIL("blkpr(8)"));
-- 
2.25.1

Re: [PATCH 1/2] blkpr: rename operation to command

[Karel Zak]

On Thu, Apr 07, 2022 at 11:49:31AM +0800, zhenwei pi wrote:
> Suggested by Karel, rename operation to command.
> Thus make this more clear.

Both patches applied. Thanks!

(I did small changes to formatting to make a little bit shorter and
usable for translators.)

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: Re: [PATCH 1/2] blkpr: rename operation to command

[zhenwei pi]

On 4/11/22 18:53, Karel Zak wrote:
> On Thu, Apr 07, 2022 at 11:49:31AM +0800, zhenwei pi wrote:
>> Suggested by Karel, rename operation to command.
>> Thus make this more clear.
> 
> Both patches applied. Thanks!
> 
> (I did small changes to formatting to make a little bit shorter and
> usable for translators.)
> 
>      Karel
> 
I test this output, it looks better than the original version.
Thanks a lot!

-- 
zhenwei pi