Re: [PATCH RFC 0/2] Fix API breakage in libblkid

Dne 08. 04. 26 v 12:35 [email protected] napsal(a):
> From: Carlos Maiolino <[email protected]>
> 
> Patch d05a84b22e54 ("libblkid: check for private DM device before open")
> broke blkid_new_probe_from_filename() API.
> 
> Before the patch users were able, via the low-level API, to open and
> create blkid probes via the device's filename even from private
> device-mapper devices.
> 

Hi

So I'm not sure how it happened that Stratis kind of 'reuses'  libdm private 
logic we have embedded into util-linux - but something wrong is likely 
happening on Stratis side here.

For  libdm  & util-linux - there is basic principle - when udev should be 
scanning  a 'DM' device that is supposed to be private - in our case it's
uuid with  "LVM-"  prefix and "-anysuffix:  - such device is supposed to be 
NOT touched by any udev rule logic - it's private - so nothing should be 
'randomly' opening a device at unpredictable moments..

Somehow it seems Stratis selected to use similar notion (at least when I look 
there in  sysfs_devno_is_dm_private()  - there is check for uuid:
"stratis-1-private"


> This change broke Stratis project and xfsprogs.
> xfsprogs uses blkid_new_probe_from_filename() to gather topology
> information from the device, and the above mentioned change now prevents

If the private is not meant to be private -  just drop:

	} else if (strncmp(id, "stratis-1-private", 17) == 0) {
		rc = 1;
	}

from sysfs_devno_is_dm_private()

> it to be done on device-mapper private devices, as Stratis does by
> attempting to initialize a XFS filesystem on it.
> 
> > I don't think the last statement here is correct.
> blkid_probe_set_device() marks the probe as BLKID_FL_NOSCAN_DEV, but it
> does not error out, so, for low-level API calls, we simply ended up
> with a probe with BLKID_FL_NOSCAN_DEV set. But the call succeeded and we
> ended up with a probe to use and query device's information.

Please don't try to break original LVM logic which was the main reason why 
sysfs_devno_is_dm_private() even exists in the first place.

> As far as I understood it, the patch aimed to close a possible race
> window when issuing a DM_DEVICE_REMOVE ioctl() to the same device being
> probed by blkid_new_probe_from_filename().
>

Yes - but there is higher logic behind - private  'device' is private and 
should not be randomly opened by any tool.

> As for xfsprogs, I have a patch which 'fixes' it replacing
> blkid_new_probe_from_filename() by blkid_probe_set_device(), but this is
> just taping over the root cause, which is the API breakage.

I think main problem is that Stratis  mangled  'private' meaning in some way.

So let's fix Stratis instead of breaking correct logic in util-linux made for 
lvm2.

Regards

Zdenek