Re: [PATCH v2] qemu: Allow @rednernode for virgl domains

[Marc-André Lureau <mlureau@xxxxxxxxxx>]

Hi


Fix the title @rednernode -> @rendernode

----- Original Message -----
> When enabling virgl, qemu opens /dev/dri/render*. So far, we are
> not allowing that in devices cgroup nor creating the file in
> domain's namespace and thus requiring users to set the paths in
> qemu.conf. This, however, is suboptimal as it allows access to
> ALL qemu processes even those which don't have virgl configured.
> Now that we have a way to specify render node that qemu will use
> we can be more cautious and enable just that.
> 
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> 
> Technically, this is v2 of:
> 
> https://www.redhat.com/archives/libvir-list/2017-February/msg00497.html
> 
> diff to v1:
> - now that we have @rendernode for <gl/> which selects just one path (and
> does
>   it in predictable fashion) only that path is enabled in the CGgroups and
>   created in the namespace.

That means in practice we are not compatible with older qemu releases, and we make rendernode attribute somehow mandatory for qemu:///system (no automatic selection).

I'd suggest to let all /dev/dri/render* if rendernode is not specified, but this can be discussed and done in a seperate patch.

> 
>  src/qemu/qemu_cgroup.c | 27 +++++++++++++++++++++++++++
>  src/qemu/qemu_domain.c | 39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index 209cbc275..f0729743a 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -335,6 +335,28 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm,
>      return ret;
>  }
>  
> +
> +static int
> +qemuSetupGraphicsCgroup(virDomainObjPtr vm,
> +                        virDomainGraphicsDefPtr gfx)
> +{
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    const char *rendernode = gfx->data.spice.rendernode;
> +    int ret;
> +
> +    if (gfx->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE ||
> +        gfx->data.spice.gl != VIR_TRISTATE_BOOL_YES ||
> +        !rendernode)
> +        return 0;
> +
> +    ret = virCgroupAllowDevicePath(priv->cgroup, rendernode,
> +                                   VIR_CGROUP_DEVICE_RW, false);
> +    virDomainAuditCgroupPath(vm, priv->cgroup, "allow", rendernode,
> +                             "rw", ret == 0);
> +    return ret;
> +}
> +
> +
>  static int
>  qemuSetupBlkioCgroup(virDomainObjPtr vm)
>  {
> @@ -604,6 +626,11 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver,
>              goto cleanup;
>      }
>  
> +    for (i = 0; i < vm->def->ngraphics; i++) {
> +        if (qemuSetupGraphicsCgroup(vm, vm->def->graphics[i]) < 0)
> +            goto cleanup;
> +    }
> +
>      for (i = 0; i < vm->def->ninputs; i++) {
>          if (qemuSetupInputCgroup(vm, vm->def->inputs[i]) < 0)
>              goto cleanup;
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 92a9a105c..ea4b28288 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -7526,6 +7526,42 @@ qemuDomainSetupTPM(virQEMUDriverPtr driver
> ATTRIBUTE_UNUSED,
>  }
>  
>  
> +static int
> +qemuDomainSetupGraphics(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
> +                        virDomainGraphicsDefPtr gfx,
> +                        const char *devPath)
> +{
> +    const char *rendernode = gfx->data.spice.rendernode;
> +
> +    if (gfx->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE ||
> +        gfx->data.spice.gl != VIR_TRISTATE_BOOL_YES ||
> +        !rendernode)
> +        return 0;
> +
> +    return qemuDomainCreateDevice(rendernode, devPath, false);
> +}
> +
> +
> +static int
> +qemuDomainSetupAllGraphics(virQEMUDriverPtr driver,
> +                           virDomainObjPtr vm,
> +                           const char *devPath)
> +{
> +    size_t i;
> +
> +    VIR_DEBUG("Setting up graphics");
> +    for (i = 0; i < vm->def->ngraphics; i++) {
> +        if (qemuDomainSetupGraphics(driver,
> +                                    vm->def->graphics[i],
> +                                    devPath) < 0)
> +            return -1;
> +    }
> +
> +    VIR_DEBUG("Setup all graphics");
> +    return 0;
> +}
> +
> +
>  static int
>  qemuDomainSetupInput(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
>                       virDomainInputDefPtr input,
> @@ -7679,6 +7715,9 @@ qemuDomainBuildNamespace(virQEMUDriverPtr driver,
>      if (qemuDomainSetupTPM(driver, vm, devPath) < 0)
>          goto cleanup;
>  
> +    if (qemuDomainSetupAllGraphics(driver, vm, devPath) < 0)
> +        goto cleanup;
> +
>      if (qemuDomainSetupAllInputs(driver, vm, devPath) < 0)
>          goto cleanup;
>  

Looks good,

Reviewed-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>

> --
> 2.11.0
> 
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list