* [PATCH] btrfs-progs: subvolume: output the prompt line only when the ioctl succeeded
@ 2024-02-27 4:11 Qu Wenruo
2024-02-27 4:59 ` HAN Yuwei
2024-03-01 12:56 ` David Sterba
0 siblings, 2 replies; 10+ messages in thread
From: Qu Wenruo @ 2024-02-27 4:11 UTC (permalink / raw
To: linux-btrfs
[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);
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);
--
2.43.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] btrfs-progs: subvolume: output the prompt line only when the ioctl succeeded
2024-02-27 4:11 [PATCH] btrfs-progs: subvolume: output the prompt line only when the ioctl succeeded Qu Wenruo
@ 2024-02-27 4:59 ` HAN Yuwei
2024-02-27 5:31 ` Qu Wenruo
2024-03-01 12:56 ` David Sterba
1 sibling, 1 reply; 10+ messages in thread
From: HAN Yuwei @ 2024-02-27 4:59 UTC (permalink / raw
To: Qu Wenruo, linux-btrfs
[-- Attachment #1.1.1: Type: text/plain, Size: 2694 bytes --]
> [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" ?
> 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);
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 1589 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] btrfs-progs: subvolume: output the prompt line only when the ioctl succeeded
2024-02-27 4:59 ` HAN Yuwei
@ 2024-02-27 5:31 ` Qu Wenruo
0 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2024-02-27 5:31 UTC (permalink / raw
To: HAN Yuwei, linux-btrfs
在 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);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] btrfs-progs: subvolume: output the prompt line only when the ioctl succeeded
2024-02-27 4:11 [PATCH] btrfs-progs: subvolume: output the prompt line only when the ioctl succeeded Qu Wenruo
2024-02-27 4:59 ` HAN Yuwei
@ 2024-03-01 12:56 ` David Sterba
2024-03-26 20:23 ` Boris Burkov
1 sibling, 1 reply; 10+ messages in thread
From: David Sterba @ 2024-03-01 12:56 UTC (permalink / raw
To: Qu Wenruo; +Cc: linux-btrfs
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] btrfs-progs: subvolume: output the prompt line only when the ioctl succeeded
2024-03-01 12:56 ` David Sterba
@ 2024-03-26 20:23 ` Boris Burkov
2024-03-26 20:27 ` Qu Wenruo
0 siblings, 1 reply; 10+ messages in thread
From: Boris Burkov @ 2024-03-26 20:23 UTC (permalink / raw
To: David Sterba; +Cc: Qu Wenruo, linux-btrfs
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] btrfs-progs: subvolume: output the prompt line only when the ioctl succeeded
2024-03-26 20:23 ` Boris Burkov
@ 2024-03-26 20:27 ` Qu Wenruo
2024-03-26 20:30 ` Qu Wenruo
2024-03-26 20:33 ` Boris Burkov
0 siblings, 2 replies; 10+ messages in thread
From: Qu Wenruo @ 2024-03-26 20:27 UTC (permalink / raw
To: Boris Burkov, David Sterba; +Cc: linux-btrfs
在 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] btrfs-progs: subvolume: output the prompt line only when the ioctl succeeded
2024-03-26 20:27 ` Qu Wenruo
@ 2024-03-26 20:30 ` Qu Wenruo
2024-03-26 20:34 ` Boris Burkov
2024-03-26 20:33 ` Boris Burkov
1 sibling, 1 reply; 10+ messages in thread
From: Qu Wenruo @ 2024-03-26 20:30 UTC (permalink / raw
To: Boris Burkov, David Sterba; +Cc: linux-btrfs
在 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] btrfs-progs: subvolume: output the prompt line only when the ioctl succeeded
2024-03-26 20:27 ` Qu Wenruo
2024-03-26 20:30 ` Qu Wenruo
@ 2024-03-26 20:33 ` Boris Burkov
1 sibling, 0 replies; 10+ messages in thread
From: Boris Burkov @ 2024-03-26 20:33 UTC (permalink / raw
To: Qu Wenruo; +Cc: David Sterba, linux-btrfs
On Wed, Mar 27, 2024 at 06:57:48AM +1030, Qu Wenruo wrote:
>
>
> 在 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
Hmm, something weird happened. I'm looking at commit
5f87b467a9e7 ("btrfs-progs: subvolume: output the prompt line only when the ioctl succeeded")
in the master branch of https://github.com/kdave/btrfs-progs.git
and it has the (relevant) diff:
- 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 readonly snapshot of '%s' in '%s/%s'\n",
+ subvol, dstdir, newname);
+ else
+ pr_verbose(LOG_DEFAULT,
+ "Create snapshot of '%s' in '%s/%s'\n",
+ subvol, dstdir, newname);
+
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] btrfs-progs: subvolume: output the prompt line only when the ioctl succeeded
2024-03-26 20:30 ` Qu Wenruo
@ 2024-03-26 20:34 ` Boris Burkov
2024-03-26 22:44 ` David Sterba
0 siblings, 1 reply; 10+ messages in thread
From: Boris Burkov @ 2024-03-26 20:34 UTC (permalink / raw
To: Qu Wenruo; +Cc: David Sterba, linux-btrfs
On Wed, Mar 27, 2024 at 07:00:29AM +1030, Qu Wenruo wrote:
>
>
> 在 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.
Agreed. Sorry about the test rant.
>
> Thanks,
> Qu
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] btrfs-progs: subvolume: output the prompt line only when the ioctl succeeded
2024-03-26 20:34 ` Boris Burkov
@ 2024-03-26 22:44 ` David Sterba
0 siblings, 0 replies; 10+ messages in thread
From: David Sterba @ 2024-03-26 22:44 UTC (permalink / raw
To: Boris Burkov; +Cc: Qu Wenruo, linux-btrfs
On Tue, Mar 26, 2024 at 01:34:43PM -0700, Boris Burkov wrote:
> On Wed, Mar 27, 2024 at 07:00:29AM +1030, Qu Wenruo wrote:
> >
> >
> > 在 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.
>
> Agreed. Sorry about the test rant.
I changed it so the message is unified with others, Anand promised to
add/fix the fstests filter. What fstests does is fragile and this is
unfortunatelly not the last time this happens.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-03-26 22:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-27 4:11 [PATCH] btrfs-progs: subvolume: output the prompt line only when the ioctl succeeded Qu Wenruo
2024-02-27 4:59 ` HAN Yuwei
2024-02-27 5:31 ` Qu Wenruo
2024-03-01 12:56 ` David Sterba
2024-03-26 20:23 ` Boris Burkov
2024-03-26 20:27 ` Qu Wenruo
2024-03-26 20:30 ` Qu Wenruo
2024-03-26 20:34 ` Boris Burkov
2024-03-26 22:44 ` David Sterba
2024-03-26 20:33 ` Boris Burkov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.