[PATCH] qemu: simplify PCI configfd handling in monitor

[Eric Blake <eblake@xxxxxxxxxx>]

This is also a bug fix - on the error path, qemu_hotplug would
leave the configfd file leaked into qemu.  At least the next
attempt to hotplug a PCI device would reuse the same fdname,
and when the qemu getfd monitor command gets a new fd by the
same name as an earlier one, it closes the earlier one, so there
is no risk of qemu running out of fds.

* src/qemu/qemu_monitor.h (qemuMonitorAddDeviceWithFd): New
prototype.
* src/qemu/qemu_monitor.c (qemuMonitorAddDevice): Move guts...
(qemuMonitorAddDeviceWithFd): ...to new function, and add support
for fd passing.
* src/qemu/qemu_hotplug.c (qemuDomainAttachHostPciDevice): Use it
to simplify code.
---

> > It is a nice idea todo the SendFileHAndle/CloseFileHandle
> > calls from here, rather than in the qemu_driver code
> > itself. It makes the latter much clearer - we should think
> > about whether it makes sense to rewrite the NIC hotplug
> > code to work this way too.

> Agreed; I'll submit a patch for that shortly.

Thanks for requesting this - I found and plugged a resource leak
in the process.  NIC hotplug cleanup coming next.

 src/qemu/qemu_hotplug.c |   19 +++++++++----------
 src/qemu/qemu_monitor.c |   24 +++++++++++++++++++++---
 src/qemu/qemu_monitor.h |    5 +++++
 3 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e4ba526..40e4159 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -829,21 +829,19 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver,
         if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &hostdev->info) < 0)
             goto error;
         if (qemuCapsGet(qemuCaps, QEMU_CAPS_PCI_CONFIGFD)) {
-            configfd = qemuOpenPCIConfig(hostdev);
+            if (priv->monConfig->type != VIR_DOMAIN_CHR_TYPE_UNIX) {
+                qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                _("pci configfd file cannot be attached: "
+                                  "qemu is not using a unix socket monitor"));
+            } else {
+                configfd = qemuOpenPCIConfig(hostdev);
+            }
             if (configfd >= 0) {
                 if (virAsprintf(&configfd_name, "fd-%s",
                                 hostdev->info.alias) < 0) {
                     virReportOOMError();
                     goto error;
                 }
-
-                qemuDomainObjEnterMonitorWithDriver(driver, vm);
-                if (qemuMonitorSendFileHandle(priv->mon, configfd_name,
-                                              configfd) < 0) {
-                    qemuDomainObjExitMonitorWithDriver(driver, vm);
-                    goto error;
-                }
-                qemuDomainObjExitMonitorWithDriver(driver, vm);
             }
         }

@@ -858,7 +856,8 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver,
             goto error;

         qemuDomainObjEnterMonitorWithDriver(driver, vm);
-        ret = qemuMonitorAddDevice(priv->mon, devstr);
+        ret = qemuMonitorAddDeviceWithFd(priv->mon, devstr,
+                                         configfd, configfd_name);
         qemuDomainObjExitMonitorWithDriver(driver, vm);
     } else {
         virDomainDevicePCIAddress guestAddr;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index da38096..f2b1721 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2016,10 +2016,13 @@ int qemuMonitorDelDevice(qemuMonitorPtr mon,
 }


-int qemuMonitorAddDevice(qemuMonitorPtr mon,
-                         const char *devicestr)
+int qemuMonitorAddDeviceWithFd(qemuMonitorPtr mon,
+                               const char *devicestr,
+                               int fd,
+                               const char *fdname)
 {
-    VIR_DEBUG("mon=%p device=%s", mon, devicestr);
+    VIR_DEBUG("mon=%p device=%s fd=%d fdname=%s", mon, devicestr, fd,
+              NULLSTR(fdname));
     int ret;

     if (!mon) {
@@ -2028,13 +2031,28 @@ int qemuMonitorAddDevice(qemuMonitorPtr mon,
         return -1;
     }

+    if (fd >= 0 && qemuMonitorSendFileHandle(mon, fdname, fd) < 0)
+        return -1;
+
     if (mon->json)
         ret = qemuMonitorJSONAddDevice(mon, devicestr);
     else
         ret = qemuMonitorTextAddDevice(mon, devicestr);
+
+    if (ret < 0 && fd >= 0) {
+        if (qemuMonitorCloseFileHandle(mon, fdname) < 0)
+            VIR_WARN("failed to close device handle '%s'", fdname);
+    }
+
     return ret;
 }

+int qemuMonitorAddDevice(qemuMonitorPtr mon,
+                         const char *devicestr)
+{
+    return qemuMonitorAddDeviceWithFd(mon, devicestr, -1, NULL);
+}
+
 int qemuMonitorAddDrive(qemuMonitorPtr mon,
                         const char *drivestr)
 {
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 7bea083..a20ff8e 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -390,6 +390,11 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon,
 int qemuMonitorAddDevice(qemuMonitorPtr mon,
                          const char *devicestr);

+int qemuMonitorAddDeviceWithFd(qemuMonitorPtr mon,
+                               const char *devicestr,
+                               int fd,
+                               const char *fdname);
+
 int qemuMonitorDelDevice(qemuMonitorPtr mon,
                          const char *devalias);

-- 
1.7.4

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