On 8/4/14, 1:51 PM, Zach Brown wrote: > On Mon, Aug 04, 2014 at 01:42:23PM -0500, Eric Sandeen wrote: >> On 8/4/14, 1:35 PM, Zach Brown wrote: >>> On Fri, Aug 01, 2014 at 06:12:37PM -0500, Eric Sandeen wrote: >>>> Reading the quota tree root may fail with ENOENT >>>> if there is no quota, which is fine, but the code was >>>> ignoring every other error as well, which is not fine. >>> >>> Kinda makes you want to write a test that would have caught this. >>> >>> Kinda. >> >> /me looks at ground, shuffles feet ... >> >>> Also, if you're still keen to iterate on this series, it looks like this >>> pattern is copied and pasted a few times in open_ctree(). With >>> temporary root pointers for each block, for some reason. A little >>> helper function could take a bite out of open_ctree(). >> >> Hm, the uuid tree is roughly similar, but not exactly. I think those >> are the only 2 "optional" roots (uuid because it'll get regenerated). >> >> I'm guessing the temporary root pointer is so we don't ever assign a >> PTR_ERR to the root in fs_info? > > It took me a while to see what you meant. > > Yeah, using a temporary root makes sense. Using a different one for > each block makes less sense. >
<snip> > - z > Yeah, fair enough, I thought about that after I hit send ;) I could send a V2 of patch 11/12 to do that w/o needing to redo the series too much. :) I'll see if there are any other comments. Thanks! -Eric -- 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
