Re: [PATCH 18/21] xfsprogs: Add parent pointer flag to cmd

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

On 05/08/2018 10:25 AM, Darrick J. Wong wrote:
> On Mon, May 07, 2018 at 09:41:16PM -0700, Allison Henderson wrote:
>> Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
> Needs commit message:
>
> mkfs: enable formatting with parent pointers
>
> Wire up parent pointer support in mkfs via the '-m parent' parameter.
>
> Signed-off-by: <etc>
Ok, will update
>> ---
>>   mkfs/xfs_mkfs.c | 16 +++++++++++++++-
>>   1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>> index 78d0ce5..554e1bf 100644
>> --- a/mkfs/xfs_mkfs.c
>> +++ b/mkfs/xfs_mkfs.c
>> @@ -129,6 +129,7 @@ enum {
>>   	M_UUID,
>>   	M_RMAPBT,
>>   	M_REFLINK,
>> +	M_PARENT,
> Hmm, is this better off in the naming section?
>
> e.g. "mkfs.xfs -n parent=1"
I suppose it could fit there too if that's preferred.  I wasnt really 
sure which section it fit best to :-)
>>   	M_MAX_OPTS,
>>   };
>>   
>> @@ -663,6 +664,7 @@ struct opt_params mopts = {
>>   		[M_UUID] = "uuid",
>>   		[M_RMAPBT] = "rmapbt",
>>   		[M_REFLINK] = "reflink",
>> +		[M_PARENT] = "parent",
>>   	},
>>   	.subopt_params = {
>>   		{ .index = M_CRC,
>> @@ -693,6 +695,13 @@ struct opt_params mopts = {
>>   		  .maxval = 1,
>>   		  .defaultval = 1,
>>   		},
>> +		{ .index = M_PARENT,
>> +		  .conflicts = { {NULL, LAST_CONFLICT } },
>> +		  .minval = 0,
>> +		  .maxval = 1,
>> +		  .defaultval = 0,
> This (misleadingly named) field is the default value if you pass the
> argument without explicitly assigning a value, i.e.
>
> # mkfs.xfs -m parent /dev/sda
>
> sets parent to whatever defaultval is.  In this case you'd get no parent
> pointers, which is a little surprising.
>
> And, uh, seeing as people keep getting this wrong maybe we should rename
> it?
Oh I see.  That is a little odd, is there really a better name though?  
Maybe just a comment or something might make it more clear.
>> +		},
>> +
>>   	},
>>   };
>>   
>> @@ -865,7 +874,7 @@ usage( void )
>>   {
>>   	fprintf(stderr, _("Usage: %s\n\
>>   /* blocksize */		[-b size=num]\n\
>> -/* metadata */		[-m crc=0|1,finobt=0|1,uuid=xxx,rmapbt=0|1,reflink=0|1]\n\
>> +/* metadata */		[-m crc=0|1,finobt=0|1,uuid=xxx,rmapbt=0|1,reflink=0|1,parent=0|1]\n\
>>   /* data subvol */	[-d agcount=n,agsize=n,file,name=xxx,size=num,\n\
>>   			    (sunit=value,swidth=value|su=num,sw=num|noalign),\n\
>>   			    sectsize=num\n\
>> @@ -1586,6 +1595,9 @@ meta_opts_parser(
>>   	case M_REFLINK:
>>   		cli->sb_feat.reflink = getnum(value, opts, subopt);
>>   		break;
>> +	case M_PARENT:
>> +		cli->sb_feat.parent_pointers = getnum(value, &mopts, M_PARENT);
>> +		break;
>>   	default:
>>   		return -EINVAL;
>>   	}
>> @@ -2887,6 +2899,8 @@ sb_set_features(
>>   		sbp->sb_features_ro_compat |= XFS_SB_FEAT_RO_COMPAT_RMAPBT;
>>   	if (fp->reflink)
>>   		sbp->sb_features_ro_compat |= XFS_SB_FEAT_RO_COMPAT_REFLINK;
>> +	if (fp->parent_pointers)
>> +		sbp->sb_features_ro_compat |= XFS_SB_FEAT_RO_COMPAT_PARENT;
> Otherwise looks ok to me....
>
> --D
Alrighty, thx!
>>   
>>   	/*
>>   	 * Sparse inode chunk support has two main inode alignment requirements.
>> -- 
>> 2.7.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=DwIBAg&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=LHZQ8fHvy6wDKXGTWcm97burZH5sQKHRDMaY1UthQxc&m=2DFx03tmCKRPVndQLwf1P1u0PChkNowVFyXqzZ_arE8&s=JiwNEPlNO54aLxRtPd9mGL_lKU8dpX4T6ECMmDQhc70&e=
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=DwIBAg&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=LHZQ8fHvy6wDKXGTWcm97burZH5sQKHRDMaY1UthQxc&m=2DFx03tmCKRPVndQLwf1P1u0PChkNowVFyXqzZ_arE8&s=JiwNEPlNO54aLxRtPd9mGL_lKU8dpX4T6ECMmDQhc70&e=