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@redhat.com> ---

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@redhat.com>

-- 2.11.0