[PATCH] iotests: fix leak of tmpdir in dry-run mode

[PATCH] iotests: fix leak of tmpdir in dry-run mode

[Daniel P. Berrangé]

Creating an instance of the 'TestEnv' class will create a temporary
directory. This dir is only deleted, however, in the __exit__ handler
invoked by a context manager.

In dry-run mode, we don't use the TestEnv via a context manager, so
were leaking the temporary directory. Since meson invokes 'check'
5 times on each configure run, developers /tmp was filling up with
empty temporary directories.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qemu-iotests/check | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index f2e9d27dcf..56d88ca423 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -184,7 +184,8 @@ if __name__ == '__main__':
         sys.exit(str(e))
 
     if args.dry_run:
-        print('\n'.join([os.path.basename(t) for t in tests]))
+        with env:
+            print('\n'.join([os.path.basename(t) for t in tests]))
     else:
         with TestRunner(env, tap=args.tap,
                         color=args.color) as tr:
-- 
2.43.0

Re: [PATCH] iotests: fix leak of tmpdir in dry-run mode

[Peter Maydell]

On Mon, 5 Feb 2024 at 15:41, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> Creating an instance of the 'TestEnv' class will create a temporary
> directory. This dir is only deleted, however, in the __exit__ handler
> invoked by a context manager.
>
> In dry-run mode, we don't use the TestEnv via a context manager, so
> were leaking the temporary directory. Since meson invokes 'check'
> 5 times on each configure run, developers /tmp was filling up with
> empty temporary directories.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Could we also arrange for the temp directory to be created
with a name that makes it easy to identify what has created it?
Very generic names like "/tmp/tmpNNNNNNNN" are very hard to
pin down to the code that created them.

thanks
-- PMM

Re: [PATCH] iotests: fix leak of tmpdir in dry-run mode

[Michael Tokarev]

05.02.2024 18:40, Daniel P. Berrangé :
> Creating an instance of the 'TestEnv' class will create a temporary
> directory. This dir is only deleted, however, in the __exit__ handler
> invoked by a context manager.
> 
> In dry-run mode, we don't use the TestEnv via a context manager, so
> were leaking the temporary directory. Since meson invokes 'check'
> 5 times on each configure run, developers /tmp was filling up with
> empty temporary directories.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/qemu-iotests/check | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index f2e9d27dcf..56d88ca423 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -184,7 +184,8 @@ if __name__ == '__main__':
>           sys.exit(str(e))
>   
>       if args.dry_run:
> -        print('\n'.join([os.path.basename(t) for t in tests]))
> +        with env:
> +            print('\n'.join([os.path.basename(t) for t in tests]))
>       else:
>           with TestRunner(env, tap=args.tap,
>                           color=args.color) as tr:

Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>

(with my limited understanding of this code)

Thanks!

/mjt

Re: [PATCH] iotests: fix leak of tmpdir in dry-run mode

[Kevin Wolf]

Am 05.02.2024 um 16:40 hat Daniel P. Berrangé geschrieben:
> Creating an instance of the 'TestEnv' class will create a temporary
> directory. This dir is only deleted, however, in the __exit__ handler
> invoked by a context manager.
> 
> In dry-run mode, we don't use the TestEnv via a context manager, so
> were leaking the temporary directory. Since meson invokes 'check'
> 5 times on each configure run, developers /tmp was filling up with
> empty temporary directories.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Thanks, applied to the block branch.

Kevin