Bug 1352529

Upstream commit:

commit 824272cb28dcda0e97961a28a1ab4a9fe7ae842f
Author: Pavel Hrdina <phrdina>
Date:   Wed Feb 22 20:25:00 2017 +0100

    qemu: properly escape socket path for graphics

I can reproduce this issue.
verify with build:
libvirt-3.1.0-2.el7.x86_64

step:
0. Set vnc_auto_unix_socket = 1 in qemu.conf and restart libvirtd.service

1. Prepare a guest with "=" in domain name and with vnc graphic:
# virsh dumpxml foo=1
...
    <graphics type='vnc' port='-1' autoport='yes'>
      <listen type='address'/>
    </graphics>

2. Try to start guest:
# virsh start rhel7
Domain rhel7 started

check guest xml:
<graphics type='vnc' socket='/var/lib/libvirt/qemu/domain-1-rhel7/vnc.sock'>
      <listen type='socket' socket='/var/lib/libvirt/qemu/domain-1-rhel7/vnc.sock'/>
    </graphics>

check qemu cmd:
....-vnc unix:/var/lib/libvirt/qemu/domain-1-rhel7/vnc.sock 

move to verified.

I test this with build:
libvirt-3.2.0-7.virtcov.el7.x86_64

the guest can't be start, same error.
step same with description.
Hi Pavel, please help to check this, thanks in advance.

error:
# virsh start foo==1
error: Failed to start domain foo==1
error: internal error: process exited while connecting to monitor: qemu-kvm: -vnc unix:/var/lib/libvirt/qemu/domain-20-foo\=\=1/vnc.sock: Invalid parameter 'unix:/var/lib/libvirt/qemu/domain-20-foo\'

Hi, so this is actually a QEMU issue introduced by upstream commit:

commit 4db14629c38611061fc19ec6927405923de84f08
Author: Gerd Hoffmann <kraxel>
Date:   Tue Sep 16 12:33:03 2014 +0200

    vnc: switch to QemuOpts, allow multiple servers

Moving to qemu-kvm-rhev.

Hmm, there is no easy way out.  This is how the QemuOpts parser works, it'll see the "=" and thinks this is a key=value assignment.  And changing QemuOpts behavier has a high chance to break stuff elsewhere.

I think libvirt should have a whitelist of allowed chars for constructing the /var/lib/libvirt/qemu/domain-20-${name}/ directory name.

Alternatively using "-vnc vnc=unix:/path,$opts" will work, but that approach has the drawback that it doesn't work with older qemu versions (before commit 4db14629c38611061fc19ec6927405923de84f08).

Pavel, what do you think?

I was worried about that it would be too invasive to rewrite QemuOpts.  I checked the code and noticed that this is the way how it works, however, I was hoping that it would be possible to improve it.

About the whitelist of allowed characters, that's kind of tricky, because that would mean to limit allowed characters for the domain name as well and that might be a regression.

Basically libvirt can adapt itself to do the best possible thing, if the QEMU is new enough we can benefit from that commit and use "-vnc vnc=unix:/path,..." and for older QEMU we will fail to start the guest with some sane error message that this is not supported with that QEMU binary.

Based on that commit it should be easy to check whether the new "vnc" option is available or not.

I'll try to come up with a patch for upstream libvirt and if nobody will object to that solution I'll move this bug back to libvirt.

Thanks

Upstream patches posted for review:

https://www.redhat.com/archives/libvir-list/2017-July/msg00911.html

> About the whitelist of allowed characters, that's kind of tricky, because
> that would mean to limit allowed characters for the domain name as well and
> that might be a regression.

My idea was to simply drop non-whitelisted chars when generating the directory name, not forbid them in the domain name, i.e. domain "foo=bar" would be translated to a "/var/lib/libvirt/qemu/domain-${nr}-foobar" directory name.  Given that the domain ID is part of the name too these names should continue to be unique even if someone creates both a "foo" and "foo=" domain.

Moving it back to libvirt even though this bug is caused by switching to QemuOpts parser for VNC options in QEMU.

That switch to QemuOpts also introduced a new way how to specify the unix socket path and it can be easily detected and fixed in libvirt.  There is no possible fix in QEMU with the old format because that's how QemuOpts parser works.

After some more investigation this actually worked before the change in QEMU so it's a regression.

I'll prepare a new patches where we will use the new syntax if it's available and keep using the old syntax which works if the new is not available.

Upstream commit:

commit ed4d1653ede210b8bf23a70796716165f5c24380
Author: Pavel Hrdina <phrdina>
Date:   Fri Jul 21 19:54:33 2017 +0200

    qemu: properly handle '=' in the VNC socket path

v3.5.0-292-ged4d1653ed

Verify pass on libvirt-3.8.0-1.virtcov.el7.x86_64

Steps:
1. Start guest with vnc listening on unix socket, and guest name contains "=":
...
    <graphics type='vnc'>
      <listen type='socket'/>
    </graphics>
...

2. Check qemu command line:
...-vnc vnc=unix:/var/lib/libvirt/qemu/domain-4-foo=1/vnc.sock...

Verify pass for spice with same scenario.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHEA-2018:0704