Re: [PATCH 1/2] btrfs-progs: subvolme: remove unused options for create and snapshot

[Sidong Yang <realwakka@gmail.com>]

On Wed, Mar 09, 2022 at 04:52:15PM +0800, Qu Wenruo wrote:
> 
> 
> On 2022/3/9 16:14, Sidong Yang wrote:
> > There are options '-c' in create subvolume and '-c' and '-x' in
> > snapshot. And the codes about them is there, but not in the manual or
> > help. This codes should be removed to avoid confusion.
> 
> I'd like more explanation on why we don't use it.
> 
> In fact the truth is, those -c/-x allows us to directly copy qgroup
> numbers from other subvolumes when creating subvolume.
> 
> This is definitely going to screw up qgroup numbers.

Actually, I don't understand that you said this option screw up qgroup
numbers. Could you explain more?
I checked that -c/-x options are not working. The commands are like
below.

# btrfs qgroup create 1/0 /mnt
# btrfs qgroup create 1/1 /mnt
# btrfs subvol create -c 1/0:1/1 /mnt/a

But it's not working. It seems that it ignores btrfs_qgroup_inherit
argument. New subvolume doesn't inherit anything.

> 
> Nowadays btrfs qgroup will automatically inherit the qgroup numbers when
> -i option is used.

Totally agree. -i option is enough to use.

Thanks,
Sidong
> 
> So the old -c/-x is no longer needed, and any inexperienced usage of
> them will lead to inconsistent qgroup numbers anyway.
> 
> Thanks,
> Qu
> 
> > 
> > Signed-off-by: Sidong Yang <realwakka@gmail.com>
> > ---
> >   cmds/subvolume.c | 25 ++-----------------------
> >   1 file changed, 2 insertions(+), 23 deletions(-)
> > 
> > diff --git a/cmds/subvolume.c b/cmds/subvolume.c
> > index fbf56566..408aebee 100644
> > --- a/cmds/subvolume.c
> > +++ b/cmds/subvolume.c
> > @@ -108,18 +108,11 @@ static int cmd_subvol_create(const struct cmd_struct *cmd,
> > 
> >   	optind = 0;
> >   	while (1) {
> > -		int c = getopt(argc, argv, "c:i:");
> > +		int c = getopt(argc, argv, "i:");
> >   		if (c < 0)
> >   			break;
> > 
> >   		switch (c) {
> > -		case 'c':
> > -			res = btrfs_qgroup_inherit_add_copy(&inherit, optarg, 0);
> > -			if (res) {
> > -				retval = res;
> > -				goto out;
> > -			}
> > -			break;
> >   		case 'i':
> >   			res = btrfs_qgroup_inherit_add_group(&inherit, optarg);
> >   			if (res) {
> > @@ -541,18 +534,11 @@ static int cmd_subvol_snapshot(const struct cmd_struct *cmd,
> >   	memset(&args, 0, sizeof(args));
> >   	optind = 0;
> >   	while (1) {
> > -		int c = getopt(argc, argv, "c:i:r");
> > +		int c = getopt(argc, argv, "i:r");
> >   		if (c < 0)
> >   			break;
> > 
> >   		switch (c) {
> > -		case 'c':
> > -			res = btrfs_qgroup_inherit_add_copy(&inherit, optarg, 0);
> > -			if (res) {
> > -				retval = res;
> > -				goto out;
> > -			}
> > -			break;
> >   		case 'i':
> >   			res = btrfs_qgroup_inherit_add_group(&inherit, optarg);
> >   			if (res) {
> > @@ -563,13 +549,6 @@ static int cmd_subvol_snapshot(const struct cmd_struct *cmd,
> >   		case 'r':
> >   			readonly = 1;
> >   			break;
> > -		case 'x':
> > -			res = btrfs_qgroup_inherit_add_copy(&inherit, optarg, 1);
> > -			if (res) {
> > -				retval = res;
> > -				goto out;
> > -			}
> > -			break;
> >   		default:
> >   			usage_unknown_option(cmd, argv);
> >   		}