Libguestfs - Oct 2015 - [PATCH 1/2] tests: Fix test-launch-race.

This test has been broken for a while.  It is meant to test that when
the appliance cachedir is empty, two simultaneous runs of libguestfs
(both rebuilding the full appliance) will not cause conflicts, because
(eg) the locking in either supermin or libguestfs is not working.

However the test only set $TMPDIR, but the ./run script sets
$LIBGUESTFS_CACHEDIR which overrides $TMPDIR, so it was simply reusing
the existing appliance, and hence not testing anything.

Fix this by clearing $LIBGUESTFS_CACHEDIR.

Note the test now takes a lot longer to run since it does a full
appliance rebuild.
---
 tests/protocol/test-launch-race.pl | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/protocol/test-launch-race.pl
b/tests/protocol/test-launch-race.pl
index fadfdbf..6fef1a5 100755
--- a/tests/protocol/test-launch-race.pl
+++ b/tests/protocol/test-launch-race.pl
@@ -1,5 +1,5 @@
 #!/usr/bin/env perl
-# Copyright (C) 2010 Red Hat Inc.
+# Copyright (C) 2010-2015 Red Hat Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -32,6 +32,9 @@ exit 77 if $ENV{SKIP_TEST_LAUNCH_RACE_PL};
 my $tmpdir = tempdir (CLEANUP => 1);
 $ENV{TMPDIR} = $tmpdir;
 
+# Unset LIBGUESTFS_CACHEDIR (set by ./run) since that will override TMPDIR.
+delete $ENV{LIBGUESTFS_CACHEDIR};
+
 my $pid = fork();
 die ("fork failed: $!") if ($pid < 0);
 
-- 
2.5.0

Calling _exit(2) in the child process has the side effect that
tmp/libguestfsXXXXXX is not cleaned up.  Clean it up by ensuring the
handle is properly closed before _exit.
---
 tests/protocol/test-launch-race.pl | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/protocol/test-launch-race.pl
b/tests/protocol/test-launch-race.pl
index 6fef1a5..f522cc1 100755
--- a/tests/protocol/test-launch-race.pl
+++ b/tests/protocol/test-launch-race.pl
@@ -42,7 +42,9 @@ if ($pid == 0) {
   my $g = Sys::Guestfs->new ();
   $g->add_drive ("/dev/null");
   $g->launch ();
-  _exit (0); # So the tmpdir is not removed.
+  $g->close ();
+  # So $tmpdir is not removed by CLEANUP => 1 above.
+  _exit (0);
 }
 
 my $g = Sys::Guestfs->new ();
-- 
2.5.0

On Tuesday 06 October 2015 11:48:54 Richard W.M. Jones
wrote:
> This test has been broken for a while.  It is meant to test that when
> the appliance cachedir is empty, two simultaneous runs of libguestfs
> (both rebuilding the full appliance) will not cause conflicts, because
> (eg) the locking in either supermin or libguestfs is not working.
> 
> However the test only set $TMPDIR, but the ./run script sets
> $LIBGUESTFS_CACHEDIR which overrides $TMPDIR, so it was simply reusing
> the existing appliance, and hence not testing anything.
> 
> Fix this by clearing $LIBGUESTFS_CACHEDIR.
> 
> Note the test now takes a lot longer to run since it does a full
> appliance rebuild.
> ---
>  tests/protocol/test-launch-race.pl | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/protocol/test-launch-race.pl
b/tests/protocol/test-launch-race.pl
> index fadfdbf..6fef1a5 100755
> --- a/tests/protocol/test-launch-race.pl
> +++ b/tests/protocol/test-launch-race.pl
> @@ -1,5 +1,5 @@
>  #!/usr/bin/env perl
> -# Copyright (C) 2010 Red Hat Inc.
> +# Copyright (C) 2010-2015 Red Hat Inc.
>  #
>  # This program is free software; you can redistribute it and/or modify
>  # it under the terms of the GNU General Public License as published by
> @@ -32,6 +32,9 @@ exit 77 if $ENV{SKIP_TEST_LAUNCH_RACE_PL};
>  my $tmpdir = tempdir (CLEANUP => 1);
>  $ENV{TMPDIR} = $tmpdir;
>  
> +# Unset LIBGUESTFS_CACHEDIR (set by ./run) since that will override
TMPDIR.
> +delete $ENV{LIBGUESTFS_CACHEDIR};
> +
Should $LIBGUESTFS_TMPDIR be unset too, just to not rely solely on that
being overridden by $TMPDIR?

Anyway, patches LGTM.	

-- 
Pino Toscano

On Wed, Oct 07, 2015 at 02:03:06PM +0200, Pino Toscano
wrote:
> On Tuesday 06 October 2015 11:48:54 Richard W.M. Jones wrote:
> > This test has been broken for a while.  It is meant to test that when
> > the appliance cachedir is empty, two simultaneous runs of libguestfs
> > (both rebuilding the full appliance) will not cause conflicts, because
> > (eg) the locking in either supermin or libguestfs is not working.
> > 
> > However the test only set $TMPDIR, but the ./run script sets
> > $LIBGUESTFS_CACHEDIR which overrides $TMPDIR, so it was simply reusing
> > the existing appliance, and hence not testing anything.
> > 
> > Fix this by clearing $LIBGUESTFS_CACHEDIR.
> > 
> > Note the test now takes a lot longer to run since it does a full
> > appliance rebuild.
> > ---
> >  tests/protocol/test-launch-race.pl | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/protocol/test-launch-race.pl
b/tests/protocol/test-launch-race.pl
> > index fadfdbf..6fef1a5 100755
> > --- a/tests/protocol/test-launch-race.pl
> > +++ b/tests/protocol/test-launch-race.pl
> > @@ -1,5 +1,5 @@
> >  #!/usr/bin/env perl
> > -# Copyright (C) 2010 Red Hat Inc.
> > +# Copyright (C) 2010-2015 Red Hat Inc.
> >  #
> >  # This program is free software; you can redistribute it and/or
modify
> >  # it under the terms of the GNU General Public License as published
by
> > @@ -32,6 +32,9 @@ exit 77 if $ENV{SKIP_TEST_LAUNCH_RACE_PL};
> >  my $tmpdir = tempdir (CLEANUP => 1);
> >  $ENV{TMPDIR} = $tmpdir;
> >  
> > +# Unset LIBGUESTFS_CACHEDIR (set by ./run) since that will override
TMPDIR.
> > +delete $ENV{LIBGUESTFS_CACHEDIR};
> > +
> 
> Should $LIBGUESTFS_TMPDIR be unset too, just to not rely solely on that
> being overridden by $TMPDIR?
Overriding $LIBGUESTFS_TMPDIR causes the libguestfs temporary
directory to be placed under $tmpdir (instead of in ./tmp), which is
not bad or anything like that, but isn't relevant to the test.
> Anyway, patches LGTM.	
Thanks, Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html