1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] qemu: fix UNIX socket chardevs operating in client mode

Daniel P. Berrangé posted 1 patch 5 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180706100300.30777-1-berrange@redhat.com
Test syntax-check passed
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
[libvirt] [PATCH] qemu: fix UNIX socket chardevs operating in client mode
Posted by Daniel P. Berrangé 5 years, 9 months ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix UNIX socket chardevs operating in client mode
Posted by Richard W.M. Jones 5 years, 9 months ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix UNIX socket chardevs operating in client mode
Posted by Daniel P. Berrangé 5 years, 9 months ago 
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 :|

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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.