Re: [PATCH v4 2/3] ui/console: Introduce dpy_gl_dmabuf_get_fd() helper

[Marc-André Lureau]

Hi

On Fri, Mar 22, 2024 at 3:45 AM <dongwon.kim@intel.com> wrote:
>
> From: Dongwon Kim <dongwon.kim@intel.com>
>
> dpy_gl_dmabuf_get_fd() is a helper for retrieving FD of the dmabuf
> from QemuDmaBuf struct.
>

Same remarks as previous patch regarding exhaustivity, setter,
accepting NULL argument etc.

> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> ---
>  include/ui/console.h | 1 +
>  hw/vfio/display.c    | 8 +++++++-
>  ui/console.c         | 9 +++++++++
>  3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 6064487fc4..d5334a806c 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -360,6 +360,7 @@ void dpy_gl_cursor_position(QemuConsole *con,
>                              uint32_t pos_x, uint32_t pos_y);
>  uint32_t dpy_gl_dmabuf_get_width(QemuDmaBuf *dmabuf);
>  uint32_t dpy_gl_dmabuf_get_height(QemuDmaBuf *dmabuf);
> +int32_t dpy_gl_dmabuf_get_fd(QemuDmaBuf *dmabuf);
>  void dpy_gl_release_dmabuf(QemuConsole *con,
>                             QemuDmaBuf *dmabuf);
>  void dpy_gl_update(QemuConsole *con,
> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> index c962e5f88f..676b2fc5f3 100644
> --- a/hw/vfio/display.c
> +++ b/hw/vfio/display.c
> @@ -259,9 +259,15 @@ static VFIODMABuf *vfio_display_get_dmabuf(VFIOPCIDevice 
> *vdev,
>
>  static void vfio_display_free_one_dmabuf(VFIODisplay *dpy, VFIODMABuf 
> *dmabuf)
>  {
> +    int fd;
> +
>      QTAILQ_REMOVE(&dpy->dmabuf.bufs, dmabuf, next);
> +    fd = dpy_gl_dmabuf_get_fd(&dmabuf->buf);
> +    if (fd > -1) {
> +        close(fd);
> +    }

The current code assumes that fd is valid. Why should we change that?


> +
>      dpy_gl_release_dmabuf(dpy->con, &dmabuf->buf);
> -    close(dmabuf->buf.fd);

>      g_free(dmabuf);
>  }
>
> diff --git a/ui/console.c b/ui/console.c
> index 1d0513a733..69560aac7e 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1504,6 +1504,15 @@ int qemu_console_get_height(QemuConsole *con, int 
> fallback)
>      }
>  }
>
> +int32_t dpy_gl_dmabuf_get_fd(QemuDmaBuf *dmabuf)
> +{
> +    if (dmabuf) {
> +        return dmabuf->fd;
> +    }
> +
> +    return -1;
> +}
> +
>  int qemu_invalidate_text_consoles(void)
>  {
>      QemuConsole *s;
> --
> 2.34.1
>
>


--
Marc-André Lureau