| Message ID | 20191118121644.15289-1-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: Fix error messages in qgroup_rescan_init | expand |
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.
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.
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
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.
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; }
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(-) -- 2.17.1