6:31 p.m.
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