[PATCH 0/3] pull: blkreset and blkreport polishing

[PATCH 0/3] pull: blkreset and blkreport polishing

[Sami Kerola]

Hello,

I should have said this a bit earlier.  It is a pity there are now two new
blkre* tools instead of single 'zblkadm (reset|report)'.  A single command
would have less boilerplating going on, and would form logical path how to
add new features in future.

Oh well, let me propose couple to existing blkre* commands.  I think these
are reasonable changes in any case (will the tools are merged or not).

----------------------------------------------------------------
The following changes since commit 7ccc1643abdd0cb9780ca0304dfdcbc4c5b11eae:
  findmnt: use line separator for --poll output (2017-02-10 17:28:07 +0100)
are available in the git repository at:
  git://github.com/kerolasa/lelux-utiliteetit.git blkre
for you to fetch changes up to 70e7defd161f5a03a473256664e9f9c32504d410:
  bash-completion: add blkreset and blkreport completion (2017-02-11 13:58:25 +0000)
----------------------------------------------------------------

Sami Kerola (3):
  blkreport, blkreset: small code clean ups
  man: improve blkreport(8) and blkreset(8) manual pages
  bash-completion: add blkreset and blkreport completion

 bash-completion/blkreset | 34 ++++++++++++++++++++++++++++++++++
 sys-utils/blkreport.8    | 29 ++++++++++++++++++++++++++++-
 sys-utils/blkreport.c    | 43 +++++++++++++++++--------------------------
 sys-utils/blkreset.8     |  9 ++++-----
 sys-utils/blkreset.c     | 17 ++++++++---------
 5 files changed, 91 insertions(+), 41 deletions(-)
 create mode 100755 bash-completion/blkreset

-- 
2.11.1

[PATCH 1/3] blkreport, blkreset: small code clean ups

[Sami Kerola]

Set variables read-only where possible.  Fix few code style issues, mostly
with spacing.  Avoid initializing variables if they are never read before
next update.  Remove "ERR: %d -> %s" message that repeated system error
three times, twice in that message and once at end of main that is the only
of these three left in place.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/blkreport.c | 43 +++++++++++++++++--------------------------
 sys-utils/blkreset.c  | 17 ++++++++---------
 2 files changed, 25 insertions(+), 35 deletions(-)

diff --git a/sys-utils/blkreport.c b/sys-utils/blkreport.c
index f3b8ebbda..c8d0a63ac 100644
--- a/sys-utils/blkreport.c
+++ b/sys-utils/blkreport.c
@@ -47,16 +47,14 @@
 #include "closestream.h"
 #include "blkdev.h"
 
-static const char * type_text[] = {
+static const char *type_text[] = {
 	"RESERVED",
 	"CONVENTIONAL",
 	"SEQ_WRITE_REQUIRED",
 	"SEQ_WRITE_PREFERRED",
 };
 
-#define ARRAY_COUNT(x) (sizeof((x))/sizeof((*x)))
-
-const char * condition_str[] = {
+static const char *condition_str[] = {
 	"cv", /* conventional zone */
 	"e0", /* empty */
 	"Oi", /* open implicit */
@@ -66,22 +64,17 @@ const char * condition_str[] = {
 	"ro", /* read only */
 	"fu", /* full */
 	"OL"  /* offline */
-	};
-
-static const char * zone_condition_str(uint8_t cond)
-{
-	return condition_str[cond & 0x0f];
-}
+};
 
-static void print_zones(struct blk_zone *info, uint32_t count)
+static void print_zones(const struct blk_zone *info, const uint32_t count)
 {
 	uint32_t iter;
 
 	printf(_("Zones returned: %u\n"), count);
 
-	for (iter = 0; iter < count; iter++ ) {
-		struct blk_zone * entry = &info[iter];
-		unsigned int type  = entry->type;
+	for (iter = 0; iter < count; iter++) {
+		const struct blk_zone *entry = &info[iter];
+		unsigned int type = entry->type;
 		uint64_t start = entry->start;
 		uint64_t wp = entry->wp;
 		uint8_t cond = entry->cond;
@@ -94,14 +87,14 @@ static void print_zones(struct blk_zone *info, uint32_t count)
 			 " reset:%u non-seq:%u, zcond:%2u(%s) [type: %u(%s)]\n"),
 			start, len, wp - start,
 			entry->reset, entry->non_seq,
-			cond, zone_condition_str(cond),
+			cond, condition_str[cond & ARRAY_SIZE(condition_str)],
 			type, type_text[type]);
 	}
 }
 
-static int do_report(int fd, uint64_t lba, uint32_t len, int verbose)
+static int do_report(const int fd, const uint64_t lba, uint32_t len, const int verbose)
 {
-	int rc = -4;
+	int rc;
 	struct blk_zone_report *zi;
 
 	zi = xmalloc(sizeof(struct blk_zone_report) + (len * sizeof(struct blk_zone)));
@@ -112,8 +105,6 @@ static int do_report(int fd, uint64_t lba, uint32_t len, int verbose)
 		if (verbose)
 			printf(_("Found %d zones\n"), zi->nr_zones);
 		print_zones(zi->zones, zi->nr_zones);
-	} else {
-		warn(_("ERR: %d -> %s"), errno, strerror(errno));
 	}
 	free(zi);
 
@@ -155,12 +146,12 @@ int main(int argc, char **argv)
 	uint64_t offset = 0ul;
 	uint32_t length = DEF_REPORT_LEN;
 	static const struct option longopts[] = {
-	    { "help",      0, 0, 'h' },
-	    { "version",   0, 0, 'V' },
-	    { "zone",      1, 0, 'z' }, /* starting LBA */
-	    { "count",     1, 0, 'c' }, /* max #of zones (entries) for result */
-	    { "verbose",   0, 0, 'v' },
-	    { NULL,        0, 0, 0 }
+	    { "help",    no_argument,       NULL, 'h' },
+	    { "version", no_argument,       NULL, 'V' },
+	    { "zone",    required_argument, NULL, 'z' }, /* starting LBA */
+	    { "count",   required_argument, NULL, 'c' }, /* max #of zones (entries) for result */
+	    { "verbose", no_argument,       NULL, 'v' },
+	    { NULL, 0, NULL, 0 }
 	};
 
 	setlocale(LC_ALL, "");
@@ -169,7 +160,7 @@ int main(int argc, char **argv)
 	atexit(close_stdout);
 
 	while ((c = getopt_long(argc, argv, "hc:z:vV", longopts, NULL)) != -1) {
-		switch(c) {
+		switch (c) {
 		case 'h':
 			usage(stdout);
 			break;
diff --git a/sys-utils/blkreset.c b/sys-utils/blkreset.c
index 76df5f6a5..9a19c33c7 100644
--- a/sys-utils/blkreset.c
+++ b/sys-utils/blkreset.c
@@ -94,7 +94,6 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
 	exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
 }
 
-
 int main(int argc, char **argv)
 {
 	char *path;
@@ -103,17 +102,17 @@ int main(int argc, char **argv)
 	struct stat sb;
 	struct blk_zone_range za;
 	uint64_t zsector = 0;
-	uint64_t zlen = 0;
+	uint64_t zlen;
 	uint64_t zcount = 1;
 	unsigned long zsize;
-	int rc = 0;
+	int rc;
 
 	static const struct option longopts[] = {
-	    { "help",      0, 0, 'h' },
-	    { "version",   0, 0, 'V' },
-	    { "zone",      1, 0, 'z' },
-	    { "count",     1, 0, 'c' },
-	    { NULL,        0, 0, 0 }
+	    { "help",    no_argument,       NULL, 'h' },
+	    { "version", no_argument,       NULL, 'V' },
+	    { "zone",    required_argument, NULL, 'z' },
+	    { "count",   required_argument, NULL, 'c' },
+	    { NULL, 0, NULL, 0 }
 	};
 
 	setlocale(LC_ALL, "");
@@ -122,7 +121,7 @@ int main(int argc, char **argv)
 	atexit(close_stdout);
 
 	while ((c = getopt_long(argc, argv, "hVz:c:", longopts, NULL)) != -1) {
-		switch(c) {
+		switch (c) {
 		case 'h':
 			usage(stdout);
 			break;
-- 
2.11.1

[PATCH 2/3] man: improve blkreport(8) and blkreset(8) manual pages

[Sami Kerola]

Tell in manual page what ZAC and ZBC acronyms mean.  Include explanation to
blkreport output, so that users do not need to search source code to
understand what the tool informed.  And fix couple dot double space issues.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 sys-utils/blkreport.8 | 29 ++++++++++++++++++++++++++++-
 sys-utils/blkreset.8  |  9 ++++-----
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/sys-utils/blkreport.8 b/sys-utils/blkreport.8
index 81417d4ad..1d00bfd24 100644
--- a/sys-utils/blkreport.8
+++ b/sys-utils/blkreport.8
@@ -12,7 +12,8 @@ blkreport \- report zones on a device
 .SH DESCRIPTION
 .B blkreport
 is used to report device zone information.  This is useful for
-zoned devices that support the ZAC or ZBC command set.
+devices that support the Zoned Block Commands (ZBC) or Zoned-device ATA
+Commands (ZAC).
 .PP
 By default,
 .B blkreport
@@ -52,6 +53,32 @@ Display version information and exit.
 .TP
 .BR \-h , " \-\-help"
 Display help text and exit.
+.SH REPORT OUTPUT
+.TS
+tab(:);
+left l l.
+start:Zone start sector
+len:Zone length in number of sectors
+wptr:Zone write pointer position
+reset:Reset write pointer recommended
+non-seq:Non-sequential write resources active
+cond:Zone condition
+type:Zone type
+.TE
+.SS Zone conditions
+.TS
+tab(:);
+left l l.
+Cl:closed
+cv:conventional zone
+e0:empty
+fu:full
+Oe:open explicit
+Oi:open implicit
+OL:offline
+ro:read only
+x?:reserved conditions (should not be reported)
+.TE
 .SH AUTHOR
 .MT shaun@tancheff.com
 Shaun Tancheff
diff --git a/sys-utils/blkreset.8 b/sys-utils/blkreset.8
index 598d89fa8..250699196 100644
--- a/sys-utils/blkreset.8
+++ b/sys-utils/blkreset.8
@@ -10,15 +10,14 @@ blkreset \- Reset a range of zones
 .IR count ]
 .SH DESCRIPTION
 .B blkreset
-is used to reset one or more zones.  This is useful for
-zoned devices that support the ZAC or ZBC command set.
-Unlike
+is used to reset one or more zones.  This is useful for devices that support
+Zoned Block Commands (ZBC) or Zoned-device ATA Commands (ZAC).  Unlike
 .BR sg_reset_wp (8) ,
 this command operates from the block layer and can reset a range of zones.
 .PP
 By default,
 .B blkreset
-will operate on the zone at device logical sector 0. Options may be used to
+will operate on the zone at device logical sector 0.  Options may be used to
 modify this behavior as well as specify the operation to be performed on
 the zone, as explained below.
 .PP
@@ -43,7 +42,7 @@ The provided offset in sector units (512 bytes) should match the start of a zone
 The default value is zero.
 .TP
 .BR \-c , " \-\-count "\fIlength\fP
-The number of zones to be reset starting from offset. Default is 1 zone.
+The number of zones to be reset starting from offset.  Default is 1 zone.
 .TP
 .BR \-V , " \-\-version"
 Display version information and exit.
-- 
2.11.1

[PATCH 3/3] bash-completion: add blkreset and blkreport completion

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 bash-completion/blkreset | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100755 bash-completion/blkreset

diff --git a/bash-completion/blkreset b/bash-completion/blkreset
new file mode 100755
index 000000000..69c5f63c1
--- /dev/null
+++ b/bash-completion/blkreset
@@ -0,0 +1,34 @@
+_blkreset_module()
+{
+	local cur prev OPTS
+	COMPREPLY=()
+	cur="${COMP_WORDS[COMP_CWORD]}"
+	prev="${COMP_WORDS[COMP_CWORD-1]}"
+	case $prev in
+		'-z'|'--zone')
+			COMPREPLY=( $(compgen -W "offset" -- $cur) )
+			return 0
+			;;
+		'-c'|'--count')
+			COMPREPLY=( $(compgen -W "length" -- $cur) )
+			return 0
+			;;
+		'-h'|'--help'|'-V'|'--version')
+			return 0
+			;;
+	esac
+	case $cur in
+		-*)
+			OPTS="--zone --count --version --help"
+			if [ "$1" = 'blkreport' ]; then
+				OPTS+=' --verbose'
+			fi
+			COMPREPLY=( $(compgen -W "${OPTS[*]}" -- $cur) )
+			return 0
+			;;
+	esac
+	COMPREPLY=( $(compgen -W "$(lsblk -pnro name)" -- $cur) )
+	return 0
+}
+complete -F _blkreset_module blkreset
+complete -F _blkreset_module blkreport
-- 
2.11.1

Re: [PATCH 0/3] pull: blkreset and blkreport polishing

[Benno Schulenberg]

On Sat, Feb 11, 2017, at 15:10, Sami Kerola wrote:
> It is a pity there are now two new
> blkre* tools instead of single 'zblkadm (reset|report)'.  A single command
> would have less boilerplating going on, and would form logical path how to
> add new features in future.

The commands haven't been in a release yet, so there is still time
for you to propose a better solution and rework things into a single
command.

Benno

-- 
http://www.fastmail.com - Choose from over 50 domains or use your own

Re: [PATCH 0/3] pull: blkreset and blkreport polishing

[Karel Zak]

On Sat, Feb 11, 2017 at 02:10:49PM +0000, Sami Kerola wrote:
> I should have said this a bit earlier. 

Yes :-)

> It is a pity there are now two new
> blkre* tools instead of single 'zblkadm (reset|report)'.  A single command
> would have less boilerplating going on, and would form logical path how to
> add new features in future.

I like the idea, but no the name. IMHO something like 

    blkzone (reset|report)
    
would be better. I'll try to merge it to the one command during this week.

    Karel

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

Re: [PATCH 0/3] pull: blkreset and blkreport polishing

[Karel Zak]

On Sat, Feb 11, 2017 at 02:10:49PM +0000, Sami Kerola wrote:
>  bash-completion/blkreset | 34 ++++++++++++++++++++++++++++++++++
>  sys-utils/blkreport.8    | 29 ++++++++++++++++++++++++++++-
>  sys-utils/blkreport.c    | 43 +++++++++++++++++--------------------------
>  sys-utils/blkreset.8     |  9 ++++-----
>  sys-utils/blkreset.c     | 17 ++++++++---------
>  5 files changed, 91 insertions(+), 41 deletions(-)
>  create mode 100755 bash-completion/blkreset

Thanks (and thanks for bash-completion).

    Karel

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