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 _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
