[PATCH] made consistent displayed help and manpage with options acepted by bcache

[PATCH] made consistent displayed help and manpage with options acepted by bcache

[Marcin Mirosław]

Hi!

This patch fix misleading in name of options in manpage and displayed
help. But still remains in manpage unimplemented options like "stats" or
"status". I don't kno how to handle it, remove?

diff --git a/bcache-format.c b/bcache-format.c
index 1b20b55..d71ae68 100644
--- a/bcache-format.c
+++ b/bcache-format.c
@@ -75,16 +75,16 @@ static void usage(void)
             "Usage: bcache format [OPTION]... <devices>\n"
             "\n"
             "Options:\n"
-            "  -b, --block=size\n"
-            "      --btree_node=size       Btree node size, default 256k\n"
+            "  -b, --block=size      default 4k\n"
+            "      --btree_node_size=size       Btree node size,
default 256k\n"
             "      --metadata_checksum_type=(none|crc32c|crc64)\n"
             "      --data_checksum_type=(none|crc32c|crc64)\n"
             "      --compression_type=(none|lz4|gzip)\n"
             "      --encrypted\n"
             "      --error_action=(continue|readonly|panic)\n"
             "                              Action to take on filesystem
error\n"
-            "  -l, --label=label\n"
-            "      --uuid=uuid\n"
+            "  -L, --label=label\n"
+            "  -U, --uuid=uuid\n"
             "  -f, --force\n"
             "\n"
             "Device specific options:\n"
diff --git a/bcache.8 b/bcache.8
index 7671cf3..5092f00 100644
--- a/bcache.8
+++ b/bcache.8
@@ -64,9 +64,9 @@ List the bcache commands
 .SH Global options:
 .TP
 .BR \-w,\ --block=
-block size, in bytes (e.g. 4k)
+block size, in bytes (e.g. 4096) - default 4k
 .TP
-.BR \-w,\ --btree_node_size=
+.BR \--btree_node_size=
 btree node size in bytes - default 256k
 .TP
 .BR \--metadata_checksum_type=TYPE
@@ -83,10 +83,10 @@ Enable encryption; passphrase will be prompted for
 .BR \--error_action=TYPE
 where TYPE is one of continue, readonly (default) or panic
 .TP
-.BR \-l\ --label=LABEL
+.BR \-L\ --label=LABEL
 Create the filesystem with the specified label
 .TP
-.BR \--uuid=UUID
+.BR \-U\ --uuid=UUID
 Create the filesystem with the specified UUID
 .TP
 .BR \--force
@@ -98,7 +98,7 @@ filesystem
 .BR \--fs_size=SIZE
 Create the filesystem using SIZE bytes on the subsequent device
 .TP
-.BR \-b\ --bucket-size=SIZE
+.BR \--bucket-size=SIZE
 Specifies the bucket size - must be greater than the btree node size
 .TP
 .BR \-t,\ --tier=INDEX

Re: [PATCH v2] made consistent displayed help and manpage with options acepted by bcache

[Marcin Mirosław]

W dniu 02.09.2016 o 14:24, Marcin Mirosław pisze:
Hi!

This patch fix misleading in name of options in manpage and displayed
help. But still remains in manpage unimplemented options like "stats" or
"status". I don't kno how to handle it, remove?

I'm resending patch due to bad formating.

diff --git a/bcache-format.c b/bcache-format.c
index 1b20b55..d71ae68 100644
--- a/bcache-format.c
+++ b/bcache-format.c
@@ -75,16 +75,16 @@ static void usage(void)
             "Usage: bcache format [OPTION]... <devices>\n"
             "\n"
             "Options:\n"
-            "  -b, --block=size\n"
-            "      --btree_node=size       Btree node size, default 256k\n"
+            "  -b, --block=size      default 4k\n"
+            "      --btree_node_size=size       Btree node size,
default 256k\n"
             "      --metadata_checksum_type=(none|crc32c|crc64)\n"
             "      --data_checksum_type=(none|crc32c|crc64)\n"
             "      --compression_type=(none|lz4|gzip)\n"
             "      --encrypted\n"
             "      --error_action=(continue|readonly|panic)\n"
             "                              Action to take on filesystem
error\n"
-            "  -l, --label=label\n"
-            "      --uuid=uuid\n"
+            "  -L, --label=label\n"
+            "  -U, --uuid=uuid\n"
             "  -f, --force\n"
             "\n"
             "Device specific options:\n"
diff --git a/bcache.8 b/bcache.8
index 7671cf3..5092f00 100644
--- a/bcache.8
+++ b/bcache.8
@@ -64,9 +64,9 @@ List the bcache commands
 .SH Global options:
 .TP
 .BR \-w,\ --block=
-block size, in bytes (e.g. 4k)
+block size, in bytes (e.g. 4096) - default 4k
 .TP
-.BR \-w,\ --btree_node_size=
+.BR \--btree_node_size=
 btree node size in bytes - default 256k
 .TP
 .BR \--metadata_checksum_type=TYPE
@@ -83,10 +83,10 @@ Enable encryption; passphrase will be prompted for
 .BR \--error_action=TYPE
 where TYPE is one of continue, readonly (default) or panic
 .TP
-.BR \-l\ --label=LABEL
+.BR \-L\ --label=LABEL
 Create the filesystem with the specified label
 .TP
-.BR \--uuid=UUID
+.BR \-U\ --uuid=UUID
 Create the filesystem with the specified UUID
 .TP
 .BR \--force
@@ -98,7 +98,7 @@ filesystem
 .BR \--fs_size=SIZE
 Create the filesystem using SIZE bytes on the subsequent device
 .TP
-.BR \-b\ --bucket-size=SIZE
+.BR \--bucket-size=SIZE
 Specifies the bucket size - must be greater than the btree node size
 .TP
 .BR \-t,\ --tier=INDEX

Re: [PATCH v2] made consistent displayed help and manpage with options acepted by bcache

[Kent Overstreet]

On Fri, Sep 02, 2016 at 02:56:16PM +0200, Marcin Mirosław wrote:
> W dniu 02.09.2016 o 14:24, Marcin Mirosław pisze:
> Hi!
> 
> This patch fix misleading in name of options in manpage and displayed
> help. But still remains in manpage unimplemented options like "stats" or
> "status". I don't kno how to handle it, remove?
> 
> I'm resending patch due to bad formating.

Can you send with git format-patch or git send-email?

Also go aheand and remove the unimplemented stuff from the manpage.

[PATCH 1/4] No need to spawn subshell for every source file.

[Marcin Mirosław]

---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index e9ca92d..a3cb1fa 100644
--- a/Makefile
+++ b/Makefile
@@ -5,8 +5,8 @@ CFLAGS:=-std=gnu99 -O2 -Wall -D_FILE_OFFSET_BITS=64 -I. $(CFLAGS)
 LDFLAGS+=
 
 PKGCONFIG_LIBS="blkid uuid libnih"
-CFLAGS+=`pkg-config --cflags	${PKGCONFIG_LIBS}`
-LDLIBS+=`pkg-config --libs	${PKGCONFIG_LIBS}` -lscrypt -lsodium -lkeyutils
+CFLAGS+=$(shell pkg-config --cflags	${PKGCONFIG_LIBS})
+LDLIBS+=$(shell pkg-config --libs	${PKGCONFIG_LIBS}) -lscrypt -lsodium -lkeyutils
 
 ifeq ($(PREFIX),/usr)
 	ROOT_SBINDIR=/sbin
-- 
2.7.3

[PATCH 2/4] Make --help and man more consistent

[Marcin Mirosław]

---
 bcache-format.c | 20 ++++++++++----------
 bcache.8        | 13 +++++++------
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/bcache-format.c b/bcache-format.c
index 1b20b55..c092344 100644
--- a/bcache-format.c
+++ b/bcache-format.c
@@ -75,23 +75,23 @@ static void usage(void)
 	     "Usage: bcache format [OPTION]... <devices>\n"
 	     "\n"
 	     "Options:\n"
-	     "  -b, --block=size\n"
-	     "      --btree_node=size       Btree node size, default 256k\n"
-	     "      --metadata_checksum_type=(none|crc32c|crc64)\n"
-	     "      --data_checksum_type=(none|crc32c|crc64)\n"
-	     "      --compression_type=(none|lz4|gzip)\n"
+	     "  -b, --block=size in bytes               default is highest block size from all formated devices\n"
+	     "      --btree_node_size=size in bytes     Btree node size, default and max supported 256k\n"
+	     "      --metadata_checksum_type=           (none|crc32c [default]|crc64)\n"
+	     "      --data_checksum_type=               (none|crc32c [default]|crc64)\n"
+	     "      --compression_type=                 (none [default]|lz4|gzip)\n"
 	     "      --encrypted\n"
-	     "      --error_action=(continue|readonly|panic)\n"
-	     "                              Action to take on filesystem error\n"
-	     "  -l, --label=label\n"
-	     "      --uuid=uuid\n"
+	     "      --error_action=                     (continue|readonly [default]|panic)\n"
+	     "                                    Action to take on filesystem error\n"
+	     "  -L, --label=label\n"
+	     "  -U, --uuid=uuid\n"
 	     "  -f, --force\n"
 	     "\n"
 	     "Device specific options:\n"
 	     "      --fs_size=size          Size of filesystem on device\n"
 	     "      --bucket=size           bucket size\n"
 	     "      --discard               Enable discards\n"
-	     "  -t, --tier=#                tier of subsequent devices\n"
+	     "  -t, --tier=#                tier level of subsequent devices\n"
 	     "\n"
 	     "  -h, --help                  display this help and exit\n"
 	     "\n"
diff --git a/bcache.8 b/bcache.8
index 7671cf3..14aae3e 100644
--- a/bcache.8
+++ b/bcache.8
@@ -64,12 +64,13 @@ List the bcache commands
 .SH Global options:
 .TP
 .BR \-w,\ --block=
-block size, in bytes (e.g. 4k)
+block size, in bytes (e.g. 4096) - default is highest block size from all formated devices
 .TP
-.BR \-w,\ --btree_node_size=
-btree node size in bytes - default 256k
+.BR \--btree_node_size=
+btree node size in bytes - default and maximum is 256k
 .TP
 .BR \--metadata_checksum_type=TYPE
+where TYPE is one of none, crc32c (default), or crc64
 .TP
 .BR \--data_checksum_type=TYPE
 where TYPE is one of none, crc32c (default), or crc64
@@ -83,10 +84,10 @@ Enable encryption; passphrase will be prompted for
 .BR \--error_action=TYPE
 where TYPE is one of continue, readonly (default) or panic
 .TP
-.BR \-l\ --label=LABEL
+.BR \-L\ --label=LABEL
 Create the filesystem with the specified label
 .TP
-.BR \--uuid=UUID
+.BR \-U\ --uuid=UUID
 Create the filesystem with the specified UUID
 .TP
 .BR \--force
@@ -98,7 +99,7 @@ filesystem
 .BR \--fs_size=SIZE
 Create the filesystem using SIZE bytes on the subsequent device
 .TP
-.BR \-b\ --bucket-size=SIZE
+.BR \--bucket-size=SIZE
 Specifies the bucket size - must be greater than the btree node size
 .TP
 .BR \-t,\ --tier=INDEX
-- 
2.7.3

[PATCH 3/4] Remove unimplemented options from manpage

[Marcin Mirosław]

---
 bcache.8 | 43 -------------------------------------------
 1 file changed, 43 deletions(-)

diff --git a/bcache.8 b/bcache.8
index 14aae3e..ddb41cd 100644
--- a/bcache.8
+++ b/bcache.8
@@ -109,46 +109,3 @@ two tiers are supported.
 .TP
 .BR \--discard
 Enable discards on subsequent devices
-
-.SH Options for register
-
-.SH Options for list-cachesets
-.TP
-.BR \-d,\ --dir=
-Provide a directory other than /sys/fs/bcache
-
-.SH Options for query-devs
-.TP
-.BR \-f,\ --force-csum
-Enables bcache to going even if the superblock crc is invalid
-
-.SH Options for status
-
-.SH Options for stats
-.TP
-.BR \-a,\ --all
-List all stats and their values for the given device.
-.TP
-.BR \-l,\ --list
-List the names of the different stats for the given device.
-.TP
-.BR \-u,\ --set=
-UUID for the cacheset
-.TP
-.BR \-d,\ --dev=
-UUID for a device within the cacheset. Must pass in the cacheset UUID in order to search for a dev.
-.TP
-.BR \-c,\ --cache=
-Cache number, starts from 0
-.TP
-.BR \--five-min-stats
-stats accumulated in the last 5 minutes
-.TP
-.BR \--hour-stats
-stats accumulated in the last hour
-.TP
-.BR \--day-stats
-stats accumulated in the last day
-.TP
-.BR \--total-stats
-stats accumulated in total
-- 
2.7.3

[PATCH 4/4] Make output message more human friendly

[Marcin Mirosław]

---
 libbcache.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libbcache.c b/libbcache.c
index f17bd4a..3a915e9 100644
--- a/libbcache.c
+++ b/libbcache.c
@@ -200,16 +200,16 @@ void bcache_format(struct dev_opts *devs, size_t nr_devs,
 		       "Set UUID:		%s\n"
 		       "version:		%u\n"
 		       "nbuckets:		%llu\n"
-		       "block_size:		%u\n"
-		       "bucket_size:		%u\n"
+		       "block_size (bytes):	%u\n"
+		       "bucket_size (bytes):	%u\n"
 		       "nr_in_set:		%u\n"
 		       "nr_this_dev:		%u\n"
 		       "first_bucket:		%u\n",
 		       uuid_str, set_uuid_str,
 		       (unsigned) sb->version,
 		       __le64_to_cpu(m->nbuckets),
-		       __le16_to_cpu(sb->block_size),
-		       __le16_to_cpu(m->bucket_size),
+		       __le16_to_cpu(sb->block_size)*512,
+		       __le16_to_cpu(m->bucket_size)*512,
 		       sb->nr_in_set,
 		       sb->nr_this_dev,
 		       __le16_to_cpu(m->first_bucket));
-- 
2.7.3

Re: [PATCH 4/4] Make output message more human friendly

[Coly Li]

在 16/9/9 下午9:28, Marcin Mirosław 写道:
> ---
>  libbcache.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/libbcache.c b/libbcache.c
> index f17bd4a..3a915e9 100644
> --- a/libbcache.c
> +++ b/libbcache.c
> @@ -200,16 +200,16 @@ void bcache_format(struct dev_opts *devs, size_t nr_devs,
>  		       "Set UUID:		%s\n"
>  		       "version:		%u\n"
>  		       "nbuckets:		%llu\n"
> -		       "block_size:		%u\n"
> -		       "bucket_size:		%u\n"
> +		       "block_size (bytes):	%u\n"
> +		       "bucket_size (bytes):	%u\n"
>  		       "nr_in_set:		%u\n"
>  		       "nr_this_dev:		%u\n"
>  		       "first_bucket:		%u\n",
>  		       uuid_str, set_uuid_str,
>  		       (unsigned) sb->version,
>  		       __le64_to_cpu(m->nbuckets),
> -		       __le16_to_cpu(sb->block_size),
> -		       __le16_to_cpu(m->bucket_size),
> +		       __le16_to_cpu(sb->block_size)*512,
> +		       __le16_to_cpu(m->bucket_size)*512,
>  		       sb->nr_in_set,
>  		       sb->nr_this_dev,
>  		       __le16_to_cpu(m->first_bucket));
> 
Hi Marcin,

NACK, the out put is content of data structure, it is helpful to debug
and analyze. If block_size and bucket_size is changed into bytes, in
many case people have to convert them back to sectors.

I agree this is an issue, I suggest to explain it in manual.

Thanks.

-- 
Coly Li

Re: [PATCH 4/4] Make output message more human friendly

[Marcin]

W dniu 2016-09-09 17:56, Coly Li napisał(a):
> 在 16/9/9 下午9:28, Marcin Mirosław 写道:
>> ---
>>  libbcache.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/libbcache.c b/libbcache.c
>> index f17bd4a..3a915e9 100644
>> --- a/libbcache.c
>> +++ b/libbcache.c
>> @@ -200,16 +200,16 @@ void bcache_format(struct dev_opts *devs, size_t 
>> nr_devs,
>>  		       "Set UUID:		%s\n"
>>  		       "version:		%u\n"
>>  		       "nbuckets:		%llu\n"
>> -		       "block_size:		%u\n"
>> -		       "bucket_size:		%u\n"
>> +		       "block_size (bytes):	%u\n"
>> +		       "bucket_size (bytes):	%u\n"
>>  		       "nr_in_set:		%u\n"
>>  		       "nr_this_dev:		%u\n"
>>  		       "first_bucket:		%u\n",
>>  		       uuid_str, set_uuid_str,
>>  		       (unsigned) sb->version,
>>  		       __le64_to_cpu(m->nbuckets),
>> -		       __le16_to_cpu(sb->block_size),
>> -		       __le16_to_cpu(m->bucket_size),
>> +		       __le16_to_cpu(sb->block_size)*512,
>> +		       __le16_to_cpu(m->bucket_size)*512,
>>  		       sb->nr_in_set,
>>  		       sb->nr_this_dev,
>>  		       __le16_to_cpu(m->first_bucket));
>> 
> Hi Marcin,
> 
> NACK, the out put is content of data structure, it is helpful to debug
> and analyze. If block_size and bucket_size is changed into bytes, in
> many case people have to convert them back to sectors.
> 
> I agree this is an issue, I suggest to explain it in manual.

Hi!
You are right, I also hardcoded sector size. Would it be acceptable 
change in displayed description without changing values?
+		       "block_size  (sectors):	%u\n"
+		       "bucket_size (sectors):	%u\n"

I prefer to not write man, my english isn't good enough for it.
Thanks for review.
Marcin