Re: [PATCH 20/21] xfsprogs: Add parent pointers during protofile creation

[Allison Henderson <allison.henderson@oracle.com>]

On 05/08/2018 02:17 PM, Eric Sandeen wrote:
> 
> 
> On 5/8/18 4:14 PM, Allison Henderson wrote:
>>
>>
>> On 05/08/2018 01:39 PM, Eric Sandeen wrote:
>>>
>>>
>>> On 5/8/18 12:43 PM, Darrick J. Wong wrote:
>>>> On Mon, May 07, 2018 at 09:41:18PM -0700, Allison Henderson wrote:
>>>>> Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
>>>>> ---
>>>>>    mkfs/proto.c         | 59 ++++++++++++++++++++++++++++++++++------------------
>>>>>    repair/attr_repair.c | 16 ++------------
>>>>>    repair/phase6.c      | 47 +++++++++++++++++++++++++----------------
>>>
>>>> Separate patches for separate utilities, please.
>>>
>>> Or at least patch per functional change; it's not at all clear to me how protofile
>>> creation has anything to do with xfs_repair, and there's nothing in the commit
>>> log to help me figure it out.  :)
>>>
>>> (I think it doesn't have anything to do w/ protofile creation, and the repair
>>> patches are fixing up orphanage entry creation; the attr_repair changes seem
>>> like some third thing that I don't grok yet, sorry - removing namecheck() for
>>> some reason?).
>>>
>>> -Eric
>>
>> Ok, it took me a minute to remember why I did this too.  We had ended up adding a parameter to libxfs_dir_createname to get the offset needed for creating the parent pointer in the protofile, and I ended up following the compiler errors back around the repair code that uses the same function calls as well.
> 
> Except that this patch isn't fixing the parameter count, it's changing it from NULL, right?
> 
> @@ -1170,7 +1181,7 @@ mv_orphanage(
>   
>   			libxfs_defer_init(&dfops, &first);
>   			err = -libxfs_dir_createname(tp, orphanage_ip, &xname,
> -						ino, &first, &dfops, nres, NULL);
> +						ino, &first, &dfops, nres, &offset);
> 
> 
> ...I wonder if there's much hope for doing this series in a bisectable way.  :)
> 
>> Maybe I should make a separate patch just for the offset like the kernel space set does, and then have two smaller patches for the protofile and repair code.
> 
> That might be good.  Any hint about what the attr_repair.c changes are?
> 
> Thanks,
> -Eric
> 
Oh, you're right.  I probably just tracked it down then.  Reasoning that 
anything using it would likely need the pptr update to go along with it. 
It can still be a separate patch though.

The changes in repair/attr_repair.c are because the parent pointer names 
are not strings, they are the binary {ino, gen, diroffset}.  So it 
doesnt make sense to run a name check on them.  I'll try to get 
something added to only skip the check for pptrs though.

Allison