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

[Qu Wenruo <wqu@suse.com>]

在 2024/3/27 06:57, Qu Wenruo 写道:
> 
> 
> 在 2024/3/27 06:53, Boris Burkov 写道:
>> On Fri, Mar 01, 2024 at 01:56:31PM +0100, David Sterba wrote:
>>> On Tue, Feb 27, 2024 at 02:41:16PM +1030, Qu Wenruo wrote:
>>>> [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>
>>>
>>> Added to devel, thanks.
>>
>> This patch breaks every test that creates snapshots or subvolumes and
>> expects the output in the outfile.
>>
>> That's because it did:
>> s/Create a snapshot/Create snapshot/
>>
>> Please run the tests when making changes! This failed on btrfs/001, so
>> it would have taken 1 second to see.
> 
> Wrong patch to blame?
> 
> The message is kept the same in the patch:
> 
> -        pr_verbose(LOG_DEFAULT,
> -               "Create a readonly snapshot of '%s' in '%s/%s'\n",
> -               subvol, dstdir, newname);
> -        pr_verbose(LOG_DEFAULT,
> -               "Create a snapshot of '%s' in '%s/%s'\n",
> -               subvol, dstdir, newname);
> 
> +        pr_verbose(LOG_DEFAULT,
> +               "Create a readonly snapshot of '%s' in '%s/%s'\n",
> +               subvol, dstdir, newname);
> +        pr_verbose(LOG_DEFAULT,
> +               "Create a snapshot of '%s' in '%s/%s'\n",
> +               subvol, dstdir, newname);
> 
> Thanks,
> Qu

OK, David seems to changed the output line when merging the patch...

That's something out of my reach.

Thanks,
Qu