Re: [PATCH 2/6] Python: expose QEMUMachine's temporary directory

[Cleber Rosa]

On Fri, Feb 12, 2021 at 12:35:26AM +0100, Philippe Mathieu-Daudé wrote:
> On 2/11/21 11:01 PM, Cleber Rosa wrote:
> > Each instance of qemu.machine.QEMUMachine currently has a "test
> > directory", which may not have any relation to a "test", and it's
> > really a temporary directory.
> > 
> > Users instantiating the QEMUMachine class will be able to set the
> > location of the directory that will *contain* the QEMUMachine unique
> > temporary directory, so that parameter name has been changed from
> > test_dir to base_temp_dir.
> > 
> > A property has been added to allow users to access it without using
> > private attributes, and with that, the directory is created on first
> > use of the property.
> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >  python/qemu/machine.py         | 24 ++++++++++++++++--------
> >  python/qemu/qtest.py           |  6 +++---
> >  tests/acceptance/virtio-gpu.py |  2 +-
> >  tests/qemu-iotests/iotests.py  |  2 +-
> >  4 files changed, 21 insertions(+), 13 deletions(-)
> > 
> > diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> > index 6e44bda337..b379fcbe72 100644
> > --- a/python/qemu/machine.py
> > +++ b/python/qemu/machine.py
> > @@ -84,7 +84,7 @@ class QEMUMachine:
> >                   args: Sequence[str] = (),
> >                   wrapper: Sequence[str] = (),
> >                   name: Optional[str] = None,
> > -                 test_dir: str = "/var/tmp",
> > +                 base_temp_dir: str = "/var/tmp",
> 
> Not this patch fault, but I see we use /var/tmp since commit
> 66613974468 ("scripts: refactor the VM class in iotests for reuse").
> Can we use an OS agnostic method to get temp storage directory instead?
> 

Sounds like a reasonable thing to do.  I do remember a few issues with
Python's tempfile.gettempdir() returning '/tmp' on most Linux/Unix,
dur to the fact that '/tmp' is a tmpfs filesystem on most modern Linux
systems.

Anyway, I agree that hardcoding '/var/tmp' needs to be reconsidered.

Cheers,
- Cleber.

signature.asc
Description: PGP signature