Re: [PATCH 03/12] btrfs: handle errors from reading the quota tree root

[Eric Sandeen <sandeen@redhat.com>]

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