Messages in this thread Patch in this message | | | From | Alyssa Ross <> | Subject | [RFC PATCH] loop: LOOP_CONFIGURE: send uevents for partitions | Date | Tue, 21 Feb 2023 22:28:51 +0000 |
| |
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@alyssa.is> 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
| |