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

[Martin Wilck]

On Wed, 2022-03-02 at 12:38 -0600, Benjamin Marzinski wrote:
> 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.

I guess that would do no harm. But parted, too, is agnostic of how the
loop device was created, so I wouldn't call it a bug that parted
currently doesn't set this flag. Arguably, the flag should be set when
the device is created, using "losetup -P". I find it strange that
Ritika calls that a "workaround". IMO it's the one and only correct way
to set up the loop device.

> 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.

I agree. kpartx is a tool for creating linear dm mappings that behave
roughly like partitions. And it should stay that way. We (made the
mistake to) add convenience functionality to setup loop devices when
kpartx is called with a regular file argument. That doesn't mean that
kpartx is a generic tool for handling loop devices or partition
devices. We should stick to the "do one thing, do it right" philosophy
here.

TBH, he usage "kpartx -av /some/file" just to create the loop device,
if the file has no partition table, looks like an abuse to me. I
wouldn't recommend to rely on that behavior. It might change in the
future.

Regards,
Martin