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

2024-02-07 Thread 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é 

Thanks, applied to the block branch.

Kevin




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

2024-02-05 Thread 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é 
---
  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 

(with my limited understanding of this code)

Thanks!

/mjt



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

2024-02-05 Thread Peter Maydell
On Mon, 5 Feb 2024 at 15:41, Daniel P. Berrangé  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é 

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/tmp" are very hard to
pin down to the code that created them.

thanks
-- PMM