[dm-devel] [PATCH v2] dm-integrity: if we have discard support, use it when recalculating

[Milan Broz]

On 05/05/2021 23:47, Melvin Vermeeren wrote:
> Hi,
> 
> On Wednesday, 5 May 2021 22:45:09 CEST Mikulas Patocka wrote:
>> So, we can ask Milan to update the manpage.
> 
> Yes, that would be fine. However, "integrity recalculate" sounds like 
> recalculating integrity. The newly implemented logic is more of a "integrity 
> wipe" or "integrity reset".
> 
> What is problematic is that actual functionality from end user point of view 
> is now completely different depending on if you use --allow-discards or not. 
> Without discard you recalculate meta, with discard you reset/wipe meta.
> 
>> It will receive integrity protection for the newly written data.
>>
>> If you create an integrity device and make a filesystem on it, the newly
>> written data matters. The old data that were on the filesystem before
>> formatting it don't care and don't need to be protected.

This is not true. Imagine blkid that tries to read various superblocks on disk
and do decisions based on it (UUID links etc). These can be in "unused" sectors.

In the normal situation dm-integrity stops the read, because checksum is wrong.
Now it seems that it returns these random data. This is not integrity protection at all.

Maybe I am missing something, but I would say this is a wrong behavior and it should
be reverted. I think this should be not supported even with additional switch,
to me is it really against the principle of sector-level integrity protection.

Also, I added wiping to integritysetup because some tools (like mkfs) actually
needef to read unitialized data before creating metadata - so without previous wipe it failed.
(These are bugs in the tools, obviously: I plan to report these  unfortunately it never happened.
Dunno if it is still the case.)

Sorry for the late reply.

Milan


> 
> One of the current possible use cases with --no-wipe --data-device is that you 
> can use existing device holding data that has no integrity and add integrity 
> to it with detached metadata device in combination with recalculate.
> 
> Then recalculation can be used in a fashion similar to trust-on-first-use for 
> this specific disk without rewriting the data meaning also no temporary copy 
> is needed. This feature is something I have used a few times as adding 
> integrity in-place can be useful in certain situations especially when dealing 
> with large amounts of data.
> 
> 
> I am not against the new reset/wipe operation, it is certainly a useful thing 
> to have. This style of initialising metadata would be especially useful with 
> formatting devices supporting discard, as it could be used to avoid 
> unnecessary writes on main data by initialising metadata only (and perhaps 
> also issue discards to underlying device).
> 
> But I do think this should be a separate, new function in addition to existing 
> recalculation feature, to me they both seem useful in different use cases.
> 
> Thoughts on this?
> 
> Thanks,
>