Re: [RFC PATCH v4 4/7] virtio-gpu: Make virtio_gpu_virgl_unmap_resource_blob() return -1 on error

["Alex Bennée" <alex.bennee@linaro.org>]

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

> Make virtio_gpu_virgl_unmap_resource_blob() return -1 for more consistency
> of error propagation style in the code, adhering to QEMU's devel/style
> recommendations and preparing code for further code changes utilizing this
> function.

I'm not so sure of that. If the function is a pass/fail then we tend to
prefer using bools in newer code. If you need richer internal error
reporting then start using your errno defines. If this is user visible
(i.e. during configuration) then we can make more of Error* and friends.

>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> ---
>  hw/display/virtio-gpu-virgl.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index edcdad0af232..a7b14a312c83 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -199,7 +199,7 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
>              qemu_log_mask(LOG_GUEST_ERROR,
>                            "%s: failed to unmap virgl resource: %s\n",
>                            __func__, strerror(-ret));
> -            return ret;
> +            return -1;
>          }
>      } else {
>          *cmd_suspended = true;
> @@ -333,7 +333,7 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
>      }
>  
>  #if VIRGL_VERSION_MAJOR >= 1
> -    if (virtio_gpu_virgl_unmap_resource_blob(g, res, cmd_suspended)) {
> +    if (virtio_gpu_virgl_unmap_resource_blob(g, res, cmd_suspended) < 0) {
>          cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
>          return;
>      }
> @@ -829,7 +829,7 @@ static void virgl_cmd_resource_unmap_blob(VirtIOGPU *g,
>      }
>  
>      ret = virtio_gpu_virgl_unmap_resource_blob(g, res, cmd_suspended);
> -    if (ret) {
> +    if (ret < 0) {
>          cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
>          return;
>      }

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro