[dm-devel] [PATCH] kpartx: Add -P option for partition scanning

[Benjamin Marzinski]

On Wed, Mar 02, 2022 at 12:07:11AM +0000, Ritika Srivastava wrote:
> On 2/28/22, 2:44 PM, "Benjamin Marzinski" wrote:
> 
>     > So unless I'm missing something, we'd only really want this for removing
>     > a kpartx device, in the case where somehow you have /dev/loopXpY
>     > partitions without the LO_FLAGS_PARTSCAN flag set on the disk. That
> 
> That's correct. We only want this option so that once PARTSCAN flag is set, 
> Kpartx -d can delete /dev/loopXpY.
> 
>     > seems like it shouldn't happen in the first place.  Obviously, you
>     > showed that it can with parted.  But I would argue that this is a bug in
>     > parted.  If parted is creating partitions, it should set
>     > LO_FLAGS_PARTSCAN so the partition nodes get cleaned up.
>     > I suppose kpartx could check if there are partition devices for the loop
>     > device, and if so, it could set LO_FLAGS_PARTSCAN before doing the
> 
> Would removing all partition nodes (/dev/loop0pY) on kpartx -d be a better solution.?

Like I said, if we fix this in multipath, then checking for /dev/loopXpY
devnodes, and setting LO_FLAGS_PARTSCAN before deleting the loop device
if they are present seems like a better solution.

But again, you can make a pretty good case that when parted creates
those partition devices, it should set LO_FLAGS_PARTSCAN so that their
devnodes will get cleaned up.
 
>     > remove. But setting it unilaterally would just cause it to create an
>     > extra set of devices that would only serve to confuse people (and lvm).
> 
> -P should be used only when partition scan needs to be enabled - only on need basis.
> 
>     > Also, the actual partition /dev/loopXpY will always get removed. It's
>     > just the devnode that stays around, and that won't confuse lvm. This
> 
> The example below shows that /dev/loop0p1 is not removed which is confusing the LVM.

But lvm decides what devices are duplicates by reading them, not by
looking at their name.  So, unless both the /dev/loopXpY and the
/dev/mapper/loopXpY actually are devices, lvm won't have an issue.  If
the /dev/loopXpY devnode is still around, but there's no actual device
connected to it, that lvm issue shouldn't happen.

Leaving devnodes around doesn't cause lvm issues. But adding loop
partitions can cause lvm issues.  This is why I don't like the idea of
kpartx creating them.

>     > isn't that odd for loop devices. The /dev/loopX devnodes will stay
>     > around once you're done with them, regardless of whether you create the
>     > loop device will kpartx or losetup.
> 
> That's correct.
> But loop device when setup with losetup -P option does remove /dev/loopXpY on detach
> Hoping to achieve the same functionality in kpartx.
> 
> 
>     >> 
>     >> // workaround - losetup -P
>     >> # kpartx -a -v test.img                                                                             // First kpartx
>     >> # ls -l /dev/loop0*
>     >> brw-rw----. 1 root disk  7,   0 Feb 22 20:05 /dev/loop0
>     >> 
>     >> # parted -a none -s /dev/loop0 mkpart primary 64s 100000s
>     >> # parted -a none -s /dev/loop0 set 1 lvm on
>     >> # kpartx -d test.img  
>     >> # ls -l /dev/loop0*
>     >> brw-rw----. 1 root disk   7,   0 Feb 22 20:05 /dev/loop0
>     >> brw-rw----. 1 root disk 259,   0 Feb 22 20:05 /dev/loop0p1
>     >> 
>     >> # kpartx -av test.img                                                                         //Second kpartx
>     >> # ls -l /dev/mapper/loop0*
>     >> lrwxrwxrwx. 1 root root       7 Feb 22 20:53 loop0p1 -> ../dm-2
>     >> 
>     >> # pvcreate /dev/mapper/loop0p1
>     >> # pvscan
>     >>   WARNING: Not using device /dev/loop0p1 for PV <UUID>
>     >>   WARNING: PV <UUID> prefers device /dev/mapper/loop0p1 because device is in dm subsystem.
> 
>     > So this example shows exactly why I don't want both /dev/loopXpY and
>     > /dev/mapper/loopXpY. Whenever we use your -P option we can run into
>     > this situation, right?
> 
> In the above example, the first `kpartx -a` could benefit from `-P` option and would remove /dev/loop0p1 on kpartx -d.
> This is to avoid having both /dev/loop0p1 and /dev/mapper/loop0p1.

But again, that pvcreate WARNING happens when both devices exist. Simply
having a devnode present won't cause it. And adding the '-P' option
won't top both devices from existing at the same time. That being said,
I do understand the desire to clean them up.

-Ben

> The second `kpartx -a` does not need a partition scan and `-P` option should not be provided.
> However, yes, you are right - if it is specified in this case, it would create both /dev/loop0p1 and /dev/mapper/loop0p1 
> - which then would have to be deleted and recreated without the `-P` option.
> Maybe a warning message here would help - something like `-P` option should be used only while creating new partitions. 
> 
> Thanks,
> Ritika
>