Re: [PATCH 3/3 v2] xfstests: Add support for sections in config file

[Dave Chinner <david@fromorbit.com>]

On Fri, Jun 28, 2013 at 03:32:00PM +0200, Lukas Czerner wrote:
> This patch add support for sections in the config file. Each section can
> contain configuration options in the format
> 
> OPTION=value
> 
> when one section is processed xfstests will proceed to next section
> until all secitons are processed, or an error occur.
> 
> The name of the section can consist of alphanumeric characters + '_',
> nothing else is allowed. Name of the section is also used to create
> results subdirectory for each section. After all the sections are
> processed summary of all runs is printed out.
> 
> If the config file does not contain sections, or we're not using config
> file at all, nothing is changed and xfstests will work the same way as
> it used to.
> 
> This is very useful for testing file system with different options. Here
> is an example of the config file with sections:
> 
> [ext4_4k_block_size]
> TEST_DEV=/dev/sda
> TEST_DIR=/mnt/test
> SCRATCH_DEV=/dev/sdb
> SCRATCH_MNT=/mnt/test1
> MKFS_OPTIONS="-q -F -b4096"
> FSTYP=ext4
> 
> [ext4_1k_block_size]
> MKFS_OPTIONS="-q -F -b1024"
> 
> [ext4_nojournal]
> MKFS_OPTIONS="-q -F -b4096 -O ^has_journal"
> 
> [ext4_discard_ssd]
> MKFS_OPTIONS="-q -F -b4096"
> TEST_DEV=/dev/sdc
> SCRATCH_DEV=/dev/sdd
> MOUNT_OPTIONS="-o discard"
> 
> Note that once the variable is set it remains set across the sections, so
> you do not have to specify all the options in all sections. However one
> have to make sure that unwanted options are not set from previous
> sections.

I like the idea, but a lot of this needs to be in the (missing)
patch 0 description for the series. YOu also need to describe how
these get invoked, what the output looks like, etc because right now
I can't work it out from this...

> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> ---
>  check         | 371 +++++++++++++++++++++++++++++++++-------------------------
>  common/config | 126 ++++++++++++--------
>  common/rc     |  63 +++++-----
>  3 files changed, 326 insertions(+), 234 deletions(-)

This patch probably needs to be broken up, too. A substantial part
of it is indentation changes, which probably should be split into 2
parts - factor code into function, then wrap loop around function.
The change to the summary information should be done as a separate
patch, too. I suspect many of the common/config changes coul dbe
split up, too.

The changing of the $RESULT_BASE should probably also
bein a separate patch, because this is something that we'll need to
discuss as it changes the structure of the output....

Oh, and why make a distinction between no sections and
$OPTIONS_HAVE_SECTIONS in the config file? Surely no sections is
just the same as having 1 section....

> +parse_config_section() {
> +	SECTION=$1
> +	if ! $OPTIONS_HAVE_SECTIONS; then
> +		return 0
> +	fi
> +	eval `sed -e 's/[[:space:]]*\=[[:space:]]*/=/g' \
> +		-e 's/#.*$//' \
> +		-e 's/[[:space:]]*$//' \
> +		-e 's/^[[:space:]]*//' \
> +		-e "s/^\(.*\)=\([^\"']*\)$/export \1=\"\2\"/" \
> +		< $HOST_OPTIONS \
> +		| sed -n -e "/^\[$SECTION\]/,/^\s*\[/{/^[^;].*\=.*/p;}"`
> +}

And line noise like this needs comments explaining the format and
what it is doing. ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs