[PATCH 1/4] btrfs-progs: Avoid interpreting options after "--" when getting unit mode

[PATCH 1/4] btrfs-progs: Avoid interpreting options after "--" when getting unit mode

[Satoru Takeuchi]

* actual result

  ======================================
  # ./btrfs device usage -- -m /btrfs

  /dev/sdf1, ID: 1
     Device size:          95367.41MiB
     Data,single:          2056.00MiB
     Metadata,DUP:         2048.00MiB
     System,DUP:             16.00MiB
     Unallocated:          91247.41MiB
  ======================================

* expected result

  ======================================
  # ./btrfs device usage -- -m /btrfs

  ERROR: can't access '-m': No such file or directory
  ======================================

Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
---
This patch can be applied to devel branch (commit: 31266d0ef811).
---
 utils.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/utils.c b/utils.c
index d7ceaa8..77f0f68 100644
--- a/utils.c
+++ b/utils.c
@@ -3024,6 +3024,9 @@ unsigned int get_unit_mode_from_arg(int *argc, char *argv[], int df_mode)
 	int arg_end;

 	for (arg_i = 0; arg_i < *argc; arg_i++) {
+		if (!strcmp(argv[arg_i], "--"))
+			break;
+
 		if (!strcmp(argv[arg_i], "--raw")) {
 			unit_mode = UNITS_RAW;
 			argv[arg_i] = NULL;
-- 
2.7.0

[PATCH 2/4] btrfs-progs: fix a reression that "property" with -t option doesn't work

[Satoru Takeuchi]

"property" is considered as working without any options
from the following commit.

commit 176aeca9a148 ("btrfs-progs: add getopt stubs where needed")

However, we can pass -t option to this command.

* actual result

  ==================================================
  # ./btrfs prop list -t f /btrfs
  btrfs property list: invalid option -- 't'
  usage: btrfs property list [-t <type>] <object>

      Lists available properties with their descriptions for the given object.

      Please see the help of 'btrfs property get' for a description of
      objects and object types.
  ==================================================

* expected result

  ==================================================
  # btrfs prop list -t f /btrfs
  label               Set/get label of device
  ==================================================

Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
---
 cmds-property.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/cmds-property.c b/cmds-property.c
index 5b4da26..4c2beb7 100644
--- a/cmds-property.c
+++ b/cmds-property.c
@@ -379,9 +379,7 @@ static int cmd_property_get(int argc, char **argv)
 	char *name = NULL;
 	int types = 0;

-	clean_args_no_options(argc, argv, cmd_property_get_usage);
-
-	if (check_argc_min(argc, 2) || check_argc_max(argc, 5))
+	if (check_argc_min(argc, 2))
 		usage(cmd_property_get_usage);

 	parse_args(argc, argv, cmd_property_get_usage, &types, &object, &name,
@@ -415,9 +413,7 @@ static int cmd_property_set(int argc, char **argv)
 	char *value = NULL;
 	int types = 0;

-	clean_args_no_options(argc, argv, cmd_property_set_usage);
-
-	if (check_argc_min(argc, 4) || check_argc_max(argc, 6))
+	if (check_argc_min(argc, 4))
 		usage(cmd_property_set_usage);

 	parse_args(argc, argv, cmd_property_set_usage, &types,
@@ -446,9 +442,7 @@ static int cmd_property_list(int argc, char **argv)
 	char *object = NULL;
 	int types = 0;

-	clean_args_no_options(argc, argv, cmd_property_list_usage);
-
-	if (check_argc_min(argc, 2) || check_argc_max(argc, 4))
+	if (check_argc_min(argc, 2))
 		usage(cmd_property_list_usage);

 	parse_args(argc, argv, cmd_property_list_usage,
-- 
2.7.0

Re: [PATCH 2/4] btrfs-progs: fix a reression that "property" with -t option doesn't work

[David Sterba]

On Mon, Mar 14, 2016 at 09:12:36AM +0900, Satoru Takeuchi wrote:
> --- a/cmds-property.c
> +++ b/cmds-property.c
> @@ -379,9 +379,7 @@ static int cmd_property_get(int argc, char **argv)
>  	char *name = NULL;
>  	int types = 0;
> 
> -	clean_args_no_options(argc, argv, cmd_property_get_usage);
> -
> -	if (check_argc_min(argc, 2) || check_argc_max(argc, 5))
> +	if (check_argc_min(argc, 2))
>  		usage(cmd_property_get_usage);
> 
>  	parse_args(argc, argv, cmd_property_get_usage, &types, &object, &name,

We still need to check the number of non-option arguments here, when the
optind is set from parse_args.

> @@ -415,9 +413,7 @@ static int cmd_property_set(int argc, char **argv)
> -	if (check_argc_min(argc, 4) || check_argc_max(argc, 6))
> +	if (check_argc_min(argc, 4))
>  		usage(cmd_property_set_usage);

...

>  	parse_args(argc, argv, cmd_property_set_usage, &types,
> @@ -446,9 +442,7 @@ static int cmd_property_list(int argc, char **argv)
> -	if (check_argc_min(argc, 2) || check_argc_max(argc, 4))
> +	if (check_argc_min(argc, 2))

...

Re: [PATCH 2/4] btrfs-progs: fix a reression that "property" with -t option doesn't work

[Satoru Takeuchi]

On 2016/03/14 21:23, David Sterba wrote:
> On Mon, Mar 14, 2016 at 09:12:36AM +0900, Satoru Takeuchi wrote:
>> --- a/cmds-property.c
>> +++ b/cmds-property.c
>> @@ -379,9 +379,7 @@ static int cmd_property_get(int argc, char **argv)
>>   	char *name = NULL;
>>   	int types = 0;
>>
>> -	clean_args_no_options(argc, argv, cmd_property_get_usage);
>> -
>> -	if (check_argc_min(argc, 2) || check_argc_max(argc, 5))
>> +	if (check_argc_min(argc, 2))
>>   		usage(cmd_property_get_usage);
>>
>>   	parse_args(argc, argv, cmd_property_get_usage, &types, &object, &name,
>
> We still need to check the number of non-option arguments here, when the
> optind is set from parse_args.

OK, I'll send a patch which checks the number after getopt.

Thanks,
Satoru

>
>> @@ -415,9 +413,7 @@ static int cmd_property_set(int argc, char **argv)
>> -	if (check_argc_min(argc, 4) || check_argc_max(argc, 6))
>> +	if (check_argc_min(argc, 4))
>>   		usage(cmd_property_set_usage);
>
> ...
>
>>   	parse_args(argc, argv, cmd_property_set_usage, &types,
>> @@ -446,9 +442,7 @@ static int cmd_property_list(int argc, char **argv)
>> -	if (check_argc_min(argc, 2) || check_argc_max(argc, 4))
>> +	if (check_argc_min(argc, 2))
>
> ...
>

[PATCH 3/4] btrfs-progs: Fix a regression that btrfs filesystem label doesn't work

[Satoru Takeuchi]

The number of arguments which is allowed to pass became wrong
from the following commit.

commit 176aeca9a148c5e29de0 ("btrfs-progs: add getopt stubs where needed")

* actual result

  ===========================================================
  # ./btrfs prop get /btrfs label
  label=foo
  # ./btrfs fi label /btrfs
  btrfs filesystem label: too few arguments
  usage: btrfs filesystem label [<device>|<mount_point>] [<newlabel>]

      Get or change the label of a filesystem

      With one argument, get the label of filesystem on <device>.
      If <newlabel> is passed, set the filesystem label to <newlabel>.

  # ./btrfs fi label /btrfs bar
  foo
  # ./btrfs prop get /btrfs label
  label=foo
  ===========================================================

* expected result

  ===========================================================
  # ./btrfs prop get /btrfs label
  label=foo
  # ./btrfs fi label /btrfs
  foo
  # ./btrfs fi label /btrfs bar
  # ./btrfs prop get /btrfs label
  label=bar
  ===========================================================

Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
---
 cmds-filesystem.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 7e74e1b..45c16d2 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -1255,11 +1255,11 @@ static int cmd_filesystem_label(int argc, char **argv)
 {
 	clean_args_no_options(argc, argv, cmd_filesystem_label_usage);

-	if (check_argc_min(argc - optind, 2) ||
-			check_argc_max(argc - optind, 3))
+	if (check_argc_min(argc - optind, 1) ||
+			check_argc_max(argc - optind, 2))
 		usage(cmd_filesystem_label_usage);

-	if (argc - optind > 2) {
+	if (argc - optind > 1) {
 		return set_label(argv[optind], argv[optind + 1]);
 	} else {
 		char label[BTRFS_LABEL_SIZE];
-- 
2.7.0

Re: [PATCH 3/4] btrfs-progs: Fix a regression that btrfs filesystem label doesn't work

[David Sterba]

On Mon, Mar 14, 2016 at 09:15:42AM +0900, Satoru Takeuchi wrote:
> The number of arguments which is allowed to pass became wrong
> from the following commit.
> 
> commit 176aeca9a148c5e29de0 ("btrfs-progs: add getopt stubs where needed")
> 
> * actual result
> 
>   ===========================================================
>   # ./btrfs prop get /btrfs label
>   label=foo
>   # ./btrfs fi label /btrfs
>   btrfs filesystem label: too few arguments
>   usage: btrfs filesystem label [<device>|<mount_point>] [<newlabel>]
> 
>       Get or change the label of a filesystem
> 
>       With one argument, get the label of filesystem on <device>.
>       If <newlabel> is passed, set the filesystem label to <newlabel>.
> 
>   # ./btrfs fi label /btrfs bar
>   foo
>   # ./btrfs prop get /btrfs label
>   label=foo
>   ===========================================================
> 
> * expected result
> 
>   ===========================================================
>   # ./btrfs prop get /btrfs label
>   label=foo
>   # ./btrfs fi label /btrfs
>   foo
>   # ./btrfs fi label /btrfs bar
>   # ./btrfs prop get /btrfs label
>   label=bar
>   ===========================================================
> 
> Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>

Applied, thanks.

[PATCH 4/4] btrfs-progs: "device ready" accepts just one device

[Satoru Takeuchi]

* actual result

  =======================================
  # ./btrfs device ready /dev/sdb foo
  #
  =======================================

* expecting result

  =======================================
  # ./btrfs device ready /dev/sdb foo
  btrfs device ready: too many arguments
  usage: btrfs device ready <device>

      Check device to see if it has all of its devices in cache for mounting

  #
  =======================================

Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
---
 cmds-device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmds-device.c b/cmds-device.c
index 33da2ce..23656c3 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -326,7 +326,7 @@ static int cmd_device_ready(int argc, char **argv)

 	clean_args_no_options(argc, argv, cmd_device_ready_usage);

-	if (check_argc_min(argc - optind, 1))
+	if (check_argc_exact(argc - optind, 1))
 		usage(cmd_device_ready_usage);

 	fd = open("/dev/btrfs-control", O_RDWR);
-- 
2.7.0

Re: [PATCH 4/4] btrfs-progs: "device ready" accepts just one device

[David Sterba]

On Mon, Mar 14, 2016 at 09:27:22AM +0900, Satoru Takeuchi wrote:
> * actual result
> 
>   =======================================
>   # ./btrfs device ready /dev/sdb foo
>   #
>   =======================================
> 
> * expecting result
> 
>   =======================================
>   # ./btrfs device ready /dev/sdb foo
>   btrfs device ready: too many arguments
>   usage: btrfs device ready <device>
> 
>       Check device to see if it has all of its devices in cache for mounting
> 
>   #
>   =======================================
> 
> Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
> ---
>  cmds-device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/cmds-device.c b/cmds-device.c
> index 33da2ce..23656c3 100644
> --- a/cmds-device.c
> +++ b/cmds-device.c
> @@ -326,7 +326,7 @@ static int cmd_device_ready(int argc, char **argv)
> 
>  	clean_args_no_options(argc, argv, cmd_device_ready_usage);
> 
> -	if (check_argc_min(argc - optind, 1))
> +	if (check_argc_exact(argc - optind, 1))

This silently changes the semantics, so far it accepts multiple values
though it contradicts the documentation. I'm not yet sure how to resolve
that.

>  		usage(cmd_device_ready_usage);
> 
>  	fd = open("/dev/btrfs-control", O_RDWR);
> -- 
> 2.7.0
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] btrfs-progs: "device ready" accepts just one device

[David Sterba]

On Mon, Mar 14, 2016 at 01:05:15PM +0100, David Sterba wrote:
> On Mon, Mar 14, 2016 at 09:27:22AM +0900, Satoru Takeuchi wrote:
> > * actual result
> > 
> >   =======================================
> >   # ./btrfs device ready /dev/sdb foo
> >   #
> >   =======================================
> > 
> > * expecting result
> > 
> >   =======================================
> >   # ./btrfs device ready /dev/sdb foo
> >   btrfs device ready: too many arguments
> >   usage: btrfs device ready <device>
> > 
> >       Check device to see if it has all of its devices in cache for mounting
> > 
> >   #
> >   =======================================
> > 
> > Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
> > ---
> >  cmds-device.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/cmds-device.c b/cmds-device.c
> > index 33da2ce..23656c3 100644
> > --- a/cmds-device.c
> > +++ b/cmds-device.c
> > @@ -326,7 +326,7 @@ static int cmd_device_ready(int argc, char **argv)
> > 
> >  	clean_args_no_options(argc, argv, cmd_device_ready_usage);
> > 
> > -	if (check_argc_min(argc - optind, 1))
> > +	if (check_argc_exact(argc - optind, 1))
> 
> This silently changes the semantics, so far it accepts multiple values
> though it contradicts the documentation. I'm not yet sure how to resolve
> that.

More than one argument did not work before, so I think it's ok to expect
just one device. Applied.

Re: [PATCH 1/4] btrfs-progs: Avoid interpreting options after "--" when getting unit mode

[David Sterba]

On Mon, Mar 14, 2016 at 09:02:34AM +0900, Satoru Takeuchi wrote:
> * actual result
> 
>   ======================================
>   # ./btrfs device usage -- -m /btrfs
> 
>   /dev/sdf1, ID: 1
>      Device size:          95367.41MiB
>      Data,single:          2056.00MiB
>      Metadata,DUP:         2048.00MiB
>      System,DUP:             16.00MiB
>      Unallocated:          91247.41MiB
>   ======================================
> 
> * expected result
> 
>   ======================================
>   # ./btrfs device usage -- -m /btrfs
> 
>   ERROR: can't access '-m': No such file or directory
>   ======================================
> 
> Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>

Applied, thanks.