[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]