On 08/25/14 20:22, Ján Tomko wrote:

Define the public API implementation and declare internal driver prototype. --- include/libvirt/libvirt.h.in | 5 ++++ src/driver.h | 7 ++++++ src/libvirt.c | 58 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ 4 files changed, 75 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 47ea695..153b386 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in

diff --git a/src/libvirt.c b/src/libvirt.c index 8349261..9de1e44 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20182,6 +20182,64 @@ 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 pair will be returned as @fd.

You return just one fd, so: "One element will be " Or perhaps "One socket .. " ?

+ * If @dom has multiple graphics backends configured, then @idx will determine + * which one is opened, starting from @idx 0. + * + * To disable any authentication, pass the VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH + * constant for @flags. + * + * 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 + * + * Returns 0 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); + + virResetLastError(); + + virCheckDomainReturn(dom, -1); + virCheckNonNullArgGoto(fd, error); + + virCheckReadOnlyGoto(dom->conn->flags, error); + + if (!VIR_DRV_SUPPORTS_FEATURE(dom->conn->driver, dom->conn, + VIR_DRV_FEATURE_FD_PASSING)) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("fd passing is not supported by this connection")); + goto error; + } + + if (dom->conn->driver->domainOpenGraphicsFD) { + int ret; + ret = dom->conn->driver->domainOpenGraphicsFD(dom, idx, fd, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(dom->conn); + return -1; +} +/** * virConnectSetKeepAlive: * @conn: pointer to a hypervisor connection * @interval: number of seconds of inactivity before a keepalive message is sent

No other objections from my side. I think this is a nice inversion of the virDomainOpenGraphics API. As this approach was suggested by DanPB I'm happy to give an ACK (with docs tweaked of course) Peter

--- daemon/remote.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 43 +++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 15 ++++++++++++++- src/remote_protocol-structs | 6 ++++++ src/rpc/virnetmessage.c | 26 ++++++++++++++++++++++++++ src/rpc/virnetmessage.h | 3 +++ 6 files changed, 136 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index ea16789..27d1aa8 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4399,6 +4399,50 @@ remoteDispatchDomainOpenGraphics(virNetServerPtr server ATTRIBUTE_UNUSED, } static int +remoteDispatchDomainOpenGraphicsFd(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg, + virNetMessageErrorPtr rerr, + remote_domain_open_graphics_fd_args *args) +{ + virDomainPtr dom = NULL; + int rv = -1; + int fd = -1; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if (virDomainOpenGraphicsFD(dom, + args->idx, + &fd, + args->flags) < 0) + goto cleanup; + + if (virNetMessageAddFD(msg, fd) < 0) + goto cleanup; + + /* return 1 here to let virNetServerProgramDispatchCall know + * we are passing a FD */ + rv = 1; + + cleanup: + VIR_FORCE_CLOSE(fd); + if (rv < 0) { + virNetMessageSaveError(rerr); + } + + if (dom) + virDomainFree(dom); + return rv; +} +static int remoteDispatchDomainGetInterfaceParameters(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, virNetMessagePtr msg ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 9a1d78f..e01216a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6446,6 +6446,48 @@ remoteDomainOpenGraphics(virDomainPtr dom, static int +remoteDomainOpenGraphicsFD(virDomainPtr dom, + unsigned int idx, + int *fd, + unsigned int flags) +{ + int rv = -1; + remote_domain_open_graphics_args args; + struct private_data *priv = dom->conn->privateData; + int *fdout = NULL; + size_t fdoutlen = 0; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, dom); + args.idx = idx; + args.flags = flags; + + if (callFull(dom->conn, priv, 0, + NULL, 0, + &fdout, &fdoutlen, + REMOTE_PROC_DOMAIN_OPEN_GRAPHICS_FD, + (xdrproc_t) xdr_remote_domain_open_graphics_fd_args, (char *) &args, + (xdrproc_t) xdr_void, NULL) == -1) + goto done; + + if (fdoutlen != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("no file descriptor received")); + goto done; + } + *fd = fdout[0]; + + rv = 0; + + done: + remoteDriverUnlock(priv); + + return rv; +} + + +static int remoteConnectSetKeepAlive(virConnectPtr conn, int interval, unsigned int count) { struct private_data *priv = conn->privateData; @@ -7963,6 +8005,7 @@ static virDriver remote_driver = { .domainOpenConsole = remoteDomainOpenConsole, /* 0.8.6 */ .domainOpenChannel = remoteDomainOpenChannel, /* 1.0.2 */ .domainOpenGraphics = remoteDomainOpenGraphics, /* 0.9.7 */ + .domainOpenGraphicsFD = remoteDomainOpenGraphicsFD, /* 1.2.8 */ .domainInjectNMI = remoteDomainInjectNMI, /* 0.9.2 */ .domainMigrateBegin3 = remoteDomainMigrateBegin3, /* 0.9.2 */ .domainMigratePrepare3 = remoteDomainMigratePrepare3, /* 0.9.2 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 5c316fb..6dc2d29 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2733,6 +2733,12 @@ struct remote_domain_open_graphics_args { unsigned int flags; }; +struct remote_domain_open_graphics_fd_args { + remote_nonnull_domain dom; + unsigned int idx; + unsigned int flags; +}; + struct remote_node_suspend_for_duration_args { unsigned int target; unsigned hyper duration; @@ -5420,5 +5426,12 @@ enum remote_procedure { * @generate: both * @acl: connect:write */ - REMOTE_PROC_CONNECT_GET_DOMAIN_CAPABILITIES = 342 + REMOTE_PROC_CONNECT_GET_DOMAIN_CAPABILITIES = 342, + + /** + * @generate: none + * @acl: domain:open_graphics + */ + REMOTE_PROC_DOMAIN_OPEN_GRAPHICS_FD = 343 + }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 9bf09b8..38bf0b8 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2153,6 +2153,11 @@ struct remote_domain_open_graphics_args { u_int idx; u_int flags; }; +struct remote_domain_open_graphics_fd_args { + remote_nonnull_domain dom; + u_int idx; + u_int flags; +}; struct remote_node_suspend_for_duration_args { u_int target; uint64_t duration; @@ -2862,4 +2867,5 @@ enum remote_procedure { REMOTE_PROC_NODE_GET_FREE_PAGES = 340, REMOTE_PROC_NETWORK_GET_DHCP_LEASES = 341, REMOTE_PROC_CONNECT_GET_DOMAIN_CAPABILITIES = 342, + REMOTE_PROC_DOMAIN_OPEN_GRAPHICS_FD = 343, }; diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c index 19b2d6c..5c57128 100644 --- a/src/rpc/virnetmessage.c +++ b/src/rpc/virnetmessage.c @@ -564,3 +564,29 @@ int virNetMessageDupFD(virNetMessagePtr msg, } return fd; } + +int virNetMessageAddFD(virNetMessagePtr msg, + int fd) +{ + int newfd = -1; + + if ((newfd = dup(fd)) < 0) { + virReportSystemError(errno, + _("Unable to duplicate FD %d"), + fd); + goto error; + } + + if (virSetInherit(newfd, false) < 0) { + virReportSystemError(errno, + _("Cannot set close-on-exec %d"), + newfd); + goto error; + } + if (VIR_APPEND_ELEMENT(msg->fds, msg->nfds, newfd) < 0) + goto error; + return 0; + error: + VIR_FORCE_CLOSE(newfd); + return -1; +} diff --git a/src/rpc/virnetmessage.h b/src/rpc/virnetmessage.h index c94dddc..89a2ebf 100644 --- a/src/rpc/virnetmessage.h +++ b/src/rpc/virnetmessage.h @@ -96,4 +96,7 @@ void virNetMessageSaveError(virNetMessageErrorPtr rerr) int virNetMessageDupFD(virNetMessagePtr msg, size_t slot); +int virNetMessageAddFD(virNetMessagePtr msg, + int fd); + #endif /* __VIR_NET_MESSAGE_H__ */ -- 1.8.5.5

On 08/25/14 20:22, Ján Tomko wrote:

Should fix https://bugzilla.redhat.com/show_bug.cgi?id=999926 --- src/qemu/qemu_driver.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ad75bd9..932c638 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15806,6 +15806,85 @@ qemuDomainOpenGraphics(virDomainPtr dom, }

static int +qemuDomainOpenGraphicsFD(virDomainPtr dom, + unsigned int idx, + int *fd, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + int ret = -1; + qemuDomainObjPrivatePtr priv; + const char *protocol; + int pair[2] = {-1, -1}; + + virCheckFlags(VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH, -1); + + if (!(vm = qemuDomObjFromDomain(dom))) + return -1; + + if (virDomainOpenGraphicsFdEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + priv = vm->privateData; + + if (idx >= vm->def->ngraphics) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("No graphics backend with index %d"), idx); + goto cleanup; + } + switch (vm->def->graphics[idx]->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + protocol = "vnc"; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + protocol = "spice"; + break; + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Can only open VNC or SPICE graphics backends, not %s"), + virDomainGraphicsTypeToString(vm->def->graphics[idx]->type)); + goto cleanup; + } + + if (virSecurityManagerSetSocketLabel(driver->securityManager, vm->def) < 0) + goto cleanup; + + if (socketpair(PF_UNIX, SOCK_STREAM, 0, pair) < 0) + goto cleanup; + + if (virSecurityManagerClearSocketLabel(driver->securityManager, vm->def) < 0) + goto cleanup;

+ /* TODO create and label the socket here */

You already do this, don't you?

+ + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + qemuDomainObjEnterMonitor(driver, vm); + ret = qemuMonitorOpenGraphics(priv->mon, protocol, pair[1], "graphicsfd", + (flags & VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH) != 0); + qemuDomainObjExitMonitor(driver, vm); + if (!qemuDomainObjEndJob(driver, vm)) + vm = NULL; + + *fd = pair[0]; + + cleanup: + if (ret < 0) { + VIR_FORCE_CLOSE(pair[0]); + VIR_FORCE_CLOSE(pair[1]); + } + if (vm) + virObjectUnlock(vm); + return ret; +} + +static int qemuDomainSetBlockIoTune(virDomainPtr dom, const char *disk, virTypedParameterPtr params, @@ -17262,6 +17341,7 @@ static virDriver qemuDriver = { .connectDomainQemuMonitorEventDeregister = qemuConnectDomainQemuMonitorEventDeregister, /* 1.2.3 */ .domainOpenConsole = qemuDomainOpenConsole, /* 0.8.6 */ .domainOpenGraphics = qemuDomainOpenGraphics, /* 0.9.7 */ + .domainOpenGraphicsFD = qemuDomainOpenGraphicsFD, /* 1.2.8 */ .domainInjectNMI = qemuDomainInjectNMI, /* 0.9.2 */ .domainMigrateBegin3 = qemuDomainMigrateBegin3, /* 0.9.2 */ .domainMigratePrepare3 = qemuDomainMigratePrepare3, /* 0.9.2 */

ACK with comment removed. Peter