Unify error checking style for virtio_gpu_create_mapping_iov() across the
codebase to improve consistency and readability.
virtio_gpu_create_mapping_iov() returns 0 on success and negative values
on error. The original code used inconsistent patterns for checking errors:
- Some used 'if (ret != 0)'
- Some used 'CHECK(!ret, cmd)'
- Some used 'CHECK(!result, cmd)'
Change all checks to use the consistent 'if (ret < 0)' or 'CHECK(ret >= 0)'
patterns, following the preferred QEMU coding convention for functions that
return 0 on success and negative on error. This makes the return value
convention immediately clear to code readers without needing to look up
the function definition.
Updated locations:
- hw/display/virtio-gpu-virgl.c: virgl_resource_attach_backing()
- hw/display/virtio-gpu.c: virtio_gpu_resource_create_blob()
- hw/display/virtio-gpu.c: virtio_gpu_resource_attach_backing()
- hw/display/virtio-gpu-rutabaga.c: rutabaga_cmd_attach_backing()
- hw/display/virtio-gpu-rutabaga.c: rutabaga_cmd_resource_create_blob()
Signed-off-by: Honglei Huang <honghuan@amd.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
hw/display/virtio-gpu-rutabaga.c | 4 ++--
hw/display/virtio-gpu-virgl.c | 4 ++--
hw/display/virtio-gpu.c | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
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);
vecs.iovecs = res->iov;
vecs.num_iovecs = res->iov_cnt;
@@ -616,7 +616,7 @@ rutabaga_cmd_resource_create_blob(VirtIOGPU *g,
result = virtio_gpu_create_mapping_iov(g, cblob.nr_entries,
sizeof(cblob), cmd, &res->addrs,
&res->iov, &res->iov_cnt);
- CHECK(!result, cmd);
+ CHECK(result >= 0, cmd);
}
rc_blob.blob_id = cblob.blob_id;
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index e60e1059df..6ebd9293e5 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -557,7 +557,7 @@ static void virgl_resource_attach_backing(VirtIOGPU *g,
ret = virtio_gpu_create_mapping_iov(g, att_rb.nr_entries, sizeof(att_rb),
cmd, NULL, &res_iovs, &res_niov);
- if (ret != 0) {
+ if (ret < 0) {
cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
return;
}
@@ -701,7 +701,7 @@ static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob),
cmd, &res->base.addrs,
&res->base.iov, &res->base.iov_cnt);
- if (ret != 0) {
+ if (ret < 0) {
cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
return;
}
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 0a1a625b0e..1038c6a49f 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -352,7 +352,7 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob),
cmd, &res->addrs, &res->iov,
&res->iov_cnt);
- if (ret != 0) {
+ if (ret < 0) {
cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
g_free(res);
return;
@@ -931,7 +931,7 @@ virtio_gpu_resource_attach_backing(VirtIOGPU *g,
ret = virtio_gpu_create_mapping_iov(g, ab.nr_entries, sizeof(ab), cmd,
&res->addrs, &res->iov, &res->iov_cnt);
- if (ret != 0) {
+ if (ret < 0) {
cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
return;
}
--
2.34.1On 11/24/25 06:24, 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); > > vecs.iovecs = res->iov; > vecs.num_iovecs = res->iov_cnt; > @@ -616,7 +616,7 @@ rutabaga_cmd_resource_create_blob(VirtIOGPU *g, > result = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, > sizeof(cblob), cmd, &res->addrs, > &res->iov, &res->iov_cnt); > - CHECK(!result, cmd); > + CHECK(result >= 0, cmd); > } The ret < 0 part looks okay, but ret >= 0 feels dubious to me given that this func doesn't return positive values. -- Best regards, Dmitry
On 2025/11/25 05:31, Dmitry Osipenko wrote: > On 11/24/25 06:24, 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); >> >> vecs.iovecs = res->iov; >> vecs.num_iovecs = res->iov_cnt; >> @@ -616,7 +616,7 @@ rutabaga_cmd_resource_create_blob(VirtIOGPU *g, >> result = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, >> sizeof(cblob), cmd, &res->addrs, >> &res->iov, &res->iov_cnt); >> - CHECK(!result, cmd); >> + CHECK(result >= 0, cmd); >> } > > The ret < 0 part looks okay, but ret >= 0 feels dubious to me given that > this func doesn't return positive values. > Will remove the ret >= 0 check according to your requirements in V6.
On 2025/11/25 05:31, Dmitry Osipenko wrote: > On 11/24/25 06:24, 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); >> >> vecs.iovecs = res->iov; >> vecs.num_iovecs = res->iov_cnt; >> @@ -616,7 +616,7 @@ rutabaga_cmd_resource_create_blob(VirtIOGPU *g, >> result = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, >> sizeof(cblob), cmd, &res->addrs, >> &res->iov, &res->iov_cnt); >> - CHECK(!result, cmd); >> + CHECK(result >= 0, cmd); >> } > > The ret < 0 part looks okay, but ret >= 0 feels dubious to me given that > this func doesn't return positive values. > Really thanks for the review. How about split patch 1 and patch 2 into different threads, for the error checking style I will submit a another thread, maybe change the virtio_gpu_create_mapping_iov return value to true or false is a better choice?
© 2016 - 2026 Red Hat, Inc.