OK, thanks for clarifying. :-)
Jan
----- Original Message -----
> From: "Dave Chinner" <david@xxxxxxxxxxxxx>
> To: "Jan Tulak" <jtulak@xxxxxxxxxx>
> Cc: "Brian Foster" <bfoster@xxxxxxxxxx>, "Dave Chinner"
> <dchinner@xxxxxxxxxx>, xfs@xxxxxxxxxxx
> Sent: Thursday, July 9, 2015 2:45:43 AM
> Subject: Re: [PATCH 01/17] xfsprogs: use common code for multi-disk detection
>
> [Please line wrap your responses at 68-72 columns]
>
> On Wed, Jul 08, 2015 at 12:14:56PM -0400, Jan Tulak wrote:
> > ----- Original Message -----
> > > From: "Dave Chinner" <david@xxxxxxxxxxxxx>
> >
> > > At one point during development of this patch set I started writing
> > > an xfstest to validate that mkfs did all the right input validation
> > > things and set parameters appropriately so that we didn't
> > > inadvertently change behaviour. I never really finished it off (like
> > > the patch set), but I've attached it below to give an idea of where
> > > I was going with it. It was based on validating the input and CLI
> > > parameters for the new code, so is guaranteed to fail on an existing
> > > mkfs binary.
> >
> > I'm using and extending it, but I'm not sure about some tests,
> > whether it is a change from current behaviour, or if it is rather
> > an issue in the test.
>
> Remember, the point of the patchset was to sanitise and clean up the
> CLI interface, not just the code. i.e. the CLI will change, not just
> the code.
>
> > > +
> > > +# basic "should fail" options
> > > +# logarithm based options are no longer valid
> > > +do_mkfs_fail -s log=9 $SCRATCH_DEV
> >
> > There are some changes in logarithm input (mkfs: validate
> > logarithmic parameters sanely), but it is still supported in the
> > patches. Is there some issue, why to remove them?
>
> They are redundant and almost nobody uses them. The size options are
> what people use, and even they have so many different units that it
> confuses people...
>
> > Otherwise, it should rather test for (in)valid input for log=xxx, right?
>
> No, it's indicative of the fact I wanted to remove the log scale
> options for variables. As I've said before - I didn't ever finish
> the patchset off.
>
> Essentially, once all the options are in a table, we only want to
> look in one place for things like ag size, fs size, log size, block
> sizes etc. Right now the table has multiple entries or multiple
> global variables for these things and we have to work out which is
> valid and correct and handle conflicts and incompatibilities, etc.
> We don't *need* that complexity to specify sizes of things, so
> getting rid of the rarely used redundant options makes a lot of
> sense as it simplifies the code and the user interface without
> reducing functionality...
>
> > > +rm -f $fsimg
> > > +$XFS_IO_PROG -f -c "truncate $fssize" $fsimg
> > > +do_mkfs_pass -d file $fsimg
> > > +do_mkfs_pass -d file,name=$fsimg
> > > +rm -f $fsimg
> > > +do_mkfs_pass -d size=$fssize,file $fsimg
> > > +rm -f $fsimg
> > > +do_mkfs_pass -d size=$fssize,file,name=$fsimg
> > > +do_mkfs_pass -d file,name=$fsimg
> >
> > Should all these inputs really pass?
>
> Yes, they should, because ....
>
> > What is the expected
> > behaviour for example on -d file,name=$fsimg if the file exists,
> > and what if there is no such file?
>
> ....in all cases the file either:
>
> a) exists and is of non-zero size and hence defines the
> size of the filesystem to be created,
> b) does not exist but the size of the filesystem to be
> created is specified on the CLI allowing us to create
> the image file correctly.
>
> We should only fail to create the image file if it doesn't exist
> and we haven't been given enough information to calculate the size
> of the filesystem via CLI parameters.
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
>
--
Jan Tulak
jtulak@xxxxxxxxxx
|