On Mon, Jul 16, 2018 at 09:57:43PM +0800, Qu Wenruo wrote: > >>> -EUCLEAN ? > >> > >> Either works for me. > > > > That's not just a cosmetic change, there's a semantic difference between > > the error codes, I maybe make that more explicit and not expect that this > > is obvious. > > > > ENOENT does not make much sense in this context, the caller (mount in > > this case) cannot do anything about a code that says 'some internal > > structure not found'. > > The point here is, if every self-checker should only return -EUCLEAN, it > won't really indicate what's going wrong, except points to some > self-checker (and such self-checkers are growing larger than our > expectation already). > > My practice here is, put some human readable and meaningful error > message. No matter what we choose to return, the error message should > tell us what's going wrong. > > In this case, I don't really care the return value. If it's explicitly > needed to return -EUCLEAN, I could make all existing checker (from > tree-checker to chunk/bg/dev-extent checker) to return -EUCLEAN if > anything is wrong (and save several "ret = -EUCLEAN" lines). > The return value doesn't really have much meaning nowadays, it's the > error message important now.
Ok, I see what you mean. The message is important as it's otherwise almost impossible to find where exactly the mount fails. The error messages perhaps fall into several categories: 1) transient errors, some failure that happens before the filesystem state is fully examined this is namely ENOMEM, or EINTR eg. returned by kthread_run maybe also a failure on a multi-device filesystem when the devices haven't been scanned yet 2) clearly some corruption/consistency condtion, with enough information available to decide like a missing tree, most of the tree-checker would fall into this category 3) same as the previous one, but thre's some extenal condition preventing a full check that's eg. a real EIO after reading a tree block The error code are IMO important to see how severe the problems are and what's the expected solution. 2 is for 'check', 3 may need degraded mount, 1 needs maybe more time to mount again. With the error messages in place, 2 can be completely covered by EUCLEAN. I briefly skimmed a few call paths and think that the 3 categories should be enough, but I'm also expecting some exceptions that can be decided case by case. The error codes are now not consistent, lots of EUCLEAN are historically EIO, but before we start cleaning that up we should have at least some guidelines. Please let me know what you think. -- 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
