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

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

On Fri, Sep 26, 2025 at 11:08:44PM +0800, Zorro Lang wrote:
> On Fri, Sep 26, 2025 at 08:12:49AM -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
> >  # 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>
> > ---
> 
> 
> 
> > 
> > v2:
> > - Add comment to document wonky losetup behavior.
> > v1: https://lore.kernel.org/fstests/20250924181235.152502-1-bfoster@redhat.com/
> > 
> >  common/rc | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/common/rc b/common/rc
> > index 81587dad..1ec84263 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -4596,6 +4596,9 @@ _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
> > +		# losetup can return error for certain configuration settings
> > +		# but still create a loop device.
> > +		test -n "$dev" && losetup -d "$dev" > /dev/null 2>&1
> 
> I'm not sure if "-b" is better than "-n", anyway "-n" makes sense too.

I doubt it makes much difference, as long as nobody races us to delete
and recreate the same loopdev.

Thanks for adding the comments,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D


> Reviewed-by: Zorro Lang <zlang@redhat.com>
> 
> >  		dev="$(losetup $args -f --show $file)" || \
> >  			_fail "Cannot assign $file to a loop device ($args)"
> >  	fi
> > -- 
> > 2.51.0
> > 
> > 
>