[RFC PATCH] loop: LOOP_CONFIGURE: send uevents for partitions

[Alyssa Ross]

LOOP_CONFIGURE is, as I understand it, supposed to be a way to combine
LOOP_SET_FD and LOOP_SET_STATUS64 into a single syscall.  When using
LOOP_SET_FD+LOOP_SET_STATUS64, a single uevent would be sent for each
partition found on the loop device during LOOP_SET_STATUS64, but when
using LOOP_CONFIGURE, no such uevent was being sent.

In the old setup, uevents are disabled for LOOP_SET_FD, but not for
LOOP_SET_STATUS64.  This makes sense, as it prevents uevents being
sent for a partially configured device during LOOP_SET_FD — they're
only sent at the end of LOOP_SET_STATUS64.  But for LOOP_CONFIGURE,
uevents were disabled for the entire operation, so that final
notification was never issued.  To fix this, I've moved the
loop_reread_partitions() call, which causes the uevents to be issued,
to after uevents are re-enabled, matching the behaviour of the
LOOP_SET_FD+LOOP_SET_STATUS64 combination.

I noticed this because Busybox's losetup program recently changed from
using LOOP_SET_FD+LOOP_SET_STATUS64 to LOOP_CONFIGURE, and this broke
my setup, for which I want a notification from the kernel any time a
new partition becomes available.

Signed-off-by: Alyssa Ross <hi@xxxxxxxxx>
Fixes: 3448914e8cc5 ("loop: Add LOOP_CONFIGURE ioctl")
---

I've marked this as an RFC because there's still a problem with this
patch that I'd be grateful for advice on how to solve: this change
accidentally makes LOOP_SET_FD emit uevents as well if max_part is
configured.  There are a few ways I could imagine resolving this:

 - Have loop_configure distinguish between LOOP_SET_FD and
   LOOP_CONFIGURE somehow.

 - Have loop_configure take a bool argument specifying whether uevents
   should be reenabled before or after the loop_reread_partitions()
   call.

 - Move re-enabling the uevent and calling loop_reread_partitions()
   out of loop_configure().

All of these have drawbacks for the understandability of the code
though, so I wanted to ask what the best way to proceed would be.

 drivers/block/loop.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 5f04235e4ff7..d8063dbf5ec1 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1110,15 +1110,19 @@ static int loop_configure(struct loop_device *lo, fmode_t mode,
 		clear_bit(GD_SUPPRESS_PART_SCAN, &lo->lo_disk->state);
 
 	loop_global_unlock(lo, is_loop);
-	if (partscan)
-		loop_reread_partitions(lo);
 	if (!(mode & FMODE_EXCL))
 		bd_abort_claiming(bdev, loop_configure);
 
+	/*
+	 * Now that we are done, reread the partitions with uevent
+	 * re-enabled to let userspace know about the changes.
+	 */
+	dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0);
+	if (partscan)
+		loop_reread_partitions(lo);
+
 	error = 0;
 done:
-	/* enable and uncork uevent now that we are done */
-	dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0);
 	return error;
 
 out_unlock:
@@ -1130,6 +1134,7 @@ static int loop_configure(struct loop_device *lo, fmode_t mode,
 	fput(file);
 	/* This is safe: open() is still holding a reference. */
 	module_put(THIS_MODULE);
+	dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0);
 	goto done;
 }
 
-- 
2.37.1