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

["Darrick J. Wong" <darrick.wong@oracle.com>]

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>

> ---
>  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"

>  	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?

> +		},
> +
>  	},
>  };
>  
> @@ -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

>  
>  	/*
>  	 * 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  http://vger.kernel.org/majordomo-info.html