[PATCH v2] mkfs: simplify the configured sector sizes setting in validate_sectorsize

[PATCH v2] mkfs: simplify the configured sector sizes setting in validate_sectorsize

[xiakaixu1987]

From: Kaixu Xia <kaixuxia@tencent.com>

There are two places that set the configured sector sizes in
validate_sectorsize, actually we can simplify them and combine into one
if statement. Use the default value structure to set the topology sectors
when probing fails.

Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
---
v2:
 -Use the default value structure to set the topology sectors.

 mkfs/xfs_mkfs.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 039b1dcc..d553b0a0 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -1696,14 +1696,6 @@ validate_sectorsize(
 	int			dry_run,
 	int			force_overwrite)
 {
-	/* set configured sector sizes in preparation for checks */
-	if (!cli->sectorsize) {
-		cfg->sectorsize = dft->sectorsize;
-	} else {
-		cfg->sectorsize = cli->sectorsize;
-	}
-	cfg->sectorlog = libxfs_highbit32(cfg->sectorsize);
-
 	/*
 	 * Before anything else, verify that we are correctly operating on
 	 * files or block devices and set the control parameters correctly.
@@ -1730,6 +1722,7 @@ validate_sectorsize(
 	memset(ft, 0, sizeof(*ft));
 	get_topology(cli->xi, ft, force_overwrite);
 
+	/* set configured sector sizes in preparation for checks */
 	if (!cli->sectorsize) {
 		/*
 		 * Unless specified manually on the command line use the
@@ -1741,9 +1734,8 @@ validate_sectorsize(
 		 * Set the topology sectors if they were not probed to the
 		 * minimum supported sector size.
 		 */
-
 		if (!ft->lsectorsize)
-			ft->lsectorsize = XFS_MIN_SECTORSIZE;
+			ft->lsectorsize = dft->sectorsize;
 
 		/* Older kernels may not have physical/logical distinction */
 		if (!ft->psectorsize)
@@ -1759,9 +1751,10 @@ _("specified blocksize %d is less than device physical sector size %d\n"
 				ft->lsectorsize);
 			cfg->sectorsize = ft->lsectorsize;
 		}
+	} else
+		cfg->sectorsize = cli->sectorsize;
 
-		cfg->sectorlog = libxfs_highbit32(cfg->sectorsize);
-	}
+	cfg->sectorlog = libxfs_highbit32(cfg->sectorsize);
 
 	/* validate specified/probed sector size */
 	if (cfg->sectorsize < XFS_MIN_SECTORSIZE ||
-- 
2.20.0

Re: [PATCH v2] mkfs: simplify the configured sector sizes setting in validate_sectorsize

[Eric Sandeen]

On 5/19/20 10:24 PM, xiakaixu1987@gmail.com wrote:
> From: Kaixu Xia <kaixuxia@tencent.com>
> 
> There are two places that set the configured sector sizes in
> validate_sectorsize, actually we can simplify them and combine into one
> if statement. Use the default value structure to set the topology sectors
> when probing fails.
> 
> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
> v2:
>  -Use the default value structure to set the topology sectors.
> 
>  mkfs/xfs_mkfs.c | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 039b1dcc..d553b0a0 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1696,14 +1696,6 @@ validate_sectorsize(
>  	int			dry_run,
>  	int			force_overwrite)
>  {
> -	/* set configured sector sizes in preparation for checks */
> -	if (!cli->sectorsize) {
> -		cfg->sectorsize = dft->sectorsize;
> -	} else {
> -		cfg->sectorsize = cli->sectorsize;
> -	}
> -	cfg->sectorlog = libxfs_highbit32(cfg->sectorsize);
> -
>  	/*
>  	 * Before anything else, verify that we are correctly operating on
>  	 * files or block devices and set the control parameters correctly.
> @@ -1730,6 +1722,7 @@ validate_sectorsize(
>  	memset(ft, 0, sizeof(*ft));
>  	get_topology(cli->xi, ft, force_overwrite);
>  
	
>  	if (!cli->sectorsize) {
>  		/*
>  		 * Unless specified manually on the command line use the
> @@ -1741,9 +1734,8 @@ validate_sectorsize(
>  		 * Set the topology sectors if they were not probed to the
>  		 * minimum supported sector size.
>  		 */
> -
>  		if (!ft->lsectorsize)
> -			ft->lsectorsize = XFS_MIN_SECTORSIZE;
> +			ft->lsectorsize = dft->sectorsize;
>  
>  		/* Older kernels may not have physical/logical distinction */
>  		if (!ft->psectorsize)
> @@ -1759,9 +1751,10 @@ _("specified blocksize %d is less than device physical sector size %d\n"
>  				ft->lsectorsize);
>  			cfg->sectorsize = ft->lsectorsize;
>  		}
> +	} else
> +		cfg->sectorsize = cli->sectorsize;
>  
> -		cfg->sectorlog = libxfs_highbit32(cfg->sectorsize);
> -	}
> +	cfg->sectorlog = libxfs_highbit32(cfg->sectorsize);
>  
>  	/* validate specified/probed sector size */
>  	if (cfg->sectorsize < XFS_MIN_SECTORSIZE ||
>