Re: [PATCH v2 1/3] btrfs-progs: fi defrag: clean up duplicate code if find errors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2017年11月28日 17:14, Su Yue wrote:
> In function cmd_filesystem_defrag(), lines of code for error handling
> are duplicate and hard to expand in further.
> 
> Create a jump label for errors.
> 
> Signed-off-by: Su Yue <suy.fnst@xxxxxxxxxxxxxx>
> ---
> Changelog:
> v2:	Record -errno to return value if open() and fstat() failed.
> 	Move change "do no exit if defrag range ioctl is unsupported"
> 	to another patch.
> 	Suggested by David.
> ---
>  cmds-filesystem.c | 44 ++++++++++++++++++++++++--------------------
>  1 file changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
> index 7728430f16a1..17d399d58adf 100644
> --- a/cmds-filesystem.c
> +++ b/cmds-filesystem.c
> @@ -1029,23 +1029,22 @@ static int cmd_filesystem_defrag(int argc, char **argv)
>  		if (fd < 0) {
>  			error("cannot open %s: %s", argv[i],
>  					strerror(errno));
> -			defrag_global_errors++;
> -			close_file_or_dir(fd, dirstream);
> -			continue;
> +			ret = -errno;
> +			goto next;
>  		}
> -		if (fstat(fd, &st)) {
> +
> +		ret = fstat(fd, &st);
> +		if (ret) {
>  			error("failed to stat %s: %s",
>  					argv[i], strerror(errno));
> -			defrag_global_errors++;
> -			close_file_or_dir(fd, dirstream);
> -			continue;
> +			ret = -errno;
> +			goto next;
>  		}
>  		if (!(S_ISDIR(st.st_mode) || S_ISREG(st.st_mode))) {
>  			error("%s is not a directory or a regular file",
>  					argv[i]);
> -			defrag_global_errors++;
> -			close_file_or_dir(fd, dirstream);
> -			continue;
> +			ret = -EINVAL;
> +			goto next;

Here, unlike the ftw callback, which ignore non-regular file, here we
count them as error.

So if there is some especial file in dir (use "example_dir" as an
example), then
# btrfs fi defrag -r example_dir
will return no error.

While
# btrfs fi defrag -r example_dir/*
Will return error.

IIRC such inconsistent behavior is a bigger problem, and reusing the
defrag_callback() will not only makes the behavior consistent, but also
simplify the loop.

Thanks,
Qu


>  		}
>  		if (recursive && S_ISDIR(st.st_mode)) {
>  			ret = nftw(argv[i], defrag_callback, 10,
> @@ -1060,20 +1059,25 @@ static int cmd_filesystem_defrag(int argc, char **argv)
>  			ret = ioctl(fd, BTRFS_IOC_DEFRAG_RANGE,
>  					&defrag_global_range);
>  			defrag_err = errno;
> -		}
> -		close_file_or_dir(fd, dirstream);
> -		if (ret && defrag_err == ENOTTY) {
> -			error(
> +			if (ret && defrag_err == ENOTTY) {
> +				error(
>  "defrag range ioctl not supported in this kernel version, 2.6.33 and newer is required");
> -			defrag_global_errors++;
> -			break;
> +				defrag_global_errors++;
> +				close_file_or_dir(fd, dirstream);
> +				break;
> +			}
> +			if (ret) {
> +				error("defrag failed on %s: %s", argv[i],
> +				      strerror(defrag_err));
> +				goto next;
> +			}
>  		}
> -		if (ret) {
> -			error("defrag failed on %s: %s", argv[i],
> -					strerror(defrag_err));
> +next:
> +		if (ret)
>  			defrag_global_errors++;
> -		}
> +		close_file_or_dir(fd, dirstream);
>  	}
> +
>  	if (defrag_global_errors)
>  		fprintf(stderr, "total %d failures\n", defrag_global_errors);
>  
> 

Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux