Re: [PATCH] losetup: don't skip adding a new device if it already has a device node

[Luis Chamberlain <mcgrof@kernel.org>]

On Fri, Feb 25, 2022 at 07:09:03PM +0100, Christoph Hellwig wrote:
> Linux plans to deprecate the auto-creation of block devices based on
> access to the devic node starting from kernel 5.18.  Without that feature
> losetup will fail to create the loop device if a device node already
> exists, but the loop device to back it in the kernel does not exist yet.
> This is a scenario that should not happen in modern udev based
> distributions, but apparently there still are various scripts around that
> manually call the superflous mknod.
> 
> Change losetup to unconditionally call loopcxt_add_device when a specific
> device node is specified on the command line.  If the loop device
> already exists the LOOP_CTL_ADD ioctl will fail, but given that losetup
> ignores the return value from loopcxt_add_device that failure has no
> further effect.

I think it would help to explain what the issue is, with a simple
example on the commit log.

By default loading the loop module we'll create only 8 loopback
devices. Prior to the new CONFIG_BLOCK_LEGACY_AUTOLOAD which intends
to deprecate the whole oldschool probe functionality which used try
to load the respective block driver (loop in this case) when the
driver is not present but the nodes are created manually, the following
piece of code would work:

losetup -D
modprobe -r loop
modprobe loop

rm -f foo.img
truncate -s 10M foo.img

# Note: /dev/loop8 by default won't exist as we default to 7
# loop devices
rm -f /dev/loop8
mknod /dev/loop8 b 7 8
losetup /dev/loop8 foo.img

When deprecating this probe --> module load logic, if the
mknod is run we'd currently fail at the last step. With this
fix the last step will still work. However please note that
CONFIG_BLOCK_LEGACY_AUTOLOAD goes away the above will require
manually loading the loop module. Scripts which fail to load
the loop module prior to mknod will fail by definition of the
deprecation effort.

> Reported-by: Luis Chamberlain <mcgrof@kernel.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

With that said:

Tested-by: Luis Chamberlain <mcgrof@kernel.org>

  Luis

> ---
>  sys-utils/losetup.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sys-utils/losetup.c b/sys-utils/losetup.c
> index c400cbf12..09c028b6b 100644
> --- a/sys-utils/losetup.c
> +++ b/sys-utils/losetup.c
> @@ -522,7 +522,7 @@ static int create_loop(struct loopdev_cxt *lc,
>  		}
>  	}
>  
> -	if (hasdev && !is_loopdev(loopcxt_get_device(lc)))
> +	if (hasdev)
>  		loopcxt_add_device(lc);
>  
>  	/* losetup --noverlap /dev/loopN file.img */
> -- 
> 2.30.2
>