Re: [PATCH] fstests: fix min_dio_alignment logic for getting device block size

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Sep 16, 2024 at 01:03:12PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> If we failed to get the dio alignment from statx we try to get the
> device's block size using the BLKSSZGET ioctl, however we failed to
> return it because we don't check if the ioctl succeeded (returned 0).
> Furthermore in case the ioctl returned an error, we end up returning an
> undefined value since the 'logical_block_size' variable ends up not
> being initialized.
> 
> This was causing some tests to be skipped on btrfs after commit
> ee799a0cf1d4 ("replace _min_dio_alignment with calls to
> src/min_dio_alignment"), like generic/240 for example:
> 
>   $ ./check generic/240
>   FSTYP         -- btrfs
>   PLATFORM      -- Linux/x86_64 debian0 6.11.0-rc7-btrfs-next-174+ #1 SMP PREEMPT_DYNAMIC Tue Sep 10 17:11:38 WEST 2024
>   MKFS_OPTIONS  -- /dev/sdc
>   MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1
> 
>   generic/240 1s ... [not run] fs block size must be larger than the device block size.  fs block size: 4096, device block size: 4096
>   Ran: generic/240
>   Not run: generic/240
>   Passed all 1 tests
> 
> Where before that commit the test ran.
> 
> Fix this by checking that the ioctl succeeded.
> 
> Fixes: 0e5f196d0a6a ("add a new min_dio_alignment helper")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Looks good, sorry for missing that during review.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  src/min_dio_alignment.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/min_dio_alignment.c b/src/min_dio_alignment.c
> index 131f6023..5a7c7d9f 100644
> --- a/src/min_dio_alignment.c
> +++ b/src/min_dio_alignment.c
> @@ -42,7 +42,7 @@ static int min_dio_alignment(const char *mntpnt, const char *devname)
>  		if (dev_fd > 0 &&
>  		    fstat(dev_fd, &st) == 0 &&
>  		    S_ISBLK(st.st_mode) &&
> -		    ioctl(dev_fd, BLKSSZGET, &logical_block_size)) {
> +		    ioctl(dev_fd, BLKSSZGET, &logical_block_size) == 0) {
>  			return logical_block_size;
>  		}
>  	}
> -- 
> 2.43.0
> 
>