On Tue, May 29, 2018 at 3:45 AM, Qu Wenruo <w...@suse.com> wrote: > As the lzo corruption reported by James Harvey, for data extents without > checksum, neither btrfs check nor kernel scrub could detect anything > wrong. > > However if our profile supports duplication, we still have a chance to > detect such corruption, as in that case the mirrors will mismatch with > each other. > > So enhance --check-data-csum option to do such check, and this could > help us to detect such corruption in an autonomous test case. > > Signed-off-by: Qu Wenruo <w...@suse.com>
I think it's good to have check --check-data-csum do such check. I also think there should be a check option that only checks the mirrored copies, so it could be ran without taking the time to check everything that scrub looks at. (If the feature isn't also added to scrub as I argue for below, one could run this option every time they scrub.) IMO, it is important to also add this to scrub. First reason is to allow online scanning, since check is recommended not to run with --force on a quiescent/read-only fs. Second, I think users expect scrub to ensure mirrored data integrity, and this goes along with that, of course without automatic correction. I think without doing so, users who periodically run scrub would be surprised if they had data corruption, that it hadn't been being checked all along. It's recommended many places that users setup scrub to run automatically on a periodic basis, but I'm not sure if I've seen anywhere recommend setting up check to do so as well, especially as doing so on a mounted filesystem is discouraged and potentially will crash. I haven't looked at this specifically, but I'm thinking this would be better in the btrfs-progs side of scrub after it runs the kernel side real scrub. I have to head out for a while. I haven't looked over or tried the code yet, but will later. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
