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

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

On Thu, Apr 24, 2025 at 02:01:06PM +0200, Luca Di Maio wrote:
> On Wed, Apr 23, 2025 at 01:09:14PM -0700, Darrick J. Wong wrote:
> > On Wed, Apr 23, 2025 at 06:03:18PM +0200, Luca Di Maio wrote:
> > > -	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
> >
> 
> Alright, that makes things easier yes, I'll just make sure to clearly
> explain the difference in the man page and help so it's clearer,
> something like:
> 
> ```
> -p prototype_options
> Section Name: [proto]
> 	These options specify the prototype parameters for populating the
> 	filesystem.  The  valid  prototype_options are:
> 
> 		[file=]
> 
> 		The  file= prefix is not required for this CLI argument for
> 		legacy reasons.  If specified as a config file directive,
> 		the prefix is required.
> 
> 		[file=]directory
> 
> 		If the optional prototype argument is given,
> 		and it's a directory, mkfs.xfs will copy the contents
> 		of the given directory or tarball into the root
> 		directory of the file system.
> 
> 		[file=]protofile
> 
> 		If  the  optional prototype argument is given,
> 		and it's a file, [ ... continue with existing man ... ]
> ```
> 
> So it's clear from the docs that the option is there and it's the same
> flag.

Sounds good to me!

--D

> L.
>