[libvirt] [PATCH] API: Tweak virDomainOpenGraphics to return fd directly

Eric Blake eblake at redhat.com
Tue Aug 26 22:05:44 UTC 2014


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 at 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(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index 27d1aa8..940dcfa 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -4419,10 +4419,9 @@ remoteDispatchDomainOpenGraphicsFd(virNetServerPtr server ATTRIBUTE_UNUSED,
     if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
         goto cleanup;

-    if (virDomainOpenGraphicsFD(dom,
-                                args->idx,
-                                &fd,
-                                args->flags) < 0)
+    if ((fd = virDomainOpenGraphicsFD(dom,
+                                      args->idx,
+                                      args->flags)) < 0)
         goto cleanup;

     if (virNetMessageAddFD(msg, fd) < 0)
@@ -4442,6 +4441,8 @@ remoteDispatchDomainOpenGraphicsFd(virNetServerPtr server ATTRIBUTE_UNUSED,
         virDomainFree(dom);
     return rv;
 }
+
+
 static int
 remoteDispatchDomainGetInterfaceParameters(virNetServerPtr server ATTRIBUTE_UNUSED,
                                            virNetServerClientPtr client ATTRIBUTE_UNUSED,
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 2a108b8..e79c9ad 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -5401,7 +5401,6 @@ int virDomainOpenGraphics(virDomainPtr dom,

 int virDomainOpenGraphicsFD(virDomainPtr dom,
                             unsigned int idx,
-                            int *fd,
                             unsigned int flags);

 int virDomainInjectNMI(virDomainPtr domain, unsigned int flags);
diff --git a/src/driver.h b/src/driver.h
index cd136ba..a7366e4 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -890,7 +890,6 @@ typedef int
 typedef int
 (*virDrvDomainOpenGraphicsFD)(virDomainPtr dom,
                               unsigned int idx,
-                              int *fd,
                               unsigned int flags);

 typedef int
diff --git a/src/libvirt.c b/src/libvirt.c
index 772cc0d..7a3f01a 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -20305,11 +20305,10 @@ virDomainOpenGraphics(virDomainPtr dom,
  * virDomainOpenGraphicsFD:
  * @dom: pointer to domain object
  * @idx: index of graphics config to open
- * @fd: returned file descriptor
  * @flags: bitwise-OR of virDomainOpenGraphicsFlags
  *
  * This will create a socket pair connected to the graphics backend of @dom.
- * One socket will be returned as @fd.
+ * One end of the socket will be returned on success.
  * If @dom has multiple graphics backends configured, then @idx will determine
  * which one is opened, starting from @idx 0.
  *
@@ -20320,21 +20319,18 @@ virDomainOpenGraphics(virDomainPtr dom,
  * libvirt hypervisor, over a UNIX domain socket. Attempts
  * to use this method over a TCP connection will always fail
  *
- * Returns 0 on success, -1 on failure
+ * Returns an fd on success, -1 on failure
  */
 int
 virDomainOpenGraphicsFD(virDomainPtr dom,
                         unsigned int idx,
-                        int *fd,
                         unsigned int flags)
 {
-    VIR_DOMAIN_DEBUG(dom, "idx=%u, fd=%p, flags=%x",
-                     idx, fd, flags);
+    VIR_DOMAIN_DEBUG(dom, "idx=%u, flags=%x", idx, flags);

     virResetLastError();

     virCheckDomainReturn(dom, -1);
-    virCheckNonNullArgGoto(fd, error);

     virCheckReadOnlyGoto(dom->conn->flags, error);

@@ -20347,7 +20343,7 @@ virDomainOpenGraphicsFD(virDomainPtr dom,

     if (dom->conn->driver->domainOpenGraphicsFD) {
         int ret;
-        ret = dom->conn->driver->domainOpenGraphicsFD(dom, idx, fd, flags);
+        ret = dom->conn->driver->domainOpenGraphicsFD(dom, idx, flags);
         if (ret < 0)
             goto error;
         return ret;
@@ -20359,6 +20355,8 @@ virDomainOpenGraphicsFD(virDomainPtr dom,
     virDispatchError(dom->conn);
     return -1;
 }
+
+
 /**
  * virConnectSetKeepAlive:
  * @conn: pointer to a hypervisor connection
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5ff2059..57c999c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15808,7 +15808,6 @@ qemuDomainOpenGraphics(virDomainPtr dom,
 static int
 qemuDomainOpenGraphicsFD(virDomainPtr dom,
                          unsigned int idx,
-                         int *fd,
                          unsigned int flags)
 {
     virQEMUDriverPtr driver = dom->conn->privateData;
@@ -15871,7 +15870,7 @@ qemuDomainOpenGraphicsFD(virDomainPtr dom,
     if (!qemuDomainObjEndJob(driver, vm))
         vm = NULL;

-    *fd = pair[0];
+    ret = pair[0];

  cleanup:
     if (ret < 0) {
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index e01216a..e949223 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -6448,7 +6448,6 @@ remoteDomainOpenGraphics(virDomainPtr dom,
 static int
 remoteDomainOpenGraphicsFD(virDomainPtr dom,
                            unsigned int idx,
-                           int *fd,
                            unsigned int flags)
 {
     int rv = -1;
@@ -6472,15 +6471,21 @@ remoteDomainOpenGraphicsFD(virDomainPtr dom,
         goto done;

     if (fdoutlen != 1) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("no file descriptor received"));
+        if (fdoutlen) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("too many file descriptors received"));
+            while (fdoutlen)
+                VIR_FORCE_CLOSE(fdout[--fdoutlen]);
+        } else {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("no file descriptor received"));
+        }
         goto done;
     }
-    *fd = fdout[0];
-
-    rv = 0;
+    rv = fdout[0];

  done:
+    VIR_FREE(fdout);
     remoteDriverUnlock(priv);

     return rv;
-- 
1.9.3




More information about the libvir-list mailing list