[PATCH] btrfs: Fix error messages in qgroup_rescan_init

[PATCH] btrfs: Fix error messages in qgroup_rescan_init

[Nikolay Borisov]

The branch of qgroup_rescan_init which is executed from the mount
path prints wrong errors messages. The textual print out in case
BTRFS_QGROUP_STATUS_FLAG_RESCAN/BTRFS_QGROUP_STATUS_FLAG_ON are not
set are transposed. Fix it by exchanging their place.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/qgroup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 93aeb2e539a4..d4282e12f2a6 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -3232,12 +3232,12 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid,
 		if (!(fs_info->qgroup_flags &
 		      BTRFS_QGROUP_STATUS_FLAG_RESCAN)) {
 			btrfs_warn(fs_info,
-			"qgroup rescan init failed, qgroup is not enabled");
+			"qgroup rescan init failed, qgroup rescan is not queued");
 			ret = -EINVAL;
 		} else if (!(fs_info->qgroup_flags &
 			     BTRFS_QGROUP_STATUS_FLAG_ON)) {
 			btrfs_warn(fs_info,
-			"qgroup rescan init failed, qgroup rescan is not queued");
+			"qgroup rescan init failed, qgroup is not enabled");
 			ret = -EINVAL;
 		}

--
2.17.1

Re: [PATCH] btrfs: Fix error messages in qgroup_rescan_init

[David Sterba]

On Mon, Nov 18, 2019 at 02:16:44PM +0200, Nikolay Borisov wrote:
> The branch of qgroup_rescan_init which is executed from the mount
> path prints wrong errors messages. The textual print out in case
> BTRFS_QGROUP_STATUS_FLAG_RESCAN/BTRFS_QGROUP_STATUS_FLAG_ON are not
> set are transposed. Fix it by exchanging their place.

While that's fixing the swapped messages, I'm considering dropping some
of the messages completely. Eg. the warning 'rescan in progress' seems
useless because why I as a user should be notified about that? If I run
rescan twice and it's still in progress, there's no problem. And with
similar usability reasoning, look at the messages and drop them
eventually.

The specific messages were added by Qu in 9593bf49675ef, to improve the
message that printed function name and error code. That was an
improvement but now I'm questioning the utility of the messages.

Re: [PATCH] btrfs: Fix error messages in qgroup_rescan_init

[Nikolay Borisov]

On 19.11.19 г. 14:45 ч., David Sterba wrote:
> On Mon, Nov 18, 2019 at 02:16:44PM +0200, Nikolay Borisov wrote:
>> The branch of qgroup_rescan_init which is executed from the mount
>> path prints wrong errors messages. The textual print out in case
>> BTRFS_QGROUP_STATUS_FLAG_RESCAN/BTRFS_QGROUP_STATUS_FLAG_ON are not
>> set are transposed. Fix it by exchanging their place.
> 
> While that's fixing the swapped messages, I'm considering dropping some
> of the messages completely. Eg. the warning 'rescan in progress' seems
> useless because why I as a user should be notified about that? If I run
> rescan twice and it's still in progress, there's no problem. And with
> similar usability reasoning, look at the messages and drop them
> eventually.
> 
> The specific messages were added by Qu in 9593bf49675ef, to improve the
> message that printed function name and error code. That was an
> improvement but now I'm questioning the utility of the messages.
> 

I'm fine either ways but if the messages are going to stay they need to
be fixed.

Re: [PATCH] btrfs: Fix error messages in qgroup_rescan_init

[Qu Wenruo]

On 2019/11/19 下午8:45, David Sterba wrote:
> On Mon, Nov 18, 2019 at 02:16:44PM +0200, Nikolay Borisov wrote:
>> The branch of qgroup_rescan_init which is executed from the mount
>> path prints wrong errors messages. The textual print out in case
>> BTRFS_QGROUP_STATUS_FLAG_RESCAN/BTRFS_QGROUP_STATUS_FLAG_ON are not
>> set are transposed. Fix it by exchanging their place.
>
> While that's fixing the swapped messages, I'm considering dropping some
> of the messages completely. Eg. the warning 'rescan in progress' seems
> useless because why I as a user should be notified about that? If I run
> rescan twice and it's still in progress, there's no problem. And with
> similar usability reasoning, look at the messages and drop them
> eventually.
>
> The specific messages were added by Qu in 9593bf49675ef, to improve the
> message that printed function name and error code. That was an
> improvement but now I'm questioning the utility of the messages.
>

Yep, that 'rescan in progress' message looks useless in that use case.

But normal user doesn't really check dmesg that frequently, and as long
as btrfs-progs doesn't report error directly to user, it should be more
or less OK for the user.

To me, such message acts like a hidden verbose message.
It doesn't provide much info, until we hit some real problem.


So I'm OK just removing that 'rescan in progress' message, but please at
least considering keep other messages so that they could provide some
clue in the future.

Thanks,
Qu

Re: [PATCH] btrfs: Fix error messages in qgroup_rescan_init

[David Sterba]

On Mon, Nov 18, 2019 at 02:16:44PM +0200, Nikolay Borisov wrote:
> The branch of qgroup_rescan_init which is executed from the mount
> path prints wrong errors messages. The textual print out in case
> BTRFS_QGROUP_STATUS_FLAG_RESCAN/BTRFS_QGROUP_STATUS_FLAG_ON are not
> set are transposed. Fix it by exchanging their place.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

I'll apply the patch now as it's a clear fix, dropping the messages can
go separately.