Re: [dm-devel] [PATCH 7/8] dm: delay registering the gendisk

[Peter Rajnoha <prajnoha@redhat.com>]

On Tue 10 Aug 2021 00:31, Alasdair G Kergon wrote:
> On Wed, Aug 04, 2021 at 11:41:46AM +0200, Christoph Hellwig wrote:
> > device mapper is currently the only outlier that tries to call
> > register_disk after add_disk, leading to fairly inconsistent state
> > of these block layer data structures.  
> 
> > Note that this introduces a user visible change: the dm kobject is
> > now only visible after the initial table has been loaded.
> 
> Indeed.  We should try to document the userspace implications of this
> change in a bit more detail.  While lvm2 and any other tools that
> followed our recommendations about how to use dm should be OK, there's
> always the chance that some other less robustly-written code will need
> to make adjustments.
> 
> Currently to make a dm device, 3 ioctls are called in sequence:
> 
> 1. DM_DEV_CREATE  - triggers 'add' uevents
> 2. DM_TABLE_LOAD
> 3. DM_SUSPEND     - triggers 'change' uevent
> 
> After this patch we have:
> 
> 1. DM_DEV_CREATE  
> 2. DM_TABLE_LOAD  - triggers 'add' uevents
> 3. DM_SUSPEND     - triggers 'change' uevent
> 
> The equivalent dmsetup commands for a simple test device are
> 0. udevadm monitor --kernel --env &   # View the uevents as they happen
> 1. dmsetup create dev1 --notable
> 2. dmsetup load --table "0 1 error" dev1
> 3. dmsetup resume dev1
> 
>   => Anyone with a udev rule that relies on 'add' needs to check if they
>      need to change their code.
> 
> The udev rules that lvm2 uses to synchronise what it is doing rely
> only on the 'change' event - which is not moving.  The 'add' event
> gets ignored.  
> 
> When loading tables, our tools also always refer to devices using
> the 'major:minor' format, which isn't affected, rather than using
> pathnames in /dev which might not exist now after this change if a table
> hasn't been loaded into a referenced device yet.  Previously this was
> permissible but we always recommended against it to avoid a pointless
> pathname lookup that's subject to races and delays.
> 
> So again, any tools that followed our recommendations ought to be
> unaffected.
> 
> Here's an example of poor code that previously worked but will fail now:
>   dmsetup create dev1 --notable
>   dmsetup create dev2 --notable
>   dmsetup ls  <-- get the minor number of dev1 (say it's 1 corresponding
> to dm-1)
>   dmsetup load dev2 --table '0 1 linear /dev/dm-1 0'
>   ...
> 
> Peter - have I missed anything?

It looks this is the only area affected, but as you say, this should be
well documented (including comments in our own udev rules) so there are
no false assumptions made by other non-lvm/non-libdm users.

(I'm not counting the very corner use case of
'dmsetup --addnodeoncreate --verifyudev' which now ends up with a dev node
in /dev that logically returns -ENODEV when accessed instead of zero-sized
device as it was before.)

-- 
Peter