Re: [libvirt] [PATCHv2 2/3] qemu: simplify PCI configfd handling in monitor

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. --- v2: simplify, due to error handling improvement in patch 1 src/qemu/qemu_hotplug.c | 11 ++--------- src/qemu/qemu_monitor.c | 24 +++++++++++++++++++++--- src/qemu/qemu_monitor.h | 5 +++++ 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 20d94e4..87fbb9a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -820,14 +820,6 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, virReportOOMError(); goto error; } - - qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (qemuMonitorSendFileHandle(priv->mon, configfd_name, - configfd) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - goto error; - } - qemuDomainObjExitMonitorWithDriver(driver, vm); } } @@ -842,7 +834,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 fb875fc..074b0b2 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2029,10 +2029,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) { @@ -2041,13 +2044,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);