API: Tweak virDomainOpenGraphics to return fd directly

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.  Furthermore, I found some memory and fd
leaks while reviewing the code - the idea is that on success,
libvirtd will have handed two fds in two different directions:
one to qemu, and one to the RPC client.

* 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,
and plug fd leak.
* src/remote/remote_driver.c (remoteDomainOpenGraphicsFD):
Likewise, and plug memory and fd leak.

Signed-off-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Eric Blake 2014-08-26 16:04:37 -06:00
parent 993fa528a6
commit b259e459b9
6 changed files with 32 additions and 28 deletions

View File

@ -4398,6 +4398,7 @@ remoteDispatchDomainOpenGraphics(virNetServerPtr server ATTRIBUTE_UNUSED,
return rv;
}
static int
remoteDispatchDomainOpenGraphicsFd(virNetServerPtr server ATTRIBUTE_UNUSED,
virNetServerClientPtr client ATTRIBUTE_UNUSED,
@ -4419,10 +4420,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 +4442,8 @@ remoteDispatchDomainOpenGraphicsFd(virNetServerPtr server ATTRIBUTE_UNUSED,
virDomainFree(dom);
return rv;
}
static int
remoteDispatchDomainGetInterfaceParameters(virNetServerPtr server ATTRIBUTE_UNUSED,
virNetServerClientPtr client ATTRIBUTE_UNUSED,

View File

@ -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);

View File

@ -890,7 +890,6 @@ typedef int
typedef int
(*virDrvDomainOpenGraphicsFD)(virDomainPtr dom,
unsigned int idx,
int *fd,
unsigned int flags);
typedef int

View File

@ -20305,11 +20305,11 @@ 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, and the other end is
* handed to the hypervisor.
* If @dom has multiple graphics backends configured, then @idx will determine
* which one is opened, starting from @idx 0.
*
@ -20318,23 +20318,20 @@ virDomainOpenGraphics(virDomainPtr dom,
*
* This method can only be used when connected to a local
* libvirt hypervisor, over a UNIX domain socket. Attempts
* to use this method over a TCP connection will always fail
* 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 +20344,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 +20356,8 @@ virDomainOpenGraphicsFD(virDomainPtr dom,
virDispatchError(dom->conn);
return -1;
}
/**
* virConnectSetKeepAlive:
* @conn: pointer to a hypervisor connection

View File

@ -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;
@ -15866,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;
*fd = pair[0];
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;

View File

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