Print out error messages when virgl fence creation fails to aid debugging
of the fence-related bugs.

Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 hw/display/virtio-gpu-virgl.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index eedae7357f1a..3c564683820b 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -892,6 +892,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
                                       struct virtio_gpu_ctrl_command *cmd)
 {
     bool cmd_suspended = false;
+    int ret;
 
     VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
 
@@ -990,7 +991,16 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
     }
 
     trace_virtio_gpu_fence_ctrl(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
-    virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
+
+    /*
+     * Unlike other virglrenderer functions, this one returns a positive
+     * error code.
+     */
+    ret = virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, 0);
+    if (ret)
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: virgl_renderer_create_fence error: %s",
+                      __func__, strerror(ret));
 }
 
 static void virgl_write_fence(void *opaque, uint32_t fence)
-- 
2.47.0

Dmitry Osipenko <dmitry.osipenko@collabora.com> writes:

> Print out error messages when virgl fence creation fails to aid debugging
> of the fence-related bugs.
>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> ---
>  hw/display/virtio-gpu-virgl.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index eedae7357f1a..3c564683820b 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -892,6 +892,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>                                        struct virtio_gpu_ctrl_command *cmd)
>  {
>      bool cmd_suspended = false;
> +    int ret;
>  
>      VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
>  
> @@ -990,7 +991,16 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>      }
>  
>      trace_virtio_gpu_fence_ctrl(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
> -    virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
> +
> +    /*
> +     * Unlike other virglrenderer functions, this one returns a positive
> +     * error code.
> +     */
> +    ret = virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, 0);
> +    if (ret)
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: virgl_renderer_create_fence error: %s",
> +                      __func__, strerror(ret));

braces please. Also if ret is only used for the if leg you might as well
combine it:

  if (virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, 0) != 0) {
    qemu_log_mask(...) 
  }

>  }
>  
>  static void virgl_write_fence(void *opaque, uint32_t fence)

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

On 10/31/24 13:21, Alex Bennée wrote:
> Dmitry Osipenko <dmitry.osipenko@collabora.com> writes:
> 
>> Print out error messages when virgl fence creation fails to aid debugging
>> of the fence-related bugs.
>>
>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>> ---
>>  hw/display/virtio-gpu-virgl.c | 12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
>> index eedae7357f1a..3c564683820b 100644
>> --- a/hw/display/virtio-gpu-virgl.c
>> +++ b/hw/display/virtio-gpu-virgl.c
>> @@ -892,6 +892,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>>                                        struct virtio_gpu_ctrl_command *cmd)
>>  {
>>      bool cmd_suspended = false;
>> +    int ret;
>>  
>>      VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
>>  
>> @@ -990,7 +991,16 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>>      }
>>  
>>      trace_virtio_gpu_fence_ctrl(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
>> -    virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
>> +
>> +    /*
>> +     * Unlike other virglrenderer functions, this one returns a positive
>> +     * error code.
>> +     */
>> +    ret = virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, 0);
>> +    if (ret)
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "%s: virgl_renderer_create_fence error: %s",
>> +                      __func__, strerror(ret));
> 
> braces please. Also if ret is only used for the if leg you might as well
> combine it:
> 
>   if (virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, 0) != 0) {
>     qemu_log_mask(...) 
>   }

Ack, wish checkpatch warned about braces

-- 
Best regards,
Dmitry