On 2017年11月08日 15:59, Nikolay Borisov wrote: > > > On 8.11.2017 02:54, Qu Wenruo wrote: >> Add checker for dir item, for key types DIR_ITEM, DIR_INDEX and >> XATTR_ITEM. >> >> This checker does comprehensive check for: >> 1) dir_item header and its data size >> Against item boundary and maximum name/xattr length. >> This part is mostly the same as old verify_dir_item(). >> >> 2) dir_type >> Against maximum file types, and against key type. >> Since XATTR key should only have FT_XATTR dir item, and normal dir >> item type should not have XATTR key. >> >> The check between key->type and dir_type is newly introduced by this >> patch. >> >> 3) name hash >> For XATTR and DIR_ITEM key, key->offset is name hash (crc32). >> Check the hash of name against key to ensure it's correct. >> >> The name hash check is only found in btrfs-progs before this patch. >> >> Signed-off-by: Qu Wenruo <w...@suse.com> >> --- >> fs/btrfs/tree-checker.c | 141 >> ++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 141 insertions(+) >> > > > I'm gonna 'hijack' this patch to discuss something - we do return the > correct EUCLEAN status from the leaf checker, however the only place > where it's called is in btree_readpage_end_io_hook and if the check > fails we only return -EIO. I wonder whether want to propagate the > EUCLEAN from there, any thoughts?
In this particular case, I think returning -EUCLEAN is *marginally* better. Much more detailed info from block layer/csum verifier/tree-checker makes more sense than just a single return value. Returning any error to make the reading fail, and cause a graceful error is good enough. The returning values between EUCLEAN and EIO makes difference, but less meaningful than specific output from tree-checker. On the other hand, if in case where return value is the only meaningful output to user-space (that's to say no much meaningful extra info like kernel message), then return value makes sense. But even in that case, the return value can't give much detailed info. Here are several common errno: EIO ENOSPC EUCLEAN ENOMEM ENOENT We can only express at most less than 10 error types. There are tons of places where thing can go wrong. What we can do is just to try our best to classify these errors, and hopes it makes sense for end user. While more meaningful error/warning message will make both developer and end user happier. Thanks, Qu > > > -- > 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 >
signature.asc
Description: OpenPGP digital signature
