[libvirt] [PATCH] qemu: Make sure permissions are set on VNC auto socket

[Martin Kletzander]

On Tue, Apr 28, 2015 at 08:13:19PM -0400, Cole Robinson wrote:
>This can cause permissions failures if qemu.conf user/group is changed.
>

I assume the issue only exists if the socket already exists, am I right?

>Fix this by filling in the VNC auto socket path in the same code area
>that we fill in default listen address, ports, etc.
>
>https://bugzilla.redhat.com/show_bug.cgi?id=1151762
>---
> src/qemu/qemu_command.c | 10 ++--------
> src/qemu/qemu_process.c | 11 ++++++++++-
> 2 files changed, 12 insertions(+), 9 deletions(-)
>
>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>index ba15dc9..45fc63c 100644
>--- a/src/qemu/qemu_command.c
>+++ b/src/qemu/qemu_command.c
>@@ -7544,7 +7544,6 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
> static int
> qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
>                                 virCommandPtr cmd,
>-                                virDomainDefPtr def,
>                                 virQEMUCapsPtr qemuCaps,
>                                 virDomainGraphicsDefPtr graphics)
> {
>@@ -7561,12 +7560,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
>         goto error;
>     }
>
>-    if (graphics->data.vnc.socket || cfg->vncAutoUnixSocket) {
>-        if (!graphics->data.vnc.socket &&
>-            virAsprintf(&graphics->data.vnc.socket,
>-                        "%s/%s.vnc", cfg->libDir, def->name) == -1)
>-            goto error;
>-
>+    if (graphics->data.vnc.socket) {
>         virBufferAsprintf(&opt, "unix:%s", graphics->data.vnc.socket);
>
>     } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) {
>@@ -7944,7 +7938,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
>         break;
>
>     case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
>-        return qemuBuildGraphicsVNCCommandLine(cfg, cmd, def, qemuCaps, graphics);
>+        return qemuBuildGraphicsVNCCommandLine(cfg, cmd, qemuCaps, graphics);
>
>     case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
>         return qemuBuildGraphicsSPICECommandLine(cfg, cmd, qemuCaps, graphics);
>diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>index 605b3c6..5de46e2 100644
>--- a/src/qemu/qemu_process.c
>+++ b/src/qemu/qemu_process.c
>@@ -4479,7 +4479,16 @@ int qemuProcessStart(virConnectPtr conn,
>                 goto cleanup;
>         }
>
>-        if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ||
>+        if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
>+            !graphics->data.vnc.socket &&
>+            cfg->vncAutoUnixSocket) {
>+            if (virAsprintf(&graphics->data.vnc.socket,
>+                        "%s/%s.vnc", cfg->libDir, vm->def->name) < 0)

Indentation's off.

>+                goto cleanup;
>+        }

From the configuration file comment:

  "This will only be enabled for VNC configurations that do not have
   a hardcoded 'listen' or 'socket' value. This setting takes preference
   over vnc_listen."

I see that that's not the true from your code, but it wasn't true even
before.  Should we change what we documented when that probably wasn't
ever true at all?

Also I don't see that neither DAC nor SELinux security drivers would
be labelling that vnc socket.  Is that happening somewhere else than
in virSecurityManagerSetAllLabel() ?

Even though this is a pretty rare case, it's still a bug and it would
be nice to have it in the release.

>+
>+        if ((graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
>+             !graphics->data.vnc.socket) ||
>             graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
>             if (graphics->nListens == 0) {
>                 if (VIR_EXPAND_N(graphics->listens, graphics->nListens, 1) < 0)
>--
>2.3.6
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150429/1ccb71ce/attachment-0001.sig>