[PATCH] btrfs-progs: quota: Check for args.progress in cmd_quota_rescan

[PATCH] btrfs-progs: quota: Check for args.progress in cmd_quota_rescan

[Marcos Paulo de Souza]

The progress struct member is only set when there is a rescan being
executed, so it's more readable read it directly.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 cmds/quota.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/cmds/quota.c b/cmds/quota.c
index 246dd277..2038707e 100644
--- a/cmds/quota.c
+++ b/cmds/quota.c
@@ -166,11 +166,11 @@ static int cmd_quota_rescan(const struct cmd_struct *cmd, int argc, char **argv)
 			error("could not obtain quota rescan status: %m");
 			return 1;
 		}
-		if (!args.flags)
-			printf("no rescan operation in progress\n");
-		else
+		if (args.progress)
 			printf("rescan operation running (current key %lld)\n",
 				args.progress);
+		else
+			printf("no rescan operation in progress\n");
 		return 0;
 	}
 
-- 
2.26.2

Re: [PATCH] btrfs-progs: quota: Check for args.progress in cmd_quota_rescan

[David Sterba]

On Fri, Jun 11, 2021 at 11:43:01AM -0300, Marcos Paulo de Souza wrote:
> The progress struct member is only set when there is a rescan being
> executed, so it's more readable read it directly.

I'm not sure this is right, kernel sets the flags and progress together,
ie. the progress value depends on the flags being set, so reading just
the progress does not follow the "ioctl protocol".

If the reason is to make the code readable I'd say it's obscuring the
fact that there's some semantics behind the ioctl so it should not be
obscured for readability reasons.

> 
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
>  cmds/quota.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/cmds/quota.c b/cmds/quota.c
> index 246dd277..2038707e 100644
> --- a/cmds/quota.c
> +++ b/cmds/quota.c
> @@ -166,11 +166,11 @@ static int cmd_quota_rescan(const struct cmd_struct *cmd, int argc, char **argv)
>  			error("could not obtain quota rescan status: %m");
>  			return 1;
>  		}
> -		if (!args.flags)
> -			printf("no rescan operation in progress\n");
> -		else
> +		if (args.progress)
>  			printf("rescan operation running (current key %lld)\n",
>  				args.progress);
> +		else
> +			printf("no rescan operation in progress\n");

This could be improved that it checks for bit 0 set, and so should do
the kernel. Right now we haven't had anything else than the progress so
switching qsa->flags = 1 to |= 1 in btrfs_ioctl_quota_rescan_status
should be doing the same thing.

>  		return 0;
>  	}
>  
> -- 
> 2.26.2