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@xxxxxxxxxx>
---
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list