On Tue, Mar 15, 2011 at 08:27:17PM -0600, Eric Blake wrote:
> 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);
>
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
