On 2025/11/20 03:16, Dmitry Osipenko wrote:
On 11/18/25 15:32, Honglei Huang wrote:
On 2025/11/18 09:48, Dmitry Osipenko wrote:
On 11/17/25 16:22, Markus Armbruster wrote:
Dmitry Osipenko <[email protected]> writes:
On 11/17/25 13:51, Honglei Huang wrote:
diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-
gpu-rutabaga.c
index ed5ae52acb..ea2928b706 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -466,7 +466,7 @@ rutabaga_cmd_attach_backing(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd)
ret = virtio_gpu_create_mapping_iov(g, att_rb.nr_entries,
sizeof(att_rb),
cmd, NULL, &res->iov,
&res->iov_cnt);
- CHECK(!ret, cmd);
+ CHECK(ret >= 0, cmd);
virtio_gpu_create_mapping_iov() doesn't return positive values, don't
see how this change improves anything. You now saying that ret > 0 is
okay, while it shall never happen.
Please see
Subject: Re: [PATCH] virtio-gpu-virgl: fix error handling in
virgl_cmd_resource_create_blob
Date: Mon, 17 Nov 2025 08:49:42 +0100
Message-ID: <[email protected]>
https://lore.kernel.org/qemu-devel/[email protected]/
It's a rather common bug when errno isn't negated by mistake and a
positive error code is returned. Ignoring positive values when they
aren't expected opens door to unnecessary problems, IMO.
How about apply the v2 or v3 firstly to fix the
virtio_gpu_create_mapping_iov() block issue in virtio-gpu?
I will create another thread for the `CHECK(!ret, cmd);` thing in rutabaga.
There was a precedent of virtio-gpu not handling positive error codes
properly [1]. To me there is no problem that needs to be fixed when
virtio_gpu_create_mapping_iov() is never expected to return positive
values and doesn't return them.
[1]
https://lore.kernel.org/qemu-devel/[email protected]/
It's a common expectation that errors are negative. But in practice it's
not always true, especially when interacting with external code.
Functionally this patch doesn't change anything. Will leave to Alex and
Akihiko to decide on it.
Hi Dmitry,
Totally agreed with you.
And actually the origin purpose of this patch is to fix virtio-gpu not
handling error codes properly. Please see the V2: [1].
The error handling logic was incorrect. virtio_gpu_create_mapping_iov()
returns 0 on success and non-zero on failure, but the code was checking
whether to set the error response.
The fix changes the condition from 'if (!ret)' to 'if (ret != 0)' to
properly handle the return value, consistent with other usage patterns
in the same codebase (see virtio-gpu.c:932 and virtio-gpu.c:354).
So I am suggesting to apply the V2.
[1]:
https://lore.kernel.org/qemu-devel/[email protected]/
v1:
https://lore.kernel.org/qemu-devel/[email protected]/
Regards,
Honglei
