Re: [PATCH 3/6] Acceptance Tests: use the job work directory for created VMs

[John Snow <jsnow@redhat.com>]

On 2/11/21 5:01 PM, Cleber Rosa wrote:
> The QEMUMachine uses a base temporary directory for all temporary
> needs.  By setting it to the Avocado's workdir, it's possible to
> keep the temporary files during debugging sessions much more
> easily by setting the "--keep-tmp" command line option.
> 

Makes sense, kinda-sorta.

Is this a band-aid?

QEMUMachine has this gem in _post_shutdown():

         if self._temp_dir is not None:
             shutil.rmtree(self._temp_dir)
             self._temp_dir = None

we probably do want a way to adjust the deletion policy that QEMUMachine 
has. Kevin and I discussed this (extremely briefly) in January. One note 
is that the QEMU logs are read into memory when the process closes, so 
iotests et al have a chance to show you those logs on failure cases. Not 
relevant here for your purposes.

Meanwhile, we could also change the behavior of QEMUMachine, and create 
a temp dir deletion policy tunable:

(1) Always delete
(2) Never delete
(3) Delete on success (keep on failure)
(4) Delete on success and anticipated failures.

(About #4: QEMUMachine has a condition where it will not report 
AbornalShutdown if .kill() is called and the retcode is observed to be 
-SIGKILL. We treat this as a kind of success.)

These avocado tests could then just use a "never delete" policy, use the 
avocado runner's working dir, and then never worry about the cleanup.

iotests could do something similar with a temporary directory 
established by the top-level runner that we could modify the behavior of 
with --keep-files[ <on-failure|always>] or similar.

It might be time to just make this less stupidly annoying for all users 
of the library once and for all.

> Reference: https://avocado-framework.readthedocs.io/en/85.0/api/test/avocado.html#avocado.Test.workdir
> Reference: https://avocado-framework.readthedocs.io/en/85.0/config/index.html#run-keep-tmp
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/avocado_qemu/__init__.py | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index bf54e419da..b7ab836455 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -172,7 +172,8 @@ class Test(avocado.Test):
>   
>       def _new_vm(self, *args):
>           self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_")
> -        vm = QEMUMachine(self.qemu_bin, sock_dir=self._sd.name)
> +        vm = QEMUMachine(self.qemu_bin, base_temp_dir=self.workdir,
> +                         sock_dir=self._sd.name)
>           if args:
>               vm.add_args(*args)
>           return vm
> 

But, you know, absent all that extra work, sure:

Reviewed-by: John Snow <jsnow@redhat.com>