[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] hw/display: fail early when multiple virgl devices are re
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v2] hw/display: fail early when multiple virgl devices are requested |
Date: |
Mon, 5 Jul 2021 16:53:14 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 |
On 7/5/21 1:08 PM, Marc-André Lureau wrote:
> Hi
>
> On Mon, Jul 5, 2021 at 3:03 PM Philippe Mathieu-Daudé <philmd@redhat.com
> <mailto:philmd@redhat.com>> wrote:
>
> On 7/5/21 12:42 PM, marcandre.lureau@redhat.com
> <mailto:marcandre.lureau@redhat.com> wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com
> <mailto:marcandre.lureau@redhat.com>>
> >
> > This avoids failing to initialize virgl and crashing later on, and
> clear
> > the user expectations.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com
> <mailto:marcandre.lureau@redhat.com>>
> > ---
> > hw/display/virtio-gpu-gl.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
> > index aea9700d5c..bc55c4767e 100644
> > --- a/hw/display/virtio-gpu-gl.c
> > +++ b/hw/display/virtio-gpu-gl.c
> > @@ -113,6 +113,11 @@ static void
> virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp)
> > return;
> > #endif
> >
> > + if (!object_resolve_path_type("", TYPE_VIRTIO_GPU_GL, NULL)) {
>
> Isn't the condition inverted?
>
>
> No, it's easy to misread though. It returns NULL if there are no or
> multiple instances.
>
> When realize() is reached the first time, we should have only one
> instance, and thus !NULL.
/**
* object_resolve_path_type:
* @path: the path to resolve
* @typename: the type to look for.
* @ambiguous: returns true if the path resolution failed because of an
* ambiguous match
*
* This is similar to object_resolve_path. However, when looking for a
* partial path only matches that implement the given type are considered.
...
*
* Returns: The matched object or NULL on path lookup failure.
*/
/**
* object_resolve_path:
...
* A successful result is only returned if
* only one match is found. If more than one match is found, a flag is
* returned to indicate that the match was ambiguous.
*
* Returns: The matched object or NULL on path lookup failure.
*/
OK... but kinda obfuscated.
Could we add some helper to simplify code review, such:
bool object_type_instance_is_singleton(const char *typename);
(or better name)?
> > + error_setg(errp, "at most one %s device is permitted",
> TYPE_VIRTIO_GPU_GL);
> > + return;
> > + }
> > +
> > if (!display_opengl) {
> > error_setg(errp, "opengl is not available");
> > return;
> >
>
>
>
>
> --
> Marc-André Lureau