tests/functional/x86_64/test_virtio_gpu.py | 46 +++++++++++----------- 1 file changed, 23 insertions(+), 23 deletions(-)
From: Thomas Huth <thuth@redhat.com>
Use the recommended order for import statements, specify the kind of
exceptions that we try to catch, use f-strings where it makes sense,
rewrite the vug_log_file part with a proper "with" statement and
fix some FIXMEs by checking for the availability of the devices, etc.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/functional/x86_64/test_virtio_gpu.py | 46 +++++++++++-----------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/tests/functional/x86_64/test_virtio_gpu.py b/tests/functional/x86_64/test_virtio_gpu.py
index be96de24da2..a25f15cdb00 100755
--- a/tests/functional/x86_64/test_virtio_gpu.py
+++ b/tests/functional/x86_64/test_virtio_gpu.py
@@ -5,22 +5,23 @@
# 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
+import socket
+import subprocess
from qemu_test import QemuSystemTest, Asset
from qemu_test import wait_for_console_pattern
from qemu_test import exec_command_and_wait_for_pattern
from qemu_test import is_readable_executable_file
-
-import os
-import socket
-import subprocess
+from qemu.machine.machine import VMLaunchFailure
def pick_default_vug_bin(test):
bld_dir_path = test.build_file("contrib", "vhost-user-gpu", "vhost-user-gpu")
if is_readable_executable_file(bld_dir_path):
return bld_dir_path
+ return None
class VirtioGPUx86(QemuSystemTest):
@@ -46,8 +47,8 @@ def wait_for_console_pattern(self, success_message, vm=None):
)
def test_virtio_vga_virgl(self):
- # FIXME: should check presence of virtio, virgl etc
self.require_accelerator('kvm')
+ self.require_device('virtio-vga-gl')
kernel_path = self.ASSET_KERNEL.fetch()
initrd_path = self.ASSET_INITRD.fetch()
@@ -68,7 +69,7 @@ def test_virtio_vga_virgl(self):
)
try:
self.vm.launch()
- except:
+ except VMLaunchFailure:
# TODO: probably fails because we are missing the VirGL features
self.skipTest("VirGL not enabled?")
@@ -78,8 +79,8 @@ def test_virtio_vga_virgl(self):
)
def test_vhost_user_vga_virgl(self):
- # FIXME: should check presence of vhost-user-gpu, virgl, memfd etc
self.require_accelerator('kvm')
+ self.require_device('vhost-user-vga')
vug = pick_default_vug_bin(self)
if not vug:
@@ -95,27 +96,26 @@ def test_vhost_user_vga_virgl(self):
os.set_inheritable(qemu_sock.fileno(), True)
os.set_inheritable(vug_sock.fileno(), True)
- self._vug_log_path = self.log_file("vhost-user-gpu.log")
- self._vug_log_file = open(self._vug_log_path, "wb")
- self.log.info('Complete vhost-user-gpu.log file can be '
- 'found at %s', self._vug_log_path)
-
- vugp = subprocess.Popen(
- [vug, "--virgl", "--fd=%d" % vug_sock.fileno()],
- stdin=subprocess.DEVNULL,
- stdout=self._vug_log_file,
- stderr=subprocess.STDOUT,
- shell=False,
- close_fds=False,
- )
- self._vug_log_file.close()
+ vug_log_path = self.log_file("vhost-user-gpu.log")
+ self.log.info('Complete vhost-user-gpu.log file can be found at %s',
+ vug_log_path)
+ with open(vug_log_path, "wb") as vug_log_file:
+ # pylint: disable=consider-using-with
+ vugp = subprocess.Popen(
+ [vug, "--virgl", f"--fd={vug_sock.fileno()}"],
+ stdin=subprocess.DEVNULL,
+ stdout=vug_log_file,
+ stderr=subprocess.STDOUT,
+ shell=False,
+ close_fds=False,
+ )
self.vm.set_console()
self.vm.add_args("-cpu", "host")
self.vm.add_args("-m", "2G")
self.vm.add_args("-object", "memory-backend-memfd,id=mem,size=2G")
self.vm.add_args("-machine", "pc,memory-backend=mem,accel=kvm")
- self.vm.add_args("-chardev", "socket,id=vug,fd=%d" % qemu_sock.fileno())
+ self.vm.add_args("-chardev", f"socket,id=vug,fd={qemu_sock.fileno()}")
self.vm.add_args("-device", "vhost-user-vga,chardev=vug")
self.vm.add_args("-display", "egl-headless")
self.vm.add_args(
@@ -128,7 +128,7 @@ def test_vhost_user_vga_virgl(self):
)
try:
self.vm.launch()
- except:
+ except VMLaunchFailure:
# TODO: probably fails because we are missing the VirGL features
self.skipTest("VirGL not enabled?")
self.wait_for_console_pattern("as init process")
--
2.51.1On 2025/11/05 21:09, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
>
> Use the recommended order for import statements, specify the kind of
> exceptions that we try to catch, use f-strings where it makes sense,
> rewrite the vug_log_file part with a proper "with" statement and
> fix some FIXMEs by checking for the availability of the devices, etc.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> tests/functional/x86_64/test_virtio_gpu.py | 46 +++++++++++-----------
> 1 file changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/tests/functional/x86_64/test_virtio_gpu.py b/tests/functional/x86_64/test_virtio_gpu.py
> index be96de24da2..a25f15cdb00 100755
> --- a/tests/functional/x86_64/test_virtio_gpu.py
> +++ b/tests/functional/x86_64/test_virtio_gpu.py
> @@ -5,22 +5,23 @@
> # 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
> +import socket
> +import subprocess
>
> from qemu_test import QemuSystemTest, Asset
> from qemu_test import wait_for_console_pattern
> from qemu_test import exec_command_and_wait_for_pattern
> from qemu_test import is_readable_executable_file
>
> -
> -import os
> -import socket
> -import subprocess
> +from qemu.machine.machine import VMLaunchFailure
>
>
> def pick_default_vug_bin(test):
> bld_dir_path = test.build_file("contrib", "vhost-user-gpu", "vhost-user-gpu")
> if is_readable_executable_file(bld_dir_path):
> return bld_dir_path
> + return None
>
>
> class VirtioGPUx86(QemuSystemTest):
> @@ -46,8 +47,8 @@ def wait_for_console_pattern(self, success_message, vm=None):
> )
>
> def test_virtio_vga_virgl(self):
> - # FIXME: should check presence of virtio, virgl etc
> self.require_accelerator('kvm')
> + self.require_device('virtio-vga-gl')
>
> kernel_path = self.ASSET_KERNEL.fetch()
> initrd_path = self.ASSET_INITRD.fetch()
> @@ -68,7 +69,7 @@ def test_virtio_vga_virgl(self):
> )
> try:
> self.vm.launch()
> - except:
> + except VMLaunchFailure:
> # TODO: probably fails because we are missing the VirGL features
> self.skipTest("VirGL not enabled?")
>
> @@ -78,8 +79,8 @@ def test_virtio_vga_virgl(self):
> )
>
> def test_vhost_user_vga_virgl(self):
> - # FIXME: should check presence of vhost-user-gpu, virgl, memfd etc
> self.require_accelerator('kvm')
> + self.require_device('vhost-user-vga')
>
> vug = pick_default_vug_bin(self)
> if not vug:
> @@ -95,27 +96,26 @@ def test_vhost_user_vga_virgl(self):
> os.set_inheritable(qemu_sock.fileno(), True)
> os.set_inheritable(vug_sock.fileno(), True)
>
> - self._vug_log_path = self.log_file("vhost-user-gpu.log")
> - self._vug_log_file = open(self._vug_log_path, "wb")
> - self.log.info('Complete vhost-user-gpu.log file can be '
> - 'found at %s', self._vug_log_path)
> -
> - vugp = subprocess.Popen(
> - [vug, "--virgl", "--fd=%d" % vug_sock.fileno()],
> - stdin=subprocess.DEVNULL,
> - stdout=self._vug_log_file,
> - stderr=subprocess.STDOUT,
> - shell=False,
> - close_fds=False,
> - )
> - self._vug_log_file.close()
> + vug_log_path = self.log_file("vhost-user-gpu.log")
> + self.log.info('Complete vhost-user-gpu.log file can be found at %s',
> + vug_log_path)
> + with open(vug_log_path, "wb") as vug_log_file:
> + # pylint: disable=consider-using-with
> + vugp = subprocess.Popen(
> + [vug, "--virgl", f"--fd={vug_sock.fileno()}"],
> + stdin=subprocess.DEVNULL,
> + stdout=vug_log_file,
> + stderr=subprocess.STDOUT,
> + shell=False,
> + close_fds=False,
> + )
Let's use with for subproces.Popen() too.
You may leave vug_log_file.close() after subprocess.Popen(); if
subprocess.Popen() raises an exception, the with statement for
vug_log_file will close the file. Otherwise, the vug_log_file.close()
after subprocess.Popen() will close it and the with statement for
vug_log_file will be no-op; the documentation says it is allowed to call
IOBase.call() twice:
https://docs.python.org/3.14/library/io.html#io.IOBase.close
>
> self.vm.set_console()
> self.vm.add_args("-cpu", "host")
> self.vm.add_args("-m", "2G")
> self.vm.add_args("-object", "memory-backend-memfd,id=mem,size=2G")
> self.vm.add_args("-machine", "pc,memory-backend=mem,accel=kvm")
> - self.vm.add_args("-chardev", "socket,id=vug,fd=%d" % qemu_sock.fileno())
> + self.vm.add_args("-chardev", f"socket,id=vug,fd={qemu_sock.fileno()}")
> self.vm.add_args("-device", "vhost-user-vga,chardev=vug")
> self.vm.add_args("-display", "egl-headless")
> self.vm.add_args(
> @@ -128,7 +128,7 @@ def test_vhost_user_vga_virgl(self):
> )
> try:
> self.vm.launch()
> - except:
> + except VMLaunchFailure:
> # TODO: probably fails because we are missing the VirGL features
> self.skipTest("VirGL not enabled?")
> self.wait_for_console_pattern("as init process")
On 06/11/2025 03.34, Akihiko Odaki wrote:
> On 2025/11/05 21:09, Thomas Huth wrote:
>> From: Thomas Huth <thuth@redhat.com>
>>
>> Use the recommended order for import statements, specify the kind of
>> exceptions that we try to catch, use f-strings where it makes sense,
>> rewrite the vug_log_file part with a proper "with" statement and
>> fix some FIXMEs by checking for the availability of the devices, etc.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> tests/functional/x86_64/test_virtio_gpu.py | 46 +++++++++++-----------
>> 1 file changed, 23 insertions(+), 23 deletions(-)
>>
>> diff --git a/tests/functional/x86_64/test_virtio_gpu.py b/tests/
>> functional/x86_64/test_virtio_gpu.py
>> index be96de24da2..a25f15cdb00 100755
>> --- a/tests/functional/x86_64/test_virtio_gpu.py
>> +++ b/tests/functional/x86_64/test_virtio_gpu.py
>> @@ -5,22 +5,23 @@
>> # 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
>> +import socket
>> +import subprocess
>> from qemu_test import QemuSystemTest, Asset
>> from qemu_test import wait_for_console_pattern
>> from qemu_test import exec_command_and_wait_for_pattern
>> from qemu_test import is_readable_executable_file
>> -
>> -import os
>> -import socket
>> -import subprocess
>> +from qemu.machine.machine import VMLaunchFailure
>> def pick_default_vug_bin(test):
>> bld_dir_path = test.build_file("contrib", "vhost-user-gpu", "vhost-
>> user-gpu")
>> if is_readable_executable_file(bld_dir_path):
>> return bld_dir_path
>> + return None
>> class VirtioGPUx86(QemuSystemTest):
>> @@ -46,8 +47,8 @@ def wait_for_console_pattern(self, success_message,
>> vm=None):
>> )
>> def test_virtio_vga_virgl(self):
>> - # FIXME: should check presence of virtio, virgl etc
>> self.require_accelerator('kvm')
>> + self.require_device('virtio-vga-gl')
>> kernel_path = self.ASSET_KERNEL.fetch()
>> initrd_path = self.ASSET_INITRD.fetch()
>> @@ -68,7 +69,7 @@ def test_virtio_vga_virgl(self):
>> )
>> try:
>> self.vm.launch()
>> - except:
>> + except VMLaunchFailure:
>> # TODO: probably fails because we are missing the VirGL
>> features
>> self.skipTest("VirGL not enabled?")
>> @@ -78,8 +79,8 @@ def test_virtio_vga_virgl(self):
>> )
>> def test_vhost_user_vga_virgl(self):
>> - # FIXME: should check presence of vhost-user-gpu, virgl, memfd etc
>> self.require_accelerator('kvm')
>> + self.require_device('vhost-user-vga')
>> vug = pick_default_vug_bin(self)
>> if not vug:
>> @@ -95,27 +96,26 @@ def test_vhost_user_vga_virgl(self):
>> os.set_inheritable(qemu_sock.fileno(), True)
>> os.set_inheritable(vug_sock.fileno(), True)
>> - self._vug_log_path = self.log_file("vhost-user-gpu.log")
>> - self._vug_log_file = open(self._vug_log_path, "wb")
>> - self.log.info('Complete vhost-user-gpu.log file can be '
>> - 'found at %s', self._vug_log_path)
>> -
>> - vugp = subprocess.Popen(
>> - [vug, "--virgl", "--fd=%d" % vug_sock.fileno()],
>> - stdin=subprocess.DEVNULL,
>> - stdout=self._vug_log_file,
>> - stderr=subprocess.STDOUT,
>> - shell=False,
>> - close_fds=False,
>> - )
>> - self._vug_log_file.close()
>> + vug_log_path = self.log_file("vhost-user-gpu.log")
>> + self.log.info('Complete vhost-user-gpu.log file can be found at %s',
>> + vug_log_path)
>> + with open(vug_log_path, "wb") as vug_log_file:
>> + # pylint: disable=consider-using-with
>> + vugp = subprocess.Popen(
>> + [vug, "--virgl", f"--fd={vug_sock.fileno()}"],
>> + stdin=subprocess.DEVNULL,
>> + stdout=vug_log_file,
>> + stderr=subprocess.STDOUT,
>> + shell=False,
>> + close_fds=False,
>> + )
>
> Let's use with for subproces.Popen() too.
>
> You may leave vug_log_file.close() after subprocess.Popen(); if
> subprocess.Popen() raises an exception, the with statement for vug_log_file
> will close the file. Otherwise, the vug_log_file.close() after
> subprocess.Popen() will close it and the with statement for vug_log_file
> will be no-op; the documentation says it is allowed to call IOBase.call()
> twice:
> https://docs.python.org/3.14/library/io.html#io.IOBase.close
Not quite sure whether I got your point, but the "vugp" variable is still
used later in the function to terminate the subprocess manually. So if we
really want to use a "when" here, I guess I have to move the whole remaining
part of this function into the "when" block, don't I? Is that what you are
suggesting?
Thomas
>> self.vm.set_console()
>> self.vm.add_args("-cpu", "host")
>> self.vm.add_args("-m", "2G")
>> self.vm.add_args("-object", "memory-backend-memfd,id=mem,size=2G")
>> self.vm.add_args("-machine", "pc,memory-backend=mem,accel=kvm")
>> - self.vm.add_args("-chardev", "socket,id=vug,fd=%d" %
>> qemu_sock.fileno())
>> + self.vm.add_args("-chardev",
>> f"socket,id=vug,fd={qemu_sock.fileno()}")
>> self.vm.add_args("-device", "vhost-user-vga,chardev=vug")
>> self.vm.add_args("-display", "egl-headless")
>> self.vm.add_args(
>> @@ -128,7 +128,7 @@ def test_vhost_user_vga_virgl(self):
>> )
>> try:
>> self.vm.launch()
>> - except:
>> + except VMLaunchFailure:
>> # TODO: probably fails because we are missing the VirGL
>> features
>> self.skipTest("VirGL not enabled?")
>> self.wait_for_console_pattern("as init process")
>
On 2025/11/06 15:30, Thomas Huth wrote:
> On 06/11/2025 03.34, Akihiko Odaki wrote:
>> On 2025/11/05 21:09, Thomas Huth wrote:
>>> From: Thomas Huth <thuth@redhat.com>
>>>
>>> Use the recommended order for import statements, specify the kind of
>>> exceptions that we try to catch, use f-strings where it makes sense,
>>> rewrite the vug_log_file part with a proper "with" statement and
>>> fix some FIXMEs by checking for the availability of the devices, etc.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>> tests/functional/x86_64/test_virtio_gpu.py | 46 +++++++++++-----------
>>> 1 file changed, 23 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/tests/functional/x86_64/test_virtio_gpu.py b/tests/
>>> functional/x86_64/test_virtio_gpu.py
>>> index be96de24da2..a25f15cdb00 100755
>>> --- a/tests/functional/x86_64/test_virtio_gpu.py
>>> +++ b/tests/functional/x86_64/test_virtio_gpu.py
>>> @@ -5,22 +5,23 @@
>>> # 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
>>> +import socket
>>> +import subprocess
>>> from qemu_test import QemuSystemTest, Asset
>>> from qemu_test import wait_for_console_pattern
>>> from qemu_test import exec_command_and_wait_for_pattern
>>> from qemu_test import is_readable_executable_file
>>> -
>>> -import os
>>> -import socket
>>> -import subprocess
>>> +from qemu.machine.machine import VMLaunchFailure
>>> def pick_default_vug_bin(test):
>>> bld_dir_path = test.build_file("contrib", "vhost-user-gpu",
>>> "vhost- user-gpu")
>>> if is_readable_executable_file(bld_dir_path):
>>> return bld_dir_path
>>> + return None
>>> class VirtioGPUx86(QemuSystemTest):
>>> @@ -46,8 +47,8 @@ def wait_for_console_pattern(self, success_message,
>>> vm=None):
>>> )
>>> def test_virtio_vga_virgl(self):
>>> - # FIXME: should check presence of virtio, virgl etc
>>> self.require_accelerator('kvm')
>>> + self.require_device('virtio-vga-gl')
>>> kernel_path = self.ASSET_KERNEL.fetch()
>>> initrd_path = self.ASSET_INITRD.fetch()
>>> @@ -68,7 +69,7 @@ def test_virtio_vga_virgl(self):
>>> )
>>> try:
>>> self.vm.launch()
>>> - except:
>>> + except VMLaunchFailure:
>>> # TODO: probably fails because we are missing the VirGL
>>> features
>>> self.skipTest("VirGL not enabled?")
>>> @@ -78,8 +79,8 @@ def test_virtio_vga_virgl(self):
>>> )
>>> def test_vhost_user_vga_virgl(self):
>>> - # FIXME: should check presence of vhost-user-gpu, virgl,
>>> memfd etc
>>> self.require_accelerator('kvm')
>>> + self.require_device('vhost-user-vga')
>>> vug = pick_default_vug_bin(self)
>>> if not vug:
>>> @@ -95,27 +96,26 @@ def test_vhost_user_vga_virgl(self):
>>> os.set_inheritable(qemu_sock.fileno(), True)
>>> os.set_inheritable(vug_sock.fileno(), True)
>>> - self._vug_log_path = self.log_file("vhost-user-gpu.log")
>>> - self._vug_log_file = open(self._vug_log_path, "wb")
>>> - self.log.info('Complete vhost-user-gpu.log file can be '
>>> - 'found at %s', self._vug_log_path)
>>> -
>>> - vugp = subprocess.Popen(
>>> - [vug, "--virgl", "--fd=%d" % vug_sock.fileno()],
>>> - stdin=subprocess.DEVNULL,
>>> - stdout=self._vug_log_file,
>>> - stderr=subprocess.STDOUT,
>>> - shell=False,
>>> - close_fds=False,
>>> - )
>>> - self._vug_log_file.close()
>>> + vug_log_path = self.log_file("vhost-user-gpu.log")
>>> + self.log.info('Complete vhost-user-gpu.log file can be found
>>> at %s',
>>> + vug_log_path)
>>> + with open(vug_log_path, "wb") as vug_log_file:
>>> + # pylint: disable=consider-using-with
>>> + vugp = subprocess.Popen(
>>> + [vug, "--virgl", f"--fd={vug_sock.fileno()}"],
>>> + stdin=subprocess.DEVNULL,
>>> + stdout=vug_log_file,
>>> + stderr=subprocess.STDOUT,
>>> + shell=False,
>>> + close_fds=False,
>>> + )
>>
>> Let's use with for subproces.Popen() too.
>>
>> You may leave vug_log_file.close() after subprocess.Popen(); if
>> subprocess.Popen() raises an exception, the with statement for
>> vug_log_file will close the file. Otherwise, the vug_log_file.close()
>> after subprocess.Popen() will close it and the with statement for
>> vug_log_file will be no-op; the documentation says it is allowed to
>> call IOBase.call() twice:
>> https://docs.python.org/3.14/library/io.html#io.IOBase.close
>
> Not quite sure whether I got your point, but the "vugp" variable is
> still used later in the function to terminate the subprocess manually.
> So if we really want to use a "when" here, I guess I have to move the
> whole remaining part of this function into the "when" block, don't I? Is
> that what you are suggesting?
I suppose you meant "with" instead of "when"; if so, yes, that's what I
meant.
Regards,
Akihiko Odaki
On 07/11/2025 02.49, Akihiko Odaki wrote:
> On 2025/11/06 15:30, Thomas Huth wrote:
>> On 06/11/2025 03.34, Akihiko Odaki wrote:
>>> On 2025/11/05 21:09, Thomas Huth wrote:
>>>> From: Thomas Huth <thuth@redhat.com>
>>>>
>>>> Use the recommended order for import statements, specify the kind of
>>>> exceptions that we try to catch, use f-strings where it makes sense,
>>>> rewrite the vug_log_file part with a proper "with" statement and
>>>> fix some FIXMEs by checking for the availability of the devices, etc.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>> tests/functional/x86_64/test_virtio_gpu.py | 46 +++++++++++-----------
>>>> 1 file changed, 23 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/tests/functional/x86_64/test_virtio_gpu.py b/tests/
>>>> functional/x86_64/test_virtio_gpu.py
>>>> index be96de24da2..a25f15cdb00 100755
>>>> --- a/tests/functional/x86_64/test_virtio_gpu.py
>>>> +++ b/tests/functional/x86_64/test_virtio_gpu.py
>>>> @@ -5,22 +5,23 @@
>>>> # 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
>>>> +import socket
>>>> +import subprocess
>>>> from qemu_test import QemuSystemTest, Asset
>>>> from qemu_test import wait_for_console_pattern
>>>> from qemu_test import exec_command_and_wait_for_pattern
>>>> from qemu_test import is_readable_executable_file
>>>> -
>>>> -import os
>>>> -import socket
>>>> -import subprocess
>>>> +from qemu.machine.machine import VMLaunchFailure
>>>> def pick_default_vug_bin(test):
>>>> bld_dir_path = test.build_file("contrib", "vhost-user-gpu",
>>>> "vhost- user-gpu")
>>>> if is_readable_executable_file(bld_dir_path):
>>>> return bld_dir_path
>>>> + return None
>>>> class VirtioGPUx86(QemuSystemTest):
>>>> @@ -46,8 +47,8 @@ def wait_for_console_pattern(self, success_message,
>>>> vm=None):
>>>> )
>>>> def test_virtio_vga_virgl(self):
>>>> - # FIXME: should check presence of virtio, virgl etc
>>>> self.require_accelerator('kvm')
>>>> + self.require_device('virtio-vga-gl')
>>>> kernel_path = self.ASSET_KERNEL.fetch()
>>>> initrd_path = self.ASSET_INITRD.fetch()
>>>> @@ -68,7 +69,7 @@ def test_virtio_vga_virgl(self):
>>>> )
>>>> try:
>>>> self.vm.launch()
>>>> - except:
>>>> + except VMLaunchFailure:
>>>> # TODO: probably fails because we are missing the VirGL
>>>> features
>>>> self.skipTest("VirGL not enabled?")
>>>> @@ -78,8 +79,8 @@ def test_virtio_vga_virgl(self):
>>>> )
>>>> def test_vhost_user_vga_virgl(self):
>>>> - # FIXME: should check presence of vhost-user-gpu, virgl, memfd etc
>>>> self.require_accelerator('kvm')
>>>> + self.require_device('vhost-user-vga')
>>>> vug = pick_default_vug_bin(self)
>>>> if not vug:
>>>> @@ -95,27 +96,26 @@ def test_vhost_user_vga_virgl(self):
>>>> os.set_inheritable(qemu_sock.fileno(), True)
>>>> os.set_inheritable(vug_sock.fileno(), True)
>>>> - self._vug_log_path = self.log_file("vhost-user-gpu.log")
>>>> - self._vug_log_file = open(self._vug_log_path, "wb")
>>>> - self.log.info('Complete vhost-user-gpu.log file can be '
>>>> - 'found at %s', self._vug_log_path)
>>>> -
>>>> - vugp = subprocess.Popen(
>>>> - [vug, "--virgl", "--fd=%d" % vug_sock.fileno()],
>>>> - stdin=subprocess.DEVNULL,
>>>> - stdout=self._vug_log_file,
>>>> - stderr=subprocess.STDOUT,
>>>> - shell=False,
>>>> - close_fds=False,
>>>> - )
>>>> - self._vug_log_file.close()
>>>> + vug_log_path = self.log_file("vhost-user-gpu.log")
>>>> + self.log.info('Complete vhost-user-gpu.log file can be found at
>>>> %s',
>>>> + vug_log_path)
>>>> + with open(vug_log_path, "wb") as vug_log_file:
>>>> + # pylint: disable=consider-using-with
>>>> + vugp = subprocess.Popen(
>>>> + [vug, "--virgl", f"--fd={vug_sock.fileno()}"],
>>>> + stdin=subprocess.DEVNULL,
>>>> + stdout=vug_log_file,
>>>> + stderr=subprocess.STDOUT,
>>>> + shell=False,
>>>> + close_fds=False,
>>>> + )
>>>
>>> Let's use with for subproces.Popen() too.
>>>
>>> You may leave vug_log_file.close() after subprocess.Popen(); if
>>> subprocess.Popen() raises an exception, the with statement for
>>> vug_log_file will close the file. Otherwise, the vug_log_file.close()
>>> after subprocess.Popen() will close it and the with statement for
>>> vug_log_file will be no-op; the documentation says it is allowed to call
>>> IOBase.call() twice:
>>> https://docs.python.org/3.14/library/io.html#io.IOBase.close
>>
>> Not quite sure whether I got your point, but the "vugp" variable is still
>> used later in the function to terminate the subprocess manually. So if we
>> really want to use a "when" here, I guess I have to move the whole
>> remaining part of this function into the "when" block, don't I? Is that
>> what you are suggesting?
>
> I suppose you meant "with" instead of "when"; if so, yes, that's what I meant.
D'oh, of course I meant "with", not "when" ... must have been too tired
yesterday to type properly, sorry!
Thomas
© 2016 - 2025 Red Hat, Inc.