Re: [PATCH] btrfs-progs: subvolume: output the prompt line only when the ioctl succeeded

[Qu Wenruo <wqu@suse.com>]

在 2024/2/27 15:29, HAN Yuwei 写道:
>> [BUG]
>> With the latest kernel patch to reject invalid qgroupids in
>> btrfs_qgroup_inherit structure, "btrfs subvolume create" or "btrfs
>> subvolume snapshot" can lead to the following output:
>>
>>   # mkfs.btrfs -O quota -f $dev
>>   # mount $dev $mnt
>>   # btrfs subvolume create -i 2/0 $mnt/subv1
>>   Create subvolume '/mnt/btrfs/subv1'
>>   ERROR: cannot create subvolume: No such file or directory
>>
>> The "btrfs subvolume" command output the first line, seemingly to
>> indicate a successful subvolume creation, then followed by an error
>> message.
>>
>> This can be a little confusing on whether if the subvolume is created or
>> not.
>>
>> [FIX]
>> Fix the output by only outputting the regular line if the ioctl
>> succeeded.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   cmds/subvolume.c | 21 +++++++++++----------
>>   1 file changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/cmds/subvolume.c b/cmds/subvolume.c
>> index 00c5eacfa694..1549adaca642 100644
>> --- a/cmds/subvolume.c
>> +++ b/cmds/subvolume.c
>> @@ -229,7 +229,6 @@ static int create_one_subvolume(const char *dst, 
>> struct btrfs_qgroup_inherit *in
>>           goto out;
>>       }
>> -    pr_verbose(LOG_DEFAULT, "Create subvolume '%s/%s'\n", dstdir, 
>> newname);
>>       if (inherit) {
>>           struct btrfs_ioctl_vol_args_v2    args;
>> @@ -253,6 +252,7 @@ static int create_one_subvolume(const char *dst, 
>> struct btrfs_qgroup_inherit *in
>>           error("cannot create subvolume: %m");
>>           goto out;
>>       }
>> +    pr_verbose(LOG_DEFAULT, "Create subvolume '%s/%s'\n", dstdir, 
>> newname);
> 
> 
> How about saying "Created subvolume %s/%s" ?

Sounds pretty reasonable, would go that way in the next update.

Thanks,
Qu
> 
> 
>>   out:
>>       close(fddst);
>> @@ -754,16 +754,8 @@ static int cmd_subvolume_snapshot(const struct 
>> cmd_struct *cmd, int argc, char *
>>       if (fd < 0)
>>           goto out;
>> -    if (readonly) {
>> +    if (readonly)
>>           args.flags |= BTRFS_SUBVOL_RDONLY;
>> -        pr_verbose(LOG_DEFAULT,
>> -               "Create a readonly snapshot of '%s' in '%s/%s'\n",
>> -               subvol, dstdir, newname);
>> -    } else {
>> -        pr_verbose(LOG_DEFAULT,
>> -               "Create a snapshot of '%s' in '%s/%s'\n",
>> -               subvol, dstdir, newname);
>> -    }
>>       args.fd = fd;
>>       if (inherit) {
>> @@ -784,6 +776,15 @@ static int cmd_subvolume_snapshot(const struct 
>> cmd_struct *cmd, int argc, char *
>>       retval = 0;    /* success */
>> +    if (readonly)
>> +        pr_verbose(LOG_DEFAULT,
>> +               "Create a readonly snapshot of '%s' in '%s/%s'\n",
>> +               subvol, dstdir, newname);
>> +    else
>> +        pr_verbose(LOG_DEFAULT,
>> +               "Create a snapshot of '%s' in '%s/%s'\n",
>> +               subvol, dstdir, newname);
>> +
>>   out:
>>       close(fddst);
>>       close(fd);