Re: [libvirt] [PATCH] API: Tweak virDomainOpenGraphics to return fd directly
On 08/26/2014 04:18 PM, Jiri Denemark wrote:
On Tue, Aug 26, 2014 at 16:05:44 -0600, Eric Blake wrote:
> Let's fix this before we bake in a painful API. Since we know
> that we have exactly one non-negative fd on success, we might
> as well return the fd directly instead of forcing the user to
> pass in a pointer. Fix a memory leak I found while reviewing.
>
> * include/libvirt/libvirt.h.in (virDomainOpenGraphicsFD): Drop
> unneeded parameter.
> * src/driver.h (virDrvDomainOpenGraphicsFD): Likewise.
> * src/libvirt.c (virDomainOpenGraphicsFD): Adjust interface to
> return fd directly.
> * daemon/remote.c (remoteDispatchDomainOpenGraphicsFd): Adjust
> semantics.
> * src/qemu/qemu_driver.c (qemuDomainOpenGraphicsFD): Likewise.
> * src/remote/remote_driver.c (remoteDomainOpenGraphicsFD):
> Likewise, and plug memory leak.
>
> Signed-off-by: Eric Blake <eblake(a)redhat.com>
> ---
> daemon/remote.c | 9 +++++----
> include/libvirt/libvirt.h.in | 1 -
> src/driver.h | 1 -
> src/libvirt.c | 14 ++++++--------
> src/qemu/qemu_driver.c | 3 +--
> src/remote/remote_driver.c | 17 +++++++++++------
> 6 files changed, 23 insertions(+), 22 deletions(-)
Makes sense. ACK.
And another fd leak that needs squashing in - we must close off the fd
that we handed to the hypervisor:
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 57c999c..f5b17ba 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -15865,18 +15865,19 @@ qemuDomainOpenGraphicsFD(virDomainPtr dom,
goto cleanup;
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorOpenGraphics(priv->mon, protocol, pair[1],
"graphicsfd",
- (flags &
VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH) != 0);
+ (flags &
VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH));
qemuDomainObjExitMonitor(driver, vm);
if (!qemuDomainObjEndJob(driver, vm))
vm = NULL;
+ if (ret < 0)
+ goto cleanup;
ret = pair[0];
+ pair[0] = -1;
cleanup:
- if (ret < 0) {
- VIR_FORCE_CLOSE(pair[0]);
- VIR_FORCE_CLOSE(pair[1]);
- }
+ VIR_FORCE_CLOSE(pair[0]);
+ VIR_FORCE_CLOSE(pair[1]);
if (vm)
virObjectUnlock(vm);
return ret;
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 539 bytes)