[PATCH 0/6] xfstests-bld: Fixes and improvements v1

[PATCH 0/6] xfstests-bld: Fixes and improvements v1

[Dmitry Monakhov]

Hi,

Please review and pull my patch-queue for xfstests-bld
# Fix build (currently xfstests-bld is broken)
xfstests-bld: build update xfstests repo
fix build typos

# Minor improvements
kvm-xfstests: update command line help
xfstest-bld: use parallel gzip if possible
kvm-xfstests: add initrd support

# Dockerize xfstests-bld build enviroment
# github has integration with hub.docker so each commit update
# trigger auto build so we have CI for free.
build: Add dockerfile


https://github.com/dmonakhov/xfstests-bld to_pull-v1

Re: [PATCH 6/6] Add dockerfile

[Theodore Ts'o]

On Fri, Aug 19, 2016 at 12:54:11AM +0400, Dmitry Monakhov wrote:
> Dockerfile is good way to create build environments
> Let's keep it as a reference build script.
> 
> XXX: Currently Ted does not have hub.docker account
>      so this docker file points to my hub. Should be updated.

I guess I'm a little confused about how this Dockerfile works.  First
of all, says "From Debian", so I guess the idea is to be based on an
unspecified version of Debian (why not debian:jessie?).  But then it
has apt-get commands?  How does that work?

And if the idea is to fetch a prebuilt docker image, and we're running
it with docker run --privilege, I'm not sure what value using docker
run is really providing.  We're not using the docker container
features.   And typing

docker run -i -t --privileged ... \
	"kvm-xfstests.sh --kernel /tmp/bzImage --update-files --update-xfstests-tar  smoke"

Is just *awkward*.  The whole point is to let kvm-xfstests read the
kernel from your ~/.config/gce-xfstests, so your work flow can be
something like this:

git am -s ~/mbox/patch.mbox
../make-ext4      # shortcut for something like "make O=/build/ext4 -j16"
kvm-xfstests smoke   # ~/.config/gce-xfststs sets GCE_KERNEL=/build/ext4

needing to type a long docker run command doesn't seem to add any
value.

Confused,

					- Ted

Re: [PATCH 1/6] xfstests-bld: build update xfstests repo

[Theodore Ts'o]

On Fri, Aug 19, 2016 at 12:54:06AM +0400, Dmitry Monakhov wrote:
> The official location for xfstests source is now:
> git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>

Applied, thanks.

					- Ted

Re: [PATCH 2/6] fix build typos

[Theodore Ts'o]

On Fri, Aug 19, 2016 at 12:54:07AM +0400, Dmitry Monakhov wrote:
> build-all: --xfsprogs-only => SKIP_XFSPROGS, not SKIP_XFSTESTS
> get-all: fix typo in case of explicit xfsprogs_commit
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>

Applied, thanks.

					- Ted

Re: [PATCH 3/6] kvm-xfstests: update command line help

[Theodore Ts'o]

On Fri, Aug 19, 2016 at 12:54:08AM +0400, Dmitry Monakhov wrote:
> diff --git a/kvm-xfstests/util/parse_cli b/kvm-xfstests/util/parse_cli
> index 83400ea..edbbafa 100644
> --- a/kvm-xfstests/util/parse_cli
> +++ b/kvm-xfstests/util/parse_cli
> @@ -26,10 +26,47 @@ print_help ()
>      echo "	-o opts		- Extra kernel command line options"
>      echo "	-r ram		- Specify memory to be used in megabytes"
>      echo "	-X test		- Exclude test from running"
> +    echo "	--no-punch	- Do not run tests w/ patch hole"
> +    echo "	--no-collapse	- Do not run tests w/ collapse range"
> +    echo "	--no-insert	- Do not run tests w/ insert range"
> +    echo "	--no-zero	- Do not run tests w/ zero range"

The reason why I don't document these options is they depend on an
out-of-tree patch for xfstests.  It's in my githup repo, but it's not
something which xfstests upstream has seen fit to take.

>      if test "$GCE_XFSTESTS" != "yes" ; then
>  	echo "	--kernel file	- Boot the specified kernel"
>  
> +    else
> +	echo "GCE speciffic commands:"
> +	echo "	ls-instances"
> +        echo "	ls|ls-instances"
> +        echo "	rm|rm-instances|abort|abort-instances"
> +        echo "	start|start-instance|start-instances"
> +        echo "	stop|stop-instance|stop-instances"
> +        echo "	ls-disks|ls-disk"
> +        echo "	rm-disks|rm-disk"
> +        echo "	ls-results|ls-gcs"
> +        echo "	rm-results|rm-gcs"
> +        echo "	get-results"
> +        echo "	setup"
> +        echo "	ssh"
> +        echo "	console"
> +        echo "	serial"
> +        echo "	get-results"
> +        echo "	create-image"
> +        echo "	export-image"
> +        echo "	import-image"
> +        echo "	--numa"
> +        echo "	--log"
> +        echo "	--local-ssd"
> +        echo "	--pmem-device"
> +        echo "	--machtype"
> +        echo "	--image-project"
> +        echo "	--instance-name"
> +        echo "	--no-virtio-rng"
>      fi
> +    echo "Base image update commands:"
> +    echo "	--update-xfstests-tar"
> +    echo "	--update-xfstests"
> +    echo "	--update-files"
> +    echo
>      echo "	--no-log	- Don't save the log file for this run"
>      echo "	--no-action	- Print the command to start the VM"
>      echo ""

The options don't belong next to the GCE specific commands.  And
gce-import, as well as the base image update options, are really
intended for the maintainer (e.g., for me to update the xfstests-cloud
public image project).

The main issue is I'm a bit concerned about making the help message
too long.  My long term vision is to go in the direction of git and
gcloud's style of help messages / man pages.  So gce-xfstests --help
and kvm-xfstests --help will display a list of the most commonly used
/ useful commands and options.  And where "gce-xfstests get-results
--help" will display a full man page for the get-results subcommand,
and "gce-xfstests help commands" will display a full list of all
commands, and "gce-xfstests help topics" will allow you to see the
documentation currently available as markdown files in Documentation.

Cheers,

						- Ted

Re: [PATCH 3/6] kvm-xfstests: update command line help

[Dmitry Monakhov]

[-- Attachment #1: Type: text/plain, Size: 3497 bytes --]

Theodore Ts'o <tytso@mit.edu> writes:

> On Fri, Aug 19, 2016 at 12:54:08AM +0400, Dmitry Monakhov wrote:
>> diff --git a/kvm-xfstests/util/parse_cli b/kvm-xfstests/util/parse_cli
>> index 83400ea..edbbafa 100644
>> --- a/kvm-xfstests/util/parse_cli
>> +++ b/kvm-xfstests/util/parse_cli
>> @@ -26,10 +26,47 @@ print_help ()
>>      echo "	-o opts		- Extra kernel command line options"
>>      echo "	-r ram		- Specify memory to be used in megabytes"
>>      echo "	-X test		- Exclude test from running"
>> +    echo "	--no-punch	- Do not run tests w/ patch hole"
>> +    echo "	--no-collapse	- Do not run tests w/ collapse range"
>> +    echo "	--no-insert	- Do not run tests w/ insert range"
>> +    echo "	--no-zero	- Do not run tests w/ zero range"
>
> The reason why I don't document these options is they depend on an
> out-of-tree patch for xfstests.  It's in my githup repo, but it's not
> something which xfstests upstream has seen fit to take.
>
>>      if test "$GCE_XFSTESTS" != "yes" ; then
>>  	echo "	--kernel file	- Boot the specified kernel"
>>  
>> +    else
>> +	echo "GCE speciffic commands:"
>> +	echo "	ls-instances"
>> +        echo "	ls|ls-instances"
>> +        echo "	rm|rm-instances|abort|abort-instances"
>> +        echo "	start|start-instance|start-instances"
>> +        echo "	stop|stop-instance|stop-instances"
>> +        echo "	ls-disks|ls-disk"
>> +        echo "	rm-disks|rm-disk"
>> +        echo "	ls-results|ls-gcs"
>> +        echo "	rm-results|rm-gcs"
>> +        echo "	get-results"
>> +        echo "	setup"
>> +        echo "	ssh"
>> +        echo "	console"
>> +        echo "	serial"
>> +        echo "	get-results"
>> +        echo "	create-image"
>> +        echo "	export-image"
>> +        echo "	import-image"
>> +        echo "	--numa"
>> +        echo "	--log"
>> +        echo "	--local-ssd"
>> +        echo "	--pmem-device"
>> +        echo "	--machtype"
>> +        echo "	--image-project"
>> +        echo "	--instance-name"
>> +        echo "	--no-virtio-rng"
>>      fi
>> +    echo "Base image update commands:"
>> +    echo "	--update-xfstests-tar"
>> +    echo "	--update-xfstests"
>> +    echo "	--update-files"
>> +    echo
>>      echo "	--no-log	- Don't save the log file for this run"
>>      echo "	--no-action	- Print the command to start the VM"
>>      echo ""
>
> The options don't belong next to the GCE specific commands.  And
> gce-import, as well as the base image update options, are really
> intended for the maintainer (e.g., for me to update the xfstests-cloud
> public image project).
>
> The main issue is I'm a bit concerned about making the help message
> too long.  My long term vision is to go in the direction of git and
> gcloud's style of help messages / man pages.  So gce-xfstests --help
> and kvm-xfstests --help will display a list of the most commonly used
> / useful commands and options.  And where "gce-xfstests get-results
> --help" will display a full man page for the get-results subcommand,
> and "gce-xfstests help commands" will display a full list of all
> commands, and "gce-xfstests help topics" will allow you to see the
> documentation currently available as markdown files in Documentation.
Agree. My point was that it is easier to:
0 place info about all commands here fist: DONE
2 visualize it,
3 make a decision about readability
4 cut less interesting options.
5 goto 2
>
> Cheers,
>
> 						- Ted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH 3/6] kvm-xfstests: update command line help

[Theodore Ts'o]

I've improved the usage commands for gen-image and gce-xfstests
somewhat.  It's roughly based on your patch, but as discussed, there
are certain options and commands which I don't want to include in the
usage messages either because they rely on out-of-tree xfstests
patches, or they are too specialized to be of interest to most users.

Cheers,

					- Ted

Re: [PATCH 5/6] kvm-xfstests: add initrd support

[Theodore Ts'o]

On Fri, Aug 19, 2016 at 12:54:10AM +0400, Dmitry Monakhov wrote:
> +	--initrd) shift
> +		  INITRD="$1"
> +		  if test ! -f "$INITRD" ; then
> +		      print_help
> +		  fi
> +		  ;;

We should only allow --initrd for kvm-xfstests, or add support for
uploading the initrd to gce-xfstests, and then adding support to it to
the gce-kexec script.

We can just allow it for kvm-xfstests first, and then only later add
support to gce-xfstests, if you don't have time to work to get
gce-xfstests support for --initrd working.

Cheers,

						- Ted

Re: [PATCH 5/6] kvm-xfstests: add initrd support

[Dmitry Monakhov]

[-- Attachment #1: Type: text/plain, Size: 998 bytes --]

Theodore Ts'o <tytso@mit.edu> writes:

> On Fri, Aug 19, 2016 at 12:54:10AM +0400, Dmitry Monakhov wrote:
>> +	--initrd) shift
>> +		  INITRD="$1"
>> +		  if test ! -f "$INITRD" ; then
>> +		      print_help
>> +		  fi
>> +		  ;;
>
> We should only allow --initrd for kvm-xfstests, or add support for
> uploading the initrd to gce-xfstests, and then adding support to it to
> the gce-kexec script.
>
> We can just allow it for kvm-xfstests first, and then only later add
> support to gce-xfstests, if you don't have time to work to get
> gce-xfstests support for --initrd working.
No problem, but it looks like my knowledge about GCE is too low at the
moment. BTW are there are any way to make a bullet prof method to stop
gce instance after predefined timout? Your systemctl timeout script
does not always work. In my case it stuck somewhere inside FS and
timeout.service can not do it's job. Probably we can do it via
kernel watchdog or external watcher ala Jenkins.

>
> Cheers,
>
> 						- Ted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH 5/6] kvm-xfstests: add initrd support

[Theodore Ts'o]

On Fri, Aug 19, 2016 at 04:59:22PM +0300, Dmitry Monakhov wrote:
> No problem, but it looks like my knowledge about GCE is too low at the
> moment. BTW are there are any way to make a bullet prof method to stop
> gce instance after predefined timout? Your systemctl timeout script
> does not always work. In my case it stuck somewhere inside FS and
> timeout.service can not do it's job. Probably we can do it via
> kernel watchdog or external watcher ala Jenkins.

My long term vision was to use an external watcher that would run in
Google App Engine.  The idea would be that this would also take care
of launching separate VM's for each of the different test cases, and
then collate the reports into a single test report.  Long term I'd
also like to have the results stored into Google Cloud Datastore, and
do automatic flaky test detection.

For now, I just simply manually keep an eye on things using
"gce-xfstests ls -l", and if I see something running for too long,
I'll connect to it using "gce-xfstests console xfstests-XXXX" to grab
the results.  In the app-engine test runner vision it would use the
equivalent of "gce-xfstests serial xfstests-XXX" and store the
complete serial console output someplace safe.  What happens today
tends to be:

1) gce-xfstests -c overlay -g auto

2) periodically I'll run gce-xfstests ls -l, and notice when the VM
apparently is no longer making foreward progress.

3) Hmm, looks like overlayfs is blowing up.  And gce-xfstests console
doesn't give me enough history since it only stores the last N lines".

4) gce-xfstests abort xfstests-XXXXXX

5) rerun "gce-xfstests -c overaly -g auto", but now after it starts,
   also run: "script -c "gce-xfstests serial xfstests-XXXXX" console-XXXXX.out"

In practice this doesn't happen often enough that I've automated this,
and it's also why I haven't made it a high priority to create some
kind of external test running / monitoring service.

     		      	      		   - Ted

P.S. I recently added overlayfs support, and it looks like overlayfs
has a bug which ends up screwing up an inode link's count, and causing
the ext4 orphan list to get corrupted, and causing subsequent ext4
warnings and BUG's to get triggered.  So this isn't a hypothetical
example; it's just one that I haven't had time to track down yet.  :-)

Re: [PATCH 5/6] kvm-xfstests: add initrd support

[Theodore Ts'o]

On Fri, Aug 19, 2016 at 12:54:10AM +0400, Dmitry Monakhov wrote:
> Sometimes it is important to run kernel which was build with
> modules, so the only way to do this it to pass initrd parameter.
> 
> Example:
> # Run test on official FC24 kernel
> #wget https://mirror.yandex.ru/fedora/linux/releases/24/Server/x86_64/os/images/pxeboot/initrd.img
> #wget https://mirror.yandex.ru/fedora/linux/releases/24/Server/x86_64/os/images/pxeboot/vmlinuz
> #kvm-xfstests.sh --kernel vmlinuz --initrd initrd.img generic/013
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>

OK, applied with some slight modifications so that --initrd is only
accepted for kvm-xfstests.

BTW, I tried using the Fedora 24 kernel, and there's something wonky
with the serial console.  It looks like every other character which is
typed is getting eaten.  I suppose it's good to be able to test with
random distribution kernels with initrds, but there may be weird
compatibility issues with different distro's initrds, so I am a bit
worried that this might be a bit of a support headache in the long
term.

						- Ted

Re: [PATCH 4/6] xfstest-bld: use parallel gzip if possible

[Theodore Ts'o]

On Fri, Aug 19, 2016 at 12:54:09AM +0400, Dmitry Monakhov wrote:
> diff --git a/kvm-xfstests/kvm-xfstests b/kvm-xfstests/kvm-xfstests
> index d608317..10782d0 100755
> --- a/kvm-xfstests/kvm-xfstests
> +++ b/kvm-xfstests/kvm-xfstests
> @@ -9,6 +9,7 @@ fi
>  . "$DIR/util/get-config"
>  
>  QUIET="quiet loglevel=0"
> +which pigz &>/dev/null && GZIP=pigz || GZIP=gzip
>  
>  . $DIR/util/parse_cli
>  
> @@ -71,7 +72,7 @@ then
>      fi
>      (cd "$DIR/test-appliance"; \
>       tar -X kvm-exclude-files -C files -cf - . | \
> -	 gzip -9n > "$TDIR/files.tar.gz")
> +	 $GZIP -9n > "$TDIR/files.tar.gz")
>      tar -r -f $VDH -C "$TDIR" files.tar.gz
>      rm -rf "$TDIR"
>  fi

There's no real point in using pigz for files.tar.gz, since the file
is so small.  In fact, it's slightly slower on my system:

% time gzip -9 < /tmp/files.tar  > /tmp/files.tar.gz

real   0m0.010s
user   0m0.000s
sys    0m0.000s

% time pigz -9 < /tmp/files.tar  > /tmp/files.tar.gz

real   0m0.012s
user   0m0.010s
sys    0m0.010s

For the xfstests.tar.gz file, it does make sense; it cuts down the
compression time form 3 seconds to half a second.  That's fair,
although at least in my workflow I'm not recreating the
xfstests.tar.gz file all that often.

Cheers,

						- Ted

Re: [PATCH 4/6] xfstest-bld: use parallel gzip if possible

[Theodore Ts'o]

On Fri, Aug 19, 2016 at 12:54:09AM +0400, Dmitry Monakhov wrote:
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
>  gen-tarball                                  | 4 +++-
>  kvm-xfstests/gce-xfstests                    | 5 +++--
>  kvm-xfstests/kvm-xfstests                    | 3 ++-
>  kvm-xfstests/test-appliance/gce-create-image | 3 ++-
>  4 files changed, 10 insertions(+), 5 deletions(-)

I've applied just the gen-tarball portion of the patch, because that's
where using the parallelized gzip makes a difference.  For small
files, pigz can actually take a tiny bit _more_ time.

       	    		      	     	 - Ted