Re: [PATCH v6 3/4] mkfs: add -P flag to populate a filesystem from a directory

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Apr 23, 2025 at 06:03:18PM +0200, Luca Di Maio wrote:
> Add a `-P` flag to populate a newly created filesystem from a directory.
> This flag will be mutually exclusive with the `-p` prototype flag.
> 
> Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
> ---
>  mkfs/xfs_mkfs.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 3f4455d..57dbeba 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -14,6 +14,7 @@
>  #include "libfrog/dahashselftest.h"
>  #include "libfrog/fsproperties.h"
>  #include "proto.h"
> +#include "populate.h"
>  #include <ini.h>
> 
>  #define TERABYTES(count, blog)	((uint64_t)(count) << (40 - (blog)))
> @@ -1022,6 +1023,7 @@ struct cli_params {
> 
>  	char	*cfgfile;
>  	char	*protofile;
> +	char	*directory;
> 
>  	enum fsprop_autofsck autofsck;
> 
> @@ -1170,6 +1172,7 @@ usage( void )
>  /* naming */		[-n size=num,version=2|ci,ftype=0|1,parent=0|1]]\n\
>  /* no-op info only */	[-N]\n\
>  /* prototype file */	[-p fname]\n\
> +/* populate */		[-P directory]\n\
>  /* quiet */		[-q]\n\
>  /* realtime subvol */	[-r extsize=num,size=num,rtdev=xxx,rgcount=n,rgsize=n,\n\
>  			    concurrency=num]\n\
> @@ -5254,7 +5257,7 @@ main(
>  	memcpy(&cli.sb_feat, &dft.sb_feat, sizeof(cli.sb_feat));
>  	memcpy(&cli.fsx, &dft.fsx, sizeof(cli.fsx));
> 
> -	while ((c = getopt_long(argc, argv, "b:c:d:i:l:L:m:n:KNp:qr:s:CfV",
> +	while ((c = getopt_long(argc, argv, "b:c:d:i:l:L:P:m:n:KNp:qr:s:CfV",
>  					long_options, &option_index)) != EOF) {
>  		switch (c) {
>  		case 0:
> @@ -5280,6 +5283,9 @@ main(
>  				illegal(optarg, "L");
>  			cfg.label = optarg;
>  			break;
> +		case 'P':
> +			cli.directory = optarg;
> +			break;

Uh... why not modify setup_proto to check the mode of the opened fd, and
call populate_from_dir if it's a directory?  Then you don't need all the
extra option parsing code.  It's not as if -p <path> has ever worked on
a directory.

--D

>  		case 'N':
>  			dry_run = 1;
>  			break;
> @@ -5478,9 +5484,20 @@ main(
>  		initialise_ag_freespace(mp, agno, worst_freelist);
> 
>  	/*
> -	 * Allocate the root inode and anything else in the proto file.
> +	 * Allocate the root inode and anything else in the proto file or source
> +	 * directory.
> +	 * Both functions will allocate the root inode, so we use them mutually.
>  	 */
> -	parse_proto(mp, &cli.fsx, &protostring, cli.proto_slashes_are_spaces);
> +	if (cli.protofile && cli.directory) {
> +		fprintf(stderr,
> +			_("%s: error: specifying both -P and -p is not supported\n"),
> +			progname);
> +		exit(1);
> +	}
> +	if (!cli.directory)
> +		parse_proto(mp, &cli.fsx, &protostring, cli.proto_slashes_are_spaces);
> +	else
> +		populate_from_dir(mp, NULL, &cli.fsx, cli.directory);
> 
>  	/*
>  	 * Protect ourselves against possible stupidity
> --
> 2.49.0
>