Re: bcache-tools package for Fedora / status probe-bcache

Re: bcache-tools package for Fedora / status probe-bcache

[Gabriel de Perthuis]

>> I'm not a fan of a blkid csum check (I pointed it out on the 
>> bug[1]). If a superblock gets scribbled or corrupted, you want 
>> bcache to complain, and you don't want blkid to look for the next 
>> possible signature.
> 
> Having blkid also verify the csum was requested by Karel Zak, the 
> maintainer of util-linux. As a packager of bcache-tools I'm in favour
> of having blkid identify bcache, but I don't have a preference on
> using csum to identify bcache. I can pass the message to Karel, but
> it would be better if we both discuss it on the appropriate 
> (util-linux?) mail list.

Karel, are you okay if blkid doesn't do the csum verification discussed above?
Checksum failures will be reported by the kernel instead.
Alternatively, do you see a way libblkid can return good magic / bad checksum
results?

>>> So now I'm wondering: are there any particular reasons to keep 
>>> probe-bcache part of the package, or will it really be obsolete?
>> 
>> If you address the above and tweak the udev rules, why not.
>> 
>> The upstream repo will need to keep probe-bcache for a while 
>> longer, because we don't have a way to require a sufficiently 
>> recent libblkid.
> 
> I agree, f20 is a specific case, but in general probe-bcache will be 
> needed for a while.

For the record, the libblkid patch is a good thing in the long run:
common interface, less forks in udev.

>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1001120#c9

Re: bcache-tools package for Fedora / status probe-bcache

[Karel Zak]

On Thu, Sep 05, 2013 at 12:53:13PM +0200, Gabriel de Perthuis wrote:
> >> I'm not a fan of a blkid csum check (I pointed it out on the 
> >> bug[1]). If a superblock gets scribbled or corrupted, you want 
> >> bcache to complain, and you don't want blkid to look for the next 
> >> possible signature.
> > 
> > Having blkid also verify the csum was requested by Karel Zak, the 
> > maintainer of util-linux. As a packager of bcache-tools I'm in favour
> > of having blkid identify bcache, but I don't have a preference on
> > using csum to identify bcache. I can pass the message to Karel, but
> > it would be better if we both discuss it on the appropriate 
> > (util-linux?) mail list.
> 
> Karel, are you okay if blkid doesn't do the csum verification discussed above?

 I don't insist on csum, but I'd like to have something more robust
 than check for a magic string only. It's usually better if there
 is some additional thing (for example within superblock offset,
 csum, etc.) -- checksums are ideal because it usually verifies 
 whole superblock (header). 
 
> Checksum failures will be reported by the kernel instead.

 I don't care about kernel :-) The important is what userspace (udev)
 thinks about the device -- is it correct to trigger any action on
 broken bcache device or the device should be ignored by userspace
 rules?

> Alternatively, do you see a way libblkid can return good magic / bad checksum
> results?

 If I good understand your patches then it makes wipefs(8) more
 "hungry" to zap incomplete superblock. I have no problem to support
 this scenario. 
 
 Something else (like report bad checksums to udev) is probably
 unnecessary. Right?

> > I agree, f20 is a specific case, but in general probe-bcache will be 
> > needed for a while.
> 
> For the record, the libblkid patch is a good thing in the long run:
> common interface, less forks in udev.

 Yes, definitely. 
 
 Maybe we can backport the patch to F20 if you need it -- it's not too
 invasive change. 

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: bcache-tools package for Fedora / status probe-bcache

[Gabriel de Perthuis]

lun. 09 sept. 2013 15:26:53 CEST, Karel Zak a écrit :
> On Thu, Sep 05, 2013 at 12:53:13PM +0200, Gabriel de Perthuis wrote:
>>>> I'm not a fan of a blkid csum check (I pointed it out on the 
>>>> bug[1]). If a superblock gets scribbled or corrupted, you want 
>>>> bcache to complain, and you don't want blkid to look for the next 
>>>> possible signature.
>>>
>>> Having blkid also verify the csum was requested by Karel Zak, the 
>>> maintainer of util-linux. As a packager of bcache-tools I'm in favour
>>> of having blkid identify bcache, but I don't have a preference on
>>> using csum to identify bcache. I can pass the message to Karel, but
>>> it would be better if we both discuss it on the appropriate 
>>> (util-linux?) mail list.
>>
>> Karel, are you okay if blkid doesn't do the csum verification discussed above?
>
>  I don't insist on csum, but I'd like to have something more robust
>  than check for a magic string only. It's usually better if there
>  is some additional thing (for example within superblock offset,
>  csum, etc.) -- checksums are ideal because it usually verifies 
>  whole superblock (header). 
>  
>> Checksum failures will be reported by the kernel instead.
>
>  I don't care about kernel :-) The important is what userspace (udev)
>  thinks about the device -- is it correct to trigger any action on
>  broken bcache device or the device should be ignored by userspace
>  rules?

It's correct insofar as current consumers expect it, and it's better
for error reporting.  The patches took care of my other objections,
so you can keep the crc check and go with what's on
https://github.com/g2p/util-linux/commits
(the csum patches I sent + Rolf's patch + a patch that depends on both).

>> Alternatively, do you see a way libblkid can return good magic / bad checksum
>> results?
>
>  If I good understand your patches then it makes wipefs(8) more
>  "hungry" to zap incomplete superblock. I have no problem to support
>  this scenario.

The second patch does that, and the first one also prevents exposing
nested data when a csum is broken, which was a dangerous failure mode.

>  Something else (like report bad checksums to udev) is probably
>  unnecessary. Right?

It would be good to have, because silent boot failures suck.
Maybe as a tweak to udev's embedded blkid?

>>> I agree, f20 is a specific case, but in general probe-bcache will be 
>>> needed for a while.
>>
>> For the record, the libblkid patch is a good thing in the long run:
>> common interface, less forks in udev.
>
>  Yes, definitely. 
>  
>  Maybe we can backport the patch to F20 if you need it -- it's not too
>  invasive change. 

(Letting Rolf field this one)

Re: bcache-tools package for Fedora / status probe-bcache

[Rolf Fokkens]

Op 09-09-13 17:28 schreef Gabriel de Perthuis <g2p.code@gmail.com>:

>> Maybe we can backport the patch to F20 if you need it -- it's not too
>>  invasive change.
>
>(Letting Rolf field this one)

Most important for me is to have bcache identification by blkid in before
F20, so the udev rules file doesn't have to call bcache-probe. Whether or
not it uses checksums is not the most important thing, and that aspect
might even change later.

Re: bcache-tools package for Fedora / status probe-bcache

[Karel Zak]

On Mon, Sep 09, 2013 at 05:28:57PM +0200, Gabriel de Perthuis wrote:
> It's correct insofar as current consumers expect it, and it's better
> for error reporting.  The patches took care of my other objections,
> so you can keep the crc check and go with what's on
> https://github.com/g2p/util-linux/commits
> (the csum patches I sent + Rolf's patch + a patch that depends on both).

Merged with some changes

 - not added blkid_probe_is_badcsum() the info about bad csum is
   exported by SBBADCSUM=1 variable, you have to use for example:

       if (blkid_probe_has_value(pr, "SBBADCSUM"))....

 - the usage type is BLKID_USAGE_OTHER rather than "raid" (we have some
   special exceptions and tests for raids that are unnecessary for
   bcache)

I have also added the test images from Rolf to util-linux test suite.

 $ blkid -o udev -p  tests/output/blkid/images-fs/bcache-C.img 
 ID_FS_UUID=7a343627-ac87-4bf0-b76f-46067cbc9b8c
 ID_FS_UUID_ENC=7a343627-ac87-4bf0-b76f-46067cbc9b8c
 ID_FS_TYPE=bcache
 ID_FS_USAGE=other

Thanks!

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com