qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tests: honor $TMPDIR for test_virtio_version
@ 2025-08-31 12:52 Maxim Cournoyer
  2025-09-01  6:41 ` Michael Tokarev
  0 siblings, 1 reply; 4+ messages in thread
From: Maxim Cournoyer @ 2025-08-31 12:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: Michael Tokarev, Maxim Cournoyer

Until 10.1.0, the test suite could be run without having a writable
/var/tmp in the build environment.  To avoid now requiring /var/tmp in
the build environment (which can be a very minimal container like in
the case of GNU Guix), consult TMPDIR first, using /var/tmp as a
fallback.

Signed-off-by: Maxim Cournoyer <maxim@guixotic.coop>
---
 tests/functional/x86_64/test_virtio_version.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/functional/x86_64/test_virtio_version.py b/tests/functional/x86_64/test_virtio_version.py
index a5ea73237f..501545f655 100755
--- a/tests/functional/x86_64/test_virtio_version.py
+++ b/tests/functional/x86_64/test_virtio_version.py
@@ -10,6 +10,8 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later.  See the COPYING file in the top-level directory.
 
+import os
+
 from qemu.machine import QEMUMachine
 from qemu_test import QemuSystemTest
 
@@ -68,7 +70,9 @@ def run_device(self, devtype, opts=None, machine='pc'):
         """
         Run QEMU with `-device DEVTYPE`, return device info from `query-pci`
         """
-        with QEMUMachine(self.qemu_bin) as vm:
+        with QEMUMachine(
+                self.qemu_bin,
+                base_temp_dir=os.environ.get('TMPDIR', '/var/tmp')) as vm:
             vm.set_machine(machine)
             if opts:
                 devtype += ',' + opts

base-commit: e101d33792530093fa0b0a6e5f43e4d8cfe4581e
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] tests: honor $TMPDIR for test_virtio_version
  2025-08-31 12:52 [PATCH] tests: honor $TMPDIR for test_virtio_version Maxim Cournoyer
@ 2025-09-01  6:41 ` Michael Tokarev
  2025-09-01  7:36   ` Maxim Cournoyer
  2025-09-02 10:36   ` Richard Henderson
  0 siblings, 2 replies; 4+ messages in thread
From: Michael Tokarev @ 2025-09-01  6:41 UTC (permalink / raw)
  To: Maxim Cournoyer, qemu-devel

On 31.08.2025 15:52, Maxim Cournoyer wrote:
> Until 10.1.0, the test suite could be run without having a writable
> /var/tmp in the build environment.  To avoid now requiring /var/tmp in
> the build environment (which can be a very minimal container like in
> the case of GNU Guix), consult TMPDIR first, using /var/tmp as a
> fallback.

> diff --git a/tests/functional/x86_64/test_virtio_version.py b/tests/functional/x86_64/test_virtio_version.py
> index a5ea73237f..501545f655 100755
> --- a/tests/functional/x86_64/test_virtio_version.py
> +++ b/tests/functional/x86_64/test_virtio_version.py

> @@ -68,7 +70,9 @@ def run_device(self, devtype, opts=None, machine='pc'):
>           """
>           Run QEMU with `-device DEVTYPE`, return device info from `query-pci`
>           """
> -        with QEMUMachine(self.qemu_bin) as vm:
> +        with QEMUMachine(
> +                self.qemu_bin,
> +                base_temp_dir=os.environ.get('TMPDIR', '/var/tmp')) as vm:

I don't think /var/tmp is a good choice here, - I'm a bit surprised it
come from you when your OS doesn't have /var/tmp by default - shouldn't
it be /tmp here?

Overall, /var/tmp is a strange choice here and in a few other tests too,
-- maybe only block.c default is the only right place to have it as the
fallback/default.

Thanks,

/mjt


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tests: honor $TMPDIR for test_virtio_version
  2025-09-01  6:41 ` Michael Tokarev
@ 2025-09-01  7:36   ` Maxim Cournoyer
  2025-09-02 10:36   ` Richard Henderson
  1 sibling, 0 replies; 4+ messages in thread
From: Maxim Cournoyer @ 2025-09-01  7:36 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: qemu-devel

Hi,

Michael Tokarev <mjt@tls.msk.ru> writes:

[...]

>> diff --git a/tests/functional/x86_64/test_virtio_version.py b/tests/functional/x86_64/test_virtio_version.py
>> index a5ea73237f..501545f655 100755
>> --- a/tests/functional/x86_64/test_virtio_version.py
>> +++ b/tests/functional/x86_64/test_virtio_version.py
>
>> @@ -68,7 +70,9 @@ def run_device(self, devtype, opts=None, machine='pc'):
>>           """
>>           Run QEMU with `-device DEVTYPE`, return device info from `query-pci`
>>           """
>> -        with QEMUMachine(self.qemu_bin) as vm:
>> +        with QEMUMachine(
>> +                self.qemu_bin,
>> +                base_temp_dir=os.environ.get('TMPDIR', '/var/tmp')) as vm:
>
> I don't think /var/tmp is a good choice here, - I'm a bit surprised it
> come from you when your OS doesn't have /var/tmp by default - shouldn't
> it be /tmp here?

I thought I'd make the smallest possible change compared the current
behavior, to maximize the chances this lands. The current behavior comes
from the default base_temp_dir value of QEMUMachine, which defaults to
/var/tmp.

If you prefer to fallback to just '/tmp' rather than '/var/tmp', that's
fine for me as well, though it could be surprising considering
QEMUMachine normally uses /var/tmp at run time.

-- 
Thanks,
Maxim


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tests: honor $TMPDIR for test_virtio_version
  2025-09-01  6:41 ` Michael Tokarev
  2025-09-01  7:36   ` Maxim Cournoyer
@ 2025-09-02 10:36   ` Richard Henderson
  1 sibling, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2025-09-02 10:36 UTC (permalink / raw)
  To: Michael Tokarev, Maxim Cournoyer, qemu-devel

On 9/1/25 16:41, Michael Tokarev wrote:
> On 31.08.2025 15:52, Maxim Cournoyer wrote:
>> Until 10.1.0, the test suite could be run without having a writable
>> /var/tmp in the build environment.  To avoid now requiring /var/tmp in
>> the build environment (which can be a very minimal container like in
>> the case of GNU Guix), consult TMPDIR first, using /var/tmp as a
>> fallback.
> 
>> diff --git a/tests/functional/x86_64/test_virtio_version.py b/tests/functional/x86_64/ 
>> test_virtio_version.py
>> index a5ea73237f..501545f655 100755
>> --- a/tests/functional/x86_64/test_virtio_version.py
>> +++ b/tests/functional/x86_64/test_virtio_version.py
> 
>> @@ -68,7 +70,9 @@ def run_device(self, devtype, opts=None, machine='pc'):
>>           """
>>           Run QEMU with `-device DEVTYPE`, return device info from `query-pci`
>>           """
>> -        with QEMUMachine(self.qemu_bin) as vm:
>> +        with QEMUMachine(
>> +                self.qemu_bin,
>> +                base_temp_dir=os.environ.get('TMPDIR', '/var/tmp')) as vm:
> 
> I don't think /var/tmp is a good choice here, - I'm a bit surprised it
> come from you when your OS doesn't have /var/tmp by default - shouldn't
> it be /tmp here?
> 
> Overall, /var/tmp is a strange choice here and in a few other tests too,
> -- maybe only block.c default is the only right place to have it as the
> fallback/default.

It's possible that it's the testsuite that should be changed to prefer /var/tmp when 
present and TMPDIR is tmpfs.  I don't know enough about what prompted the original change 
in the first place.

But my point is that with *only* this change, it is very likely that this simply exchanges 
one set of failures for another.


r~


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-09-02 10:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-31 12:52 [PATCH] tests: honor $TMPDIR for test_virtio_version Maxim Cournoyer
2025-09-01  6:41 ` Michael Tokarev
2025-09-01  7:36   ` Maxim Cournoyer
2025-09-02 10:36   ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).