On Mon, Sep 16, 2024 at 01:03:12PM +0100, fdmanana@xxxxxxxxxx wrote:
> From: Filipe Manana <fdmanana@xxxxxxxx>
>
> 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@xxxxxxxx>
Looks good, sorry for missing that during review.
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--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
>
>
