[libvirt] [PATCH v2] qemu: ensure vhostuser FD is initialized to -1

Daniel P. Berrangé posted 1 patch 4 years, 6 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20190930113908.5675-1-berrange@redhat.com
src/qemu/qemu_domain.c                                 |  2 ++
.../vhost-user-gpu-secondary.x86_64-latest.args        |  4 ++--
.../qemuxml2argvdata/vhost-user-vga.x86_64-latest.args |  2 +-
tests/qemuxml2argvdata/virtio-options.args             |  2 +-
tests/qemuxml2argvtest.c                               | 10 ++++++++++
5 files changed, 16 insertions(+), 4 deletions(-)
[libvirt] [PATCH v2] qemu: ensure vhostuser FD is initialized to -1
Posted by Daniel P. Berrangé 4 years, 6 months ago
The video private data was not initializing the vhostuser FD
causing us to attempt to close FD 0 many times over.

Fixes

  commit ca60ecfa8cc1bd85baf7137dd1864d5f00f019f0
  Author: Marc-André Lureau <marcandre.lureau@redhat.com>
  Date:   Mon Sep 23 14:44:36 2019 +0400

      qemu: add qemuDomainVideoPrivate

Since the test suite does not invoke qemuExtDevicesStart(), no
vhost_user_fd will be present when generating test XML. To deal
with this we can must a fake FD number. While the current XML
is using FD == 0, we pick a very interesting number that's unlikely
to be a real FD, so that we're more likely to see any mistakes
closing the invalid FD.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---

In v2:

 - Fix FD initialization in test suite

 src/qemu/qemu_domain.c                                 |  2 ++
 .../vhost-user-gpu-secondary.x86_64-latest.args        |  4 ++--
 .../qemuxml2argvdata/vhost-user-vga.x86_64-latest.args |  2 +-
 tests/qemuxml2argvdata/virtio-options.args             |  2 +-
 tests/qemuxml2argvtest.c                               | 10 ++++++++++
 5 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e8e895d9aa..e2b78c2457 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1367,6 +1367,8 @@ qemuDomainVideoPrivateNew(void)
     if (!(priv = virObjectNew(qemuDomainVideoPrivateClass)))
         return NULL;
 
+    priv->vhost_user_fd = -1;
+
     return (virObjectPtr) priv;
 }
 
diff --git a/tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args b/tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args
index 58f49595e7..77643d31c0 100644
--- a/tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args
@@ -31,8 +31,8 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 -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=chr-vu-video0,fd=0 \
--chardev socket,id=chr-vu-video1,fd=0 \
+-chardev socket,id=chr-vu-video0,fd=1729 \
+-chardev socket,id=chr-vu-video1,fd=1729 \
 -device vhost-user-vga,id=video0,max_outputs=1,chardev=chr-vu-video0,bus=pci.0,\
 addr=0x2 \
 -device vhost-user-gpu-pci,id=video1,max_outputs=1,chardev=chr-vu-video1,\
diff --git a/tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args b/tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args
index 6640d86fa5..dd5f9800d9 100644
--- a/tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args
@@ -31,7 +31,7 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 -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=chr-vu-video0,fd=0 \
+-chardev socket,id=chr-vu-video0,fd=1729 \
 -device vhost-user-vga,id=video0,max_outputs=1,chardev=chr-vu-video0,bus=pci.0,\
 addr=0x2 \
 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \
diff --git a/tests/qemuxml2argvdata/virtio-options.args b/tests/qemuxml2argvdata/virtio-options.args
index 79216a5503..33ecd8f4e8 100644
--- a/tests/qemuxml2argvdata/virtio-options.args
+++ b/tests/qemuxml2argvdata/virtio-options.args
@@ -49,7 +49,7 @@ ats=on \
 ats=on \
 -device virtio-input-host-pci,id=input3,evdev=/dev/input/event1234,bus=pci.0,\
 addr=0x12,iommu_platform=on,ats=on \
--chardev socket,id=chr-vu-video0,fd=0 \
+-chardev socket,id=chr-vu-video0,fd=1729 \
 -device vhost-user-gpu-pci,id=video0,max_outputs=1,chardev=chr-vu-video0,\
 bus=pci.0,addr=0x2,iommu_platform=on,ats=on \
 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0xc,iommu_platform=on,\
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 5bbac1c8b8..5f4e87aa6d 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -528,6 +528,16 @@ testCompareXMLToArgv(const void *data)
        }
     }
 
+    for (i = 0; i < vm->def->nvideos; i++) {
+        virDomainVideoDefPtr video = vm->def->videos[i];
+
+        if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) {
+            qemuDomainVideoPrivatePtr vpriv = QEMU_DOMAIN_VIDEO_PRIVATE(video);
+
+            vpriv->vhost_user_fd = 1729;
+        }
+    }
+
     if (flags & FLAG_SLIRP_HELPER) {
         for (i = 0; i < vm->def->nnets; i++) {
             virDomainNetDefPtr net = vm->def->nets[i];
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: ensure vhostuser FD is initialized to -1
Posted by Michal Privoznik 4 years, 6 months ago
On 9/30/19 1:39 PM, Daniel P. Berrangé wrote:
> The video private data was not initializing the vhostuser FD
> causing us to attempt to close FD 0 many times over.
> 
> Fixes
> 
>    commit ca60ecfa8cc1bd85baf7137dd1864d5f00f019f0
>    Author: Marc-André Lureau <marcandre.lureau@redhat.com>
>    Date:   Mon Sep 23 14:44:36 2019 +0400
> 
>        qemu: add qemuDomainVideoPrivate
> 
> Since the test suite does not invoke qemuExtDevicesStart(), no
> vhost_user_fd will be present when generating test XML. To deal
> with this we can must a fake FD number. While the current XML
> is using FD == 0, we pick a very interesting number that's unlikely
> to be a real FD, so that we're more likely to see any mistakes
> closing the invalid FD.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> 
> In v2:
> 
>   - Fix FD initialization in test suite
> 
>   src/qemu/qemu_domain.c                                 |  2 ++
>   .../vhost-user-gpu-secondary.x86_64-latest.args        |  4 ++--
>   .../qemuxml2argvdata/vhost-user-vga.x86_64-latest.args |  2 +-
>   tests/qemuxml2argvdata/virtio-options.args             |  2 +-
>   tests/qemuxml2argvtest.c                               | 10 ++++++++++
>   5 files changed, 16 insertions(+), 4 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

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