[libvirt] [PATCH] qemu: fix UNIX socket chardevs operating in client mode
When support was adding for passing a pre-opened listener socket to UNIX chardevs, it accidentally passed the listener socket for client mode chardevs too with predictable amounts of fail resulting. Expand the unit test coverage to validate that we are only doing FD passing when operating in server mode. Reported-by: Richard W.M. Jones <rjones@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_command.c | 3 +- .../qemuxml2argvdata/serial-unix-chardev.args | 2 ++ .../serial-unix-chardev.x86_64-latest.args | 36 +++++++++++++++++++ .../qemuxml2argvdata/serial-unix-chardev.xml | 4 +++ tests/qemuxml2argvtest.c | 1 + 5 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/serial-unix-chardev.x86_64-latest.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 82d8030a33..32eb59b6ab 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5083,7 +5083,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, break; case VIR_DOMAIN_CHR_TYPE_UNIX: - if ((flags & QEMU_BUILD_CHARDEV_UNIX_FD_PASS) && + if (dev->data.nix.listen && + (flags & QEMU_BUILD_CHARDEV_UNIX_FD_PASS) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS)) { if (qemuSecuritySetSocketLabel(secManager, (virDomainDefPtr)def) < 0) goto cleanup; diff --git a/tests/qemuxml2argvdata/serial-unix-chardev.args b/tests/qemuxml2argvdata/serial-unix-chardev.args index 584f4a1dd1..873d3263c6 100644 --- a/tests/qemuxml2argvdata/serial-unix-chardev.args +++ b/tests/qemuxml2argvdata/serial-unix-chardev.args @@ -26,4 +26,6 @@ server,nowait \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ -chardev socket,id=charserial0,path=/tmp/serial.sock \ -device isa-serial,chardev=charserial0,id=serial0 \ +-chardev socket,id=charserial1,path=/tmp/serial-server.sock,server,nowait \ +-device isa-serial,chardev=charserial1,id=serial1 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/serial-unix-chardev.x86_64-latest.args b/tests/qemuxml2argvdata/serial-unix-chardev.x86_64-latest.args new file mode 100644 index 0000000000..ce7a7f80d7 --- /dev/null +++ b/tests/qemuxml2argvdata/serial-unix-chardev.x86_64-latest.args @@ -0,0 +1,36 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-realtime mlock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ +-chardev socket,id=charserial0,path=/tmp/serial.sock \ +-device isa-serial,chardev=charserial0,id=serial0 \ +-chardev socket,id=charserial1,fd=1729,server,nowait \ +-device isa-serial,chardev=charserial1,id=serial1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/serial-unix-chardev.xml b/tests/qemuxml2argvdata/serial-unix-chardev.xml index 04f83779ce..af513d6445 100644 --- a/tests/qemuxml2argvdata/serial-unix-chardev.xml +++ b/tests/qemuxml2argvdata/serial-unix-chardev.xml @@ -25,6 +25,10 @@ <source mode='connect' path='/tmp/serial.sock'/> <target port='0'/> </serial> + <serial type='unix'> + <source mode='bind' path='/tmp/serial-server.sock'/> + <target port='1'/> + </serial> <console type='unix'> <source mode='connect' path='/tmp/serial.sock'/> <target port='0'/> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2d52f352b0..3be5af03aa 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1335,6 +1335,7 @@ mymain(void) QEMU_CAPS_CHARDEV_FILE_APPEND); DO_TEST("serial-unix-chardev", QEMU_CAPS_DEVICE_ISA_SERIAL); + DO_TEST_CAPS_LATEST("serial-unix-chardev"); DO_TEST("serial-tcp-chardev", QEMU_CAPS_DEVICE_ISA_SERIAL); DO_TEST("serial-udp-chardev", -- 2.17.1
On Fri, Jul 06, 2018 at 11:03:00AM +0100, Daniel P. Berrangé wrote:
When support was adding for passing a pre-opened listener socket to UNIX chardevs, it accidentally passed the listener socket for client mode chardevs too with predictable amounts of fail resulting.
Expand the unit test coverage to validate that we are only doing FD passing when operating in server mode.
Reported-by: Richard W.M. Jones <rjones@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
I suppose you might want to mention in the commit message that the bug only affects qemu 2.12, and the bug number (1598440). Anyhow, I tested this patch and it fixes the problem, so: Tested-by: Richard W.M. Jones <rjones@redhat.com> There is a scratch build for Rawhide containing the patch here: https://koji.fedoraproject.org/koji/taskinfo?taskID=28050872 Note that the patch requires quite a bit of adjustment to apply and build against 4.5.0. I wasn't at all confident that I've done it right so I didn't push anything to Rawhide. It'd be good to have this fix in Rawhide & RHEL 7 asap though. Rich.
src/qemu/qemu_command.c | 3 +- .../qemuxml2argvdata/serial-unix-chardev.args | 2 ++ .../serial-unix-chardev.x86_64-latest.args | 36 +++++++++++++++++++ .../qemuxml2argvdata/serial-unix-chardev.xml | 4 +++ tests/qemuxml2argvtest.c | 1 + 5 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/serial-unix-chardev.x86_64-latest.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 82d8030a33..32eb59b6ab 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5083,7 +5083,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, break;
case VIR_DOMAIN_CHR_TYPE_UNIX: - if ((flags & QEMU_BUILD_CHARDEV_UNIX_FD_PASS) && + if (dev->data.nix.listen && + (flags & QEMU_BUILD_CHARDEV_UNIX_FD_PASS) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS)) { if (qemuSecuritySetSocketLabel(secManager, (virDomainDefPtr)def) < 0) goto cleanup; diff --git a/tests/qemuxml2argvdata/serial-unix-chardev.args b/tests/qemuxml2argvdata/serial-unix-chardev.args index 584f4a1dd1..873d3263c6 100644 --- a/tests/qemuxml2argvdata/serial-unix-chardev.args +++ b/tests/qemuxml2argvdata/serial-unix-chardev.args @@ -26,4 +26,6 @@ server,nowait \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ -chardev socket,id=charserial0,path=/tmp/serial.sock \ -device isa-serial,chardev=charserial0,id=serial0 \ +-chardev socket,id=charserial1,path=/tmp/serial-server.sock,server,nowait \ +-device isa-serial,chardev=charserial1,id=serial1 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/serial-unix-chardev.x86_64-latest.args b/tests/qemuxml2argvdata/serial-unix-chardev.x86_64-latest.args new file mode 100644 index 0000000000..ce7a7f80d7 --- /dev/null +++ b/tests/qemuxml2argvdata/serial-unix-chardev.x86_64-latest.args @@ -0,0 +1,36 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-realtime mlock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ +-chardev socket,id=charserial0,path=/tmp/serial.sock \ +-device isa-serial,chardev=charserial0,id=serial0 \ +-chardev socket,id=charserial1,fd=1729,server,nowait \ +-device isa-serial,chardev=charserial1,id=serial1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/serial-unix-chardev.xml b/tests/qemuxml2argvdata/serial-unix-chardev.xml index 04f83779ce..af513d6445 100644 --- a/tests/qemuxml2argvdata/serial-unix-chardev.xml +++ b/tests/qemuxml2argvdata/serial-unix-chardev.xml @@ -25,6 +25,10 @@ <source mode='connect' path='/tmp/serial.sock'/> <target port='0'/> </serial> + <serial type='unix'> + <source mode='bind' path='/tmp/serial-server.sock'/> + <target port='1'/> + </serial> <console type='unix'> <source mode='connect' path='/tmp/serial.sock'/> <target port='0'/> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2d52f352b0..3be5af03aa 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1335,6 +1335,7 @@ mymain(void) QEMU_CAPS_CHARDEV_FILE_APPEND); DO_TEST("serial-unix-chardev", QEMU_CAPS_DEVICE_ISA_SERIAL); + DO_TEST_CAPS_LATEST("serial-unix-chardev"); DO_TEST("serial-tcp-chardev", QEMU_CAPS_DEVICE_ISA_SERIAL); DO_TEST("serial-udp-chardev", -- 2.17.1
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
On Fri, Jul 06, 2018 at 12:12:51PM +0100, Richard W.M. Jones wrote:
On Fri, Jul 06, 2018 at 11:03:00AM +0100, Daniel P. Berrangé wrote:
When support was adding for passing a pre-opened listener socket to UNIX chardevs, it accidentally passed the listener socket for client mode chardevs too with predictable amounts of fail resulting.
Expand the unit test coverage to validate that we are only doing FD passing when operating in server mode.
Reported-by: Richard W.M. Jones <rjones@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
I suppose you might want to mention in the commit message that the bug only affects qemu 2.12, and the bug number (1598440).
Anyhow, I tested this patch and it fixes the problem, so:
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Thanks, added that info and push it.
There is a scratch build for Rawhide containing the patch here:
https://koji.fedoraproject.org/koji/taskinfo?taskID=28050872
Note that the patch requires quite a bit of adjustment to apply and build against 4.5.0. I wasn't at all confident that I've done it right so I didn't push anything to Rawhide. It'd be good to have this fix in Rawhide & RHEL 7 asap though.
It just needs the 3 prior patches including first. I've added those all to Fedora rawhide. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrangé -
Richard W.M. Jones