Re: [PATCH] common/rc: destroy loop dev before fallback recreation

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

On Wed, Sep 24, 2025 at 02:12:35PM -0400, Brian Foster wrote:
> When running fstests on an s390x box I observed failure to unmount
> filesystem errors due to stale loop devices being left around. This
> root caused down to generic/361 leaving around an attached loop0
> device. On further inspection, the test actually created two loop
> devices (loop0 and loop1), and executed on and cleaned up the
> latter.
> 
> The origin of the former appears to be that the initial losetup
> command in _create_loop_device() fails due to $dio_args in this
> environment, but still creates the loop device. For example:
> 
>  # losetup --direct-io=on -f --show /mnt/scratch/fs.img
>  /dev/loop0
>  losetup: /dev/loop0: set direct io failed: Invalid argument

Egad following the argument parsing in losetup is awful.  I had thought
that the -f would set act == ACT_FIND_FREE which would then set up the
loop device with one configure call, but the error message clearly
indicates that we're failing here:

	case A_SET_DIRECT_IO:
		res = loopcxt_ioctl_dio(&lc, use_dio);
		if (res)
			warn(_("%s: set direct io failed"),
			        loopcxt_get_device(&lc));
		break;

In this case, we clearly don't tear down the loop device after this
failure, so yes, you've found a bug.  losetup can totally create a loop
device, fail to configure it, and return EXIT_FAILURE without tearing
down that loop device.

>  # losetup -a
>  /dev/loop0: [64771]:131 (/mnt/scratch/fs.img)
> 
> The helper then goes on to create loop1, but it or the test never
> deals with loop0. To avoid this problem, detach any old loop device
> if one was set up before the fallback losetup command.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
> 
> This appears to be fallout from recent commit aa14b84a8d1a2 ("xfs/259:
> try to force loop device block size"). I'm not really sure why losetup
> creates the device with bad dio settings but not with block size. Maybe
> it's more of a dynamic setting or whatever and that's why this was
> previously a separate losetup command..? Anyways, this seems to work for
> me..

It probably has to do with the underlying fs not supporting directio or
something.  What fstype is /mnt/scratch, and which kernel version?

> Brian
> 
>  common/rc | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/common/rc b/common/rc
> index 81587dad..891f6b7e 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -4596,6 +4596,7 @@ _create_loop_device()
>  	# size to the directio alignment of the underlying fs, so if we want to
>  	# use our own sector size, we need to specify that at creation time.
>  	if ! dev="$(losetup $dio_args $args -f --show $file 2>/dev/null)"; then
> +		test -n "$dev" && losetup -d "$dev" > /dev/null 2>&1

The logic looks sound, but I think there ought to be a comment
explicitly documenting this behavior of losetup:

		# losetup can create a loop device, fail to configure
		# it, and return EXIT_FAILURE without tearing down that
		# loop device.
		test -n "$dev" && losetup -d "$dev" &>/dev/null

Because I won't remember this subtlety 3 months from now. :(

--D

>  		dev="$(losetup $args -f --show $file)" || \
>  			_fail "Cannot assign $file to a loop device ($args)"
>  	fi
> -- 
> 2.51.0
> 
>