[libvirt] [PATCH 0/8 v2] Allow a client to connect to QEMU's VNC by passing an FD via libvirt
A overdue followup to https://www.redhat.com/archives/libvir-list/2011-June/msg01130.html New in this series - Completely re-written all RPC integration, to work with the new RPC layer code - Added systemtap probes for FD passing - Split into multiple patches - Renamed API to virDomainOpenGraphics - Untangled a patch to the graphics event
From: "Daniel P. Berrange" <berrange@redhat.com> The virDomainOpenGraphics API allows a libvirt client to pass in a file descriptor for an open socket pair, and get it connected to the graphics display of the guest. This is limited to working with local libvirt hypervisors conencted over a UNIX domain socket, since it will use UNIX FD passing * include/libvirt/libvirt.h.in: Define virDomainOpenGraphics * src/driver.h: Define driver for virDomainOpenGraphics * src/libvirt_public.syms, src/libvirt.c: Entry point for virDomainOpenGraphics --- include/libvirt/libvirt.h.in | 9 ++++++ src/driver.h | 6 ++++ src/libvirt.c | 66 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 82 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 361881a..daf8f6a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3104,6 +3104,15 @@ int virDomainOpenConsole(virDomainPtr dom, virStreamPtr st, unsigned int flags); +typedef enum { + VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH = (1 << 0), +} virDomainOpenGraphicsFlags; + +int virDomainOpenGraphics(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 b899d0e..4c14aaa 100644 --- a/src/driver.h +++ b/src/driver.h @@ -632,6 +632,11 @@ typedef int const char *dev_name, virStreamPtr st, unsigned int flags); +typedef int + (*virDrvDomainOpenGraphics)(virDomainPtr dom, + unsigned int idx, + int fd, + unsigned int flags); typedef int (*virDrvDomainInjectNMI)(virDomainPtr dom, unsigned int flags); @@ -881,6 +886,7 @@ struct _virDriver { virDrvDomainQemuMonitorCommand qemuDomainMonitorCommand; virDrvDomainQemuAttach qemuDomainAttach; virDrvDomainOpenConsole domainOpenConsole; + virDrvDomainOpenGraphics domainOpenGraphics; virDrvDomainInjectNMI domainInjectNMI; virDrvDomainMigrateBegin3 domainMigrateBegin3; virDrvDomainMigratePrepare3 domainMigratePrepare3; diff --git a/src/libvirt.c b/src/libvirt.c index 0b975da..5d7f1ce 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16975,3 +16975,69 @@ error: virDispatchError(dom->conn); return -1; } + + +/** + * virDomainOpenGraphics: + * @dom: pointer to domain object + * @fd: file descriptor to attach graphics to + * @idx: index of graphics config to open + * @flags: flags to control open operation + * + * This will attempt to connect the file descriptor @fd, to + * the graphics backend of @dom. 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. + * + * The caller should use an anonymous socketpair to open + * @fd before invocation. + * + * 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 virDomainOpenGraphics(virDomainPtr dom, + unsigned int idx, + int fd, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(dom, "idx=%u, fd=%d, flags=%x", + idx, fd, flags); + + virResetLastError(); + + if (!VIR_IS_DOMAIN(dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (fd < 0) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + if (dom->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (dom->conn->driver->domainOpenGraphics) { + int ret; + ret = dom->conn->driver->domainOpenGraphics(dom, idx, fd, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 9762fc4..caa21a2 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -495,6 +495,7 @@ LIBVIRT_0.9.7 { virDomainSnapshotGetParent; virDomainSnapshotListChildrenNames; virDomainSnapshotNumChildren; + virDomainOpenGraphics; } LIBVIRT_0.9.5; # .... define new API here using predicted next version number .... -- 1.7.6.4
On 10/21/2011 06:55 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
The virDomainOpenGraphics API allows a libvirt client to pass in a file descriptor for an open socket pair, and get it connected to the graphics display of the guest. This is limited to working with local libvirt hypervisors conencted over a UNIX domain
s/conencted/connected/
socket, since it will use UNIX FD passing
* include/libvirt/libvirt.h.in: Define virDomainOpenGraphics * src/driver.h: Define driver for virDomainOpenGraphics * src/libvirt_public.syms, src/libvirt.c: Entry point for virDomainOpenGraphics --- include/libvirt/libvirt.h.in | 9 ++++++ src/driver.h | 6 ++++ src/libvirt.c | 66 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 82 insertions(+), 0 deletions(-)
+++ b/src/libvirt.c @@ -16975,3 +16975,69 @@ error: virDispatchError(dom->conn); return -1; } + + +/** + * virDomainOpenGraphics: + * @dom: pointer to domain object + * @fd: file descriptor to attach graphics to + * @idx: index of graphics config to open
Swap these two lines to match parameter order.
+ * @flags: flags to control open operation + * + * This will attempt to connect the file descriptor @fd, to + * the graphics backend of @dom. 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. + * + * The caller should use an anonymous socketpair to open + * @fd before invocation. + * + * 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 virDomainOpenGraphics(virDomainPtr dom, + unsigned int idx, + int fd, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(dom, "idx=%u, fd=%d, flags=%x", + idx, fd, flags); + + virResetLastError(); + + if (!VIR_IS_DOMAIN(dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (fd< 0) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + }
Should we also fstat() and validate that fd is a socket fd?
+ + if (dom->conn->flags& VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (dom->conn->driver->domainOpenGraphics) { + int ret; + ret = dom->conn->driver->domainOpenGraphics(dom, idx, fd, flags); + if (ret< 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return -1; +}
ACK with nits fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Oct 24, 2011 at 02:56:36PM -0600, Eric Blake wrote:
On 10/21/2011 06:55 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
The virDomainOpenGraphics API allows a libvirt client to pass in a file descriptor for an open socket pair, and get it connected to the graphics display of the guest. This is limited to working with local libvirt hypervisors conencted over a UNIX domain
s/conencted/connected/
socket, since it will use UNIX FD passing
* include/libvirt/libvirt.h.in: Define virDomainOpenGraphics * src/driver.h: Define driver for virDomainOpenGraphics * src/libvirt_public.syms, src/libvirt.c: Entry point for virDomainOpenGraphics --- include/libvirt/libvirt.h.in | 9 ++++++ src/driver.h | 6 ++++ src/libvirt.c | 66 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 82 insertions(+), 0 deletions(-)
+++ b/src/libvirt.c @@ -16975,3 +16975,69 @@ error: virDispatchError(dom->conn); return -1; } + + +/** + * virDomainOpenGraphics: + * @dom: pointer to domain object + * @fd: file descriptor to attach graphics to + * @idx: index of graphics config to open
Swap these two lines to match parameter order.
+ * @flags: flags to control open operation + * + * This will attempt to connect the file descriptor @fd, to + * the graphics backend of @dom. 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. + * + * The caller should use an anonymous socketpair to open + * @fd before invocation. + * + * 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 virDomainOpenGraphics(virDomainPtr dom, + unsigned int idx, + int fd, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(dom, "idx=%u, fd=%d, flags=%x", + idx, fd, flags); + + virResetLastError(); + + if (!VIR_IS_DOMAIN(dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (fd< 0) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + }
Should we also fstat() and validate that fd is a socket fd?
Ok, I guess we can. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
From: "Daniel P. Berrange" <berrange@redhat.com> Not all VNC/SPICE servers use a TCP socket for their connections. It is possible to configure a UNIX socket server. The graphics event must thus include a UNIX socket address type. * include/libvirt/libvirt.h.in: Add UNIX socket address type for graphics event * src/qemu/qemu_monitor_json.c: Add 'unix' string to address type enum --- include/libvirt/libvirt.h.in | 5 +++-- src/qemu/qemu_monitor_json.c | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index daf8f6a..49797a7 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2894,6 +2894,7 @@ typedef enum { typedef enum { VIR_DOMAIN_EVENT_GRAPHICS_ADDRESS_IPV4, /* IPv4 address */ VIR_DOMAIN_EVENT_GRAPHICS_ADDRESS_IPV6, /* IPv6 address */ + VIR_DOMAIN_EVENT_GRAPHICS_ADDRESS_UNIX, /* UNIX socket path */ } virDomainEventGraphicsAddressType; @@ -2905,8 +2906,8 @@ typedef enum { */ struct _virDomainEventGraphicsAddress { int family; /* Address family, virDomainEventGraphicsAddressType */ - const char *node; /* Address of node (eg IP address) */ - const char *service; /* Service name/number (eg TCP port) */ + const char *node; /* Address of node (eg IP address, or UNIX path) */ + const char *service; /* Service name/number (eg TCP port, or NULL) */ }; typedef struct _virDomainEventGraphicsAddress virDomainEventGraphicsAddress; typedef virDomainEventGraphicsAddress *virDomainEventGraphicsAddressPtr; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index cd8f1e5..46f98ee 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -615,8 +615,8 @@ static void qemuMonitorJSONHandleIOError(qemuMonitorPtr mon, virJSONValuePtr dat VIR_ENUM_DECL(qemuMonitorGraphicsAddressFamily) -VIR_ENUM_IMPL(qemuMonitorGraphicsAddressFamily, VIR_DOMAIN_EVENT_GRAPHICS_ADDRESS_IPV6 + 1, - "ipv4", "ipv6"); +VIR_ENUM_IMPL(qemuMonitorGraphicsAddressFamily, VIR_DOMAIN_EVENT_GRAPHICS_ADDRESS_UNIX + 1, + "ipv4", "ipv6", "unix"); static void qemuMonitorJSONHandleVNC(qemuMonitorPtr mon, virJSONValuePtr data, int phase) { -- 1.7.6.4
On 10/21/2011 06:55 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
Not all VNC/SPICE servers use a TCP socket for their connections. It is possible to configure a UNIX socket server. The graphics event must thus include a UNIX socket address type.
* include/libvirt/libvirt.h.in: Add UNIX socket address type for graphics event * src/qemu/qemu_monitor_json.c: Add 'unix' string to address type enum --- include/libvirt/libvirt.h.in | 5 +++-- src/qemu/qemu_monitor_json.c | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> The QEMU monitor command 'add_client' can be used to connect to a VNC or SPICE graphics display. This allows for implementaton of the virDomainOpenGraphics API * src/qemu/qemu_driver.c: Implement virDomainOpenGraphics * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h, src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h, src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Add binding for 'add_client' command --- src/qemu/qemu_driver.c | 71 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 33 +++++++++++++++++++ src/qemu/qemu_monitor.h | 6 +++ src/qemu/qemu_monitor_json.c | 27 ++++++++++++++++ src/qemu/qemu_monitor_json.h | 5 +++ src/qemu/qemu_monitor_text.c | 32 +++++++++++++++++++ src/qemu/qemu_monitor_text.h | 5 +++ 7 files changed, 179 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 84ef4dd..fe509b7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10653,6 +10653,76 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, return ret; } +static int +qemuDomainOpenGraphics(virDomainPtr dom, + unsigned int idx, + int fd, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + int ret = -1; + qemuDomainObjPrivatePtr priv; + const char *protocol; + + virCheckFlags(VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH, -1); + + qemuDriverLock(driver); + virUUIDFormat(dom->uuid, uuidstr); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + if (!vm) { + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + priv = vm->privateData; + + if (idx >= vm->def->ngraphics) { + qemuReportError(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: + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Can only open VNC or SPICE graphics backends, not %s"), + virDomainGraphicsTypeToString(vm->def->graphics[idx]->type)); + goto cleanup; + } + + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + qemuDomainObjEnterMonitorWithDriver(driver, vm); + ret = qemuMonitorOpenGraphics(priv->mon, protocol, fd, "graphicsfd", + flags & VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH); + qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjEndJob(driver, vm) == 0) { + vm = NULL; + goto cleanup; + } + +cleanup: + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = "QEMU", @@ -10783,6 +10853,7 @@ static virDriver qemuDriver = { .qemuDomainMonitorCommand = qemuDomainMonitorCommand, /* 0.8.3 */ .qemuDomainAttach = qemuDomainAttach, /* 0.9.4 */ .domainOpenConsole = qemuDomainOpenConsole, /* 0.8.6 */ + .domainOpenGraphics = qemuDomainOpenGraphics, /* 0.9.7 */ .domainInjectNMI = qemuDomainInjectNMI, /* 0.9.2 */ .domainMigrateBegin3 = qemuDomainMigrateBegin3, /* 0.9.2 */ .domainMigratePrepare3 = qemuDomainMigratePrepare3, /* 0.9.2 */ diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 182e63d..f7cea2e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2591,3 +2591,36 @@ int qemuMonitorVMStatusToPausedReason(const char *status) } return VIR_DOMAIN_PAUSED_UNKNOWN; } + + +int qemuMonitorOpenGraphics(qemuMonitorPtr mon, + const char *protocol, + int fd, + const char *fdname, + bool skipauth) +{ + VIR_DEBUG("mon=%p protocol=%s fd=%d fdname=%s skipauth=%d", + mon, protocol, fd, NULLSTR(fdname), skipauth); + int ret; + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (qemuMonitorSendFileHandle(mon, fdname, fd) < 0) + return -1; + + if (mon->json) + ret = qemuMonitorJSONOpenGraphics(mon, protocol, fdname, skipauth); + else + ret = qemuMonitorTextOpenGraphics(mon, protocol, fdname, skipauth); + + if (ret < 0) { + if (qemuMonitorCloseFileHandle(mon, fdname) < 0) + VIR_WARN("failed to close device handle '%s'", fdname); + } + + return ret; +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 90e7b45..883e0aa 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -515,6 +515,12 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode); +int qemuMonitorOpenGraphics(qemuMonitorPtr mon, + const char *protocol, + int fd, + const char *fdname, + bool skipauth); + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 46f98ee..ced6b72 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3245,3 +3245,30 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, virJSONValueFree(reply); return ret; } + +int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon, + const char *protocol, + const char *fdname, + bool skipauth) +{ + int ret; + virJSONValuePtr cmd, reply = NULL; + + cmd = qemuMonitorJSONMakeCommand("add_client", + "s:protocol", protocol, + "s:fdname", fdname, + "b:skipauth", skipauth, + NULL); + + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index a638b41..f10d7d2 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -250,4 +250,9 @@ int qemuMonitorJSONSetLink(qemuMonitorPtr mon, const char *name, enum virDomainNetInterfaceLinkState state); +int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon, + const char *protocol, + const char *fdname, + bool skipauth); + #endif /* QEMU_MONITOR_JSON_H */ diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 4774df9..03b6b44 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -3394,3 +3394,35 @@ cleanup: VIR_FREE(reply); return ret; } + + +int qemuMonitorTextOpenGraphics(qemuMonitorPtr mon, + const char *protocol, + const char *fdname, + bool skipauth) +{ + char *cmd = NULL; + char *reply = NULL; + int ret = -1; + + if (virAsprintf(&cmd, "add_client %s %s %d", protocol, fdname, skipauth ? 0 : 1) < 0){ + virReportOOMError(); + goto cleanup; + } + + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("adding graphics client failed")); + goto cleanup; + } + + if (STRNEQ(reply, "")) + goto cleanup; + + ret = 0; + +cleanup: + VIR_FREE(reply); + VIR_FREE(cmd); + return ret; +} diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index cc2a252..f32fce0 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -243,4 +243,9 @@ int qemuMonitorTextSetLink(qemuMonitorPtr mon, const char *name, enum virDomainNetInterfaceLinkState state); +int qemuMonitorTextOpenGraphics(qemuMonitorPtr mon, + const char *protocol, + const char *fdname, + bool skipauth); + #endif /* QEMU_MONITOR_TEXT_H */ -- 1.7.6.4
On 10/21/2011 06:55 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
The QEMU monitor command 'add_client' can be used to connect to a VNC or SPICE graphics display. This allows for implementaton
s/implementaton/implementation/
of the virDomainOpenGraphics API
* src/qemu/qemu_driver.c: Implement virDomainOpenGraphics * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h, src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h, src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Add binding for 'add_client' command --- src/qemu/qemu_driver.c | 71 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 33 +++++++++++++++++++ src/qemu/qemu_monitor.h | 6 +++ src/qemu/qemu_monitor_json.c | 27 ++++++++++++++++ src/qemu/qemu_monitor_json.h | 5 +++ src/qemu/qemu_monitor_text.c | 32 +++++++++++++++++++ src/qemu/qemu_monitor_text.h | 5 +++ 7 files changed, 179 insertions(+), 0 deletions(-)
+ + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY)< 0) + goto cleanup; + qemuDomainObjEnterMonitorWithDriver(driver, vm); + ret = qemuMonitorOpenGraphics(priv->mon, protocol, fd, "graphicsfd", + flags& VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH);
'flags & bit' cannot portably convert to <stdbool.h> under the rules of gnulib's replacement <stdbool.h> (yes, C99 requires it to work, but all the world is not C99). This has to be: (flags & VIR_DOMAIN_OPEN_GRAPHICS_SKIPAUTH) != 0 with the explicit comparison to force a bool result. ACK with that fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Add APIs to the virNetSocket object, to allow file descriptors to be send/received over UNIX domain socket connections * src/rpc/virnetsocket.c, src/rpc/virnetsocket.h, src/libvirt_private.syms: Add APIs for FD send/recv --- examples/systemtap/rpc-monitor.stp | 10 ++++++ src/libvirt_private.syms | 4 ++ src/probes.d | 2 + src/rpc/virnetsocket.c | 62 ++++++++++++++++++++++++++++++++++++ src/rpc/virnetsocket.h | 5 +++ 5 files changed, 83 insertions(+), 0 deletions(-) diff --git a/examples/systemtap/rpc-monitor.stp b/examples/systemtap/rpc-monitor.stp index f246571..b76564f 100755 --- a/examples/systemtap/rpc-monitor.stp +++ b/examples/systemtap/rpc-monitor.stp @@ -155,3 +155,13 @@ probe libvirt.rpc.server_client_free { delete serverSocks[pid(), client]; } } + + +probe libvirt.rpc.socket_send_fd { + print_ts(sprintf("= %-16p send fd=%d", sock, fd)); +} + + +probe libvirt.rpc.socket_recv_fd { + print_ts(sprintf("= %-16p recv fd=%d", sock, fd)); +} \ No newline at end of file diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dedbd16..0648e49 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1244,6 +1244,10 @@ virNetSocketDupFD; virNetSocketFree; virNetSocketGetFD; virNetSocketListen; +virNetSocketIsLocal; +virNetSocketHasPassFD; +virNetSocketSendFD; +virNetSocketRecvFD; virNetSocketNewConnectTCP; virNetSocketNewListenUNIX; diff --git a/src/probes.d b/src/probes.d index 6d95c84..2f5d059 100644 --- a/src/probes.d +++ b/src/probes.d @@ -19,6 +19,8 @@ provider libvirt { # file: src/rpc/virnetsocket.c # prefix: rpc probe rpc_socket_new(void *sock, int refs, int fd, int errfd, int pid, const char *localAddr, const char *remoteAddr); + probe rpc_socket_send_fd(void *sock, int fd); + probe rpc_socket_recv_fd(void *sock, int fd); probe rpc_socket_ref(void *sock, int refs); probe rpc_socket_free(void *sock, int refs); diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index e4eff49..e88d84e 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -43,6 +43,8 @@ #include "event.h" #include "threads.h" +#include "passfd.h" + #define VIR_FROM_THIS VIR_FROM_RPC #define virNetError(code, ...) \ @@ -791,6 +793,17 @@ bool virNetSocketIsLocal(virNetSocketPtr sock) } +bool virNetSocketHasPassFD(virNetSocketPtr sock) +{ + bool hasPassFD = false; + virMutexLock(&sock->lock); + if (sock->localAddr.data.sa.sa_family == AF_UNIX) + hasPassFD = true; + virMutexUnlock(&sock->lock); + return hasPassFD; +} + + int virNetSocketGetPort(virNetSocketPtr sock) { int port; @@ -1128,6 +1141,55 @@ ssize_t virNetSocketWrite(virNetSocketPtr sock, const char *buf, size_t len) } +int virNetSocketSendFD(virNetSocketPtr sock, int fd) +{ + int ret = -1; + if (!virNetSocketHasPassFD(sock)) { + virNetError(VIR_ERR_INTERNAL_ERROR, + _("Sending file descriptors is not supported on this socket")); + return -1; + } + virMutexLock(&sock->lock); + PROBE(RPC_SOCKET_SEND_FD, + "sock=%p fd=%d", sock, fd); + if (sendfd(sock->fd, fd) < 0) { + virReportSystemError(errno, + _("Failed to send file descriptor %d"), + fd); + goto cleanup; + } + ret = 0; + +cleanup: + virMutexUnlock(&sock->lock); + return ret; +} + + +int virNetSocketRecvFD(virNetSocketPtr sock) +{ + int ret = -1; + if (!virNetSocketHasPassFD(sock)) { + virNetError(VIR_ERR_INTERNAL_ERROR, + _("Sending file descriptors is not supported on this socket")); + return -1; + } + virMutexLock(&sock->lock); + + if ((ret = recvfd(sock->fd, 0)) < 0) { + virReportSystemError(errno, "%s", + _("Failed to recv file descriptor")); + goto cleanup; + } + PROBE(RPC_SOCKET_RECV_FD, + "sock=%p fd=%d", sock, ret); + +cleanup: + virMutexUnlock(&sock->lock); + return ret; +} + + int virNetSocketListen(virNetSocketPtr sock, int backlog) { virMutexLock(&sock->lock); diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index 9c4f112..13cbb14 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -82,6 +82,8 @@ int virNetSocketDupFD(virNetSocketPtr sock, bool cloexec); bool virNetSocketIsLocal(virNetSocketPtr sock); +bool virNetSocketHasPassFD(virNetSocketPtr sock); + int virNetSocketGetPort(virNetSocketPtr sock); int virNetSocketGetLocalIdentity(virNetSocketPtr sock, @@ -94,6 +96,9 @@ int virNetSocketSetBlocking(virNetSocketPtr sock, ssize_t virNetSocketRead(virNetSocketPtr sock, char *buf, size_t len); ssize_t virNetSocketWrite(virNetSocketPtr sock, const char *buf, size_t len); +int virNetSocketSendFD(virNetSocketPtr sock, int fd); +int virNetSocketRecvFD(virNetSocketPtr sock); + void virNetSocketSetTLSSession(virNetSocketPtr sock, virNetTLSSessionPtr sess); # ifdef HAVE_SASL -- 1.7.6.4
On 10/21/2011 06:55 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
Add APIs to the virNetSocket object, to allow file descriptors to be send/received over UNIX domain socket connections
s/send/sent/
* src/rpc/virnetsocket.c, src/rpc/virnetsocket.h, src/libvirt_private.syms: Add APIs for FD send/recv --- examples/systemtap/rpc-monitor.stp | 10 ++++++ src/libvirt_private.syms | 4 ++ src/probes.d | 2 + src/rpc/virnetsocket.c | 62 ++++++++++++++++++++++++++++++++++++ src/rpc/virnetsocket.h | 5 +++ 5 files changed, 83 insertions(+), 0 deletions(-)
diff --git a/examples/systemtap/rpc-monitor.stp b/examples/systemtap/rpc-monitor.stp index f246571..b76564f 100755 --- a/examples/systemtap/rpc-monitor.stp +++ b/examples/systemtap/rpc-monitor.stp @@ -155,3 +155,13 @@ probe libvirt.rpc.server_client_free { delete serverSocks[pid(), client]; } } + + +probe libvirt.rpc.socket_send_fd { + print_ts(sprintf("= %-16p send fd=%d", sock, fd)); +} + + +probe libvirt.rpc.socket_recv_fd { + print_ts(sprintf("= %-16p recv fd=%d", sock, fd)); +} \ No newline at end of file
We should fix that, and supply a trailing newline.
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dedbd16..0648e49 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1244,6 +1244,10 @@ virNetSocketDupFD; virNetSocketFree; virNetSocketGetFD; virNetSocketListen; +virNetSocketIsLocal;
Did we really forget to export this one earlier?
+virNetSocketHasPassFD; +virNetSocketSendFD; +virNetSocketRecvFD;
Sorting?
+int virNetSocketRecvFD(virNetSocketPtr sock) +{ + int ret = -1; + if (!virNetSocketHasPassFD(sock)) { + virNetError(VIR_ERR_INTERNAL_ERROR, + _("Sending file descriptors is not supported on this socket"));
s/Sending/Receiving/
+ return -1; + } + virMutexLock(&sock->lock); + + if ((ret = recvfd(sock->fd, 0))< 0) {
Should we s/0/O_CLOEXEC/ for flags, so the received socket has the cloexec bit already set? ACK with nits fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Oct 24, 2011 at 03:31:13PM -0600, Eric Blake wrote:
On 10/21/2011 06:55 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
Add APIs to the virNetSocket object, to allow file descriptors to be send/received over UNIX domain socket connections
+ return -1; + } + virMutexLock(&sock->lock); + + if ((ret = recvfd(sock->fd, 0))< 0) {
Should we s/0/O_CLOEXEC/ for flags, so the received socket has the cloexec bit already set?
Yep, might as well Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
From: "Daniel P. Berrange" <berrange@redhat.com> Define two new RPC message types VIR_NET_CALL_WITH_FDS and VIR_NET_REPLY_WITH_FDS. These message types are equivalent to VIR_NET_CALL and VIR_NET_REPLY, except that between the message header, and payload there is a 32-bit integer field specifying how many file descriptors have been passed. The actual file descriptors are sent/recv'd out of band. * src/rpc/virnetmessage.c, src/rpc/virnetmessage.h, src/libvirt_private.syms: Add support for handling passed file descriptors * src/rpc/virnetprotocol.x: Extend protocol for FD passing --- src/libvirt_private.syms | 2 + src/rpc/virnetmessage.c | 111 +++++++++++++++++++++++++++++++++++++++++++++- src/rpc/virnetmessage.h | 9 ++++ src/rpc/virnetprotocol.x | 23 +++++++++- 4 files changed, 142 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0648e49..f798c4a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1178,6 +1178,8 @@ virFileFdopen; virNetMessageClear; virNetMessageEncodeHeader; virNetMessageEncodePayload; +virNetMessageEncodeNumFDs; +virNetMessageDecodeNumFDs; virNetMessageFree; virNetMessageNew; virNetMessageQueuePush; diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c index a1ae9c4..a833c90 100644 --- a/src/rpc/virnetmessage.c +++ b/src/rpc/virnetmessage.c @@ -21,11 +21,13 @@ #include <config.h> #include <stdlib.h> +#include <unistd.h> #include "virnetmessage.h" #include "memory.h" #include "virterror_internal.h" #include "logging.h" +#include "virfile.h" #define VIR_FROM_THIS VIR_FROM_RPC #define virNetError(code, ...) \ @@ -51,6 +53,13 @@ virNetMessagePtr virNetMessageNew(bool tracked) void virNetMessageClear(virNetMessagePtr msg) { bool tracked = msg->tracked; + size_t i; + + VIR_DEBUG("msg=%p nfds=%zu", msg, msg->nfds); + + for (i = 0 ; i < msg->nfds ; i++) + VIR_FORCE_CLOSE(msg->fds[i]); + VIR_FREE(msg->fds); memset(msg, 0, sizeof(*msg)); msg->tracked = tracked; } @@ -58,14 +67,18 @@ void virNetMessageClear(virNetMessagePtr msg) void virNetMessageFree(virNetMessagePtr msg) { + size_t i; if (!msg) return; + VIR_DEBUG("msg=%p nfds=%zu cb=%p", msg, msg->nfds, msg->cb); + if (msg->cb) msg->cb(msg, msg->opaque); - VIR_DEBUG("msg=%p", msg); - + for (i = 0 ; i < msg->nfds ; i++) + VIR_FORCE_CLOSE(msg->fds[i]); + VIR_FREE(msg->fds); VIR_FREE(msg); } @@ -239,6 +252,78 @@ cleanup: } +int virNetMessageEncodeNumFDs(virNetMessagePtr msg) +{ + XDR xdr; + unsigned int numFDs = msg->nfds; + int ret = -1; + + xdrmem_create(&xdr, msg->buffer + msg->bufferOffset, + msg->bufferLength - msg->bufferOffset, XDR_ENCODE); + + if (numFDs > VIR_NET_MESSAGE_NUM_FDS_MAX) { + virNetError(VIR_ERR_RPC, + _("Too many FDs to send %d, expected %d maximum"), + numFDs, VIR_NET_MESSAGE_NUM_FDS_MAX); + goto cleanup; + } + + if (!xdr_u_int(&xdr, &numFDs)) { + virNetError(VIR_ERR_RPC, "%s", _("Unable to encode number of FDs")); + goto cleanup; + } + msg->bufferOffset += xdr_getpos(&xdr); + + VIR_DEBUG("Send %zu FDs to peer", msg->nfds); + + ret = 0; + +cleanup: + xdr_destroy(&xdr); + return ret; +} + + +int virNetMessageDecodeNumFDs(virNetMessagePtr msg) +{ + XDR xdr; + unsigned int numFDs; + int ret = -1; + size_t i; + + xdrmem_create(&xdr, msg->buffer + msg->bufferOffset, + msg->bufferLength - msg->bufferOffset, XDR_DECODE); + if (!xdr_u_int(&xdr, &numFDs)) { + virNetError(VIR_ERR_RPC, "%s", _("Unable to decode number of FDs")); + goto cleanup; + } + msg->bufferOffset += xdr_getpos(&xdr); + + if (numFDs > VIR_NET_MESSAGE_NUM_FDS_MAX) { + virNetError(VIR_ERR_RPC, + _("Received too many FDs %d, expected %d maximum"), + numFDs, VIR_NET_MESSAGE_NUM_FDS_MAX); + goto cleanup; + } + + msg->nfds = numFDs; + if (VIR_ALLOC_N(msg->fds, msg->nfds) < 0) { + virReportOOMError(); + goto cleanup; + } + for (i = 0 ; i < msg->nfds ; i++) + msg->fds[i] = -1; + + VIR_DEBUG("Got %zu FDs from peer", msg->nfds); + + ret = 0; + +cleanup: + xdr_destroy(&xdr); + return ret; +} + + int virNetMessageEncodePayload(virNetMessagePtr msg, xdrproc_t filter, void *data) @@ -403,3 +488,25 @@ void virNetMessageSaveError(virNetMessageErrorPtr rerr) rerr->level = VIR_ERR_ERROR; } } + + +int virNetMessageDupFD(virNetMessagePtr msg, + size_t slot) +{ + int fd; + + if (slot >= msg->nfds) { + virNetError(VIR_ERR_INTERNAL_ERROR, + _("No FD available at slot %zu"), slot); + return -1; + } + + if ((fd = dup(msg->fds[slot])) < 0) { + virReportSystemError(errno, + _("Unable to duplicate FD %d"), + msg->fds[slot]); + return -1; + } + + return fd; +} diff --git a/src/rpc/virnetmessage.h b/src/rpc/virnetmessage.h index 307a041..ad63409 100644 --- a/src/rpc/virnetmessage.h +++ b/src/rpc/virnetmessage.h @@ -46,6 +46,9 @@ struct _virNetMessage { virNetMessageFreeCallback cb; void *opaque; + size_t nfds; + int *fds; + virNetMessagePtr next; }; @@ -78,6 +81,9 @@ int virNetMessageDecodePayload(virNetMessagePtr msg, void *data) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virNetMessageEncodeNumFDs(virNetMessagePtr msg); +int virNetMessageDecodeNumFDs(virNetMessagePtr msg); + int virNetMessageEncodePayloadRaw(virNetMessagePtr msg, const char *buf, size_t len) @@ -88,4 +94,7 @@ int virNetMessageEncodePayloadEmpty(virNetMessagePtr msg) void virNetMessageSaveError(virNetMessageErrorPtr rerr) ATTRIBUTE_NONNULL(1); +int virNetMessageDupFD(virNetMessagePtr msg, + size_t slot); + #endif /* __VIR_NET_MESSAGE_H__ */ diff --git a/src/rpc/virnetprotocol.x b/src/rpc/virnetprotocol.x index 306fafa..4520e38 100644 --- a/src/rpc/virnetprotocol.x +++ b/src/rpc/virnetprotocol.x @@ -62,6 +62,11 @@ const VIR_NET_MESSAGE_LEN_MAX = 4; */ const VIR_NET_MESSAGE_STRING_MAX = 65536; +/* Limit on number of File Descriptors allowed to be + * passed per message + */ +const VIR_NET_MESSAGE_NUM_FDS_MAX = 32; + /* * RPC wire format * @@ -126,6 +131,18 @@ const VIR_NET_MESSAGE_STRING_MAX = 65536; * remote_error error information * * status == VIR_NET_OK * <empty> + * + * - type == VIR_NET_CALL_WITH_FDS + * int8 - number of FDs + * XXX_args for procedure + * + * - type == VIR_NET_REPLY_WITH_FDS + * int8 - number of FDs + * * status == VIR_NET_OK + * XXX_ret for procedure + * * status == VIR_NET_ERROR + * remote_error Error information + * */ enum virNetMessageType { /* client -> server. args from a method call */ @@ -135,7 +152,11 @@ enum virNetMessageType { /* either direction. async notification */ VIR_NET_MESSAGE = 2, /* either direction. stream data packet */ - VIR_NET_STREAM = 3 + VIR_NET_STREAM = 3, + /* client -> server. args from a method call, with passed FDs */ + VIR_NET_CALL_WITH_FDS = 4, + /* server -> client. reply/error from a method call, with passed FDs */ + VIR_NET_REPLY_WITH_FDS = 5 }; enum virNetMessageStatus { -- 1.7.6.4
On 10/21/2011 06:55 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
Define two new RPC message types VIR_NET_CALL_WITH_FDS and VIR_NET_REPLY_WITH_FDS. These message types are equivalent to VIR_NET_CALL and VIR_NET_REPLY, except that between the message header, and payload there is a 32-bit integer field specifying how many file descriptors have been passed.
The actual file descriptors are sent/recv'd out of band.
* src/rpc/virnetmessage.c, src/rpc/virnetmessage.h, src/libvirt_private.syms: Add support for handling passed file descriptors * src/rpc/virnetprotocol.x: Extend protocol for FD passing --- src/libvirt_private.syms | 2 + src/rpc/virnetmessage.c | 111 +++++++++++++++++++++++++++++++++++++++++++++- src/rpc/virnetmessage.h | 9 ++++ src/rpc/virnetprotocol.x | 23 +++++++++- 4 files changed, 142 insertions(+), 3 deletions(-)
Where's the edits to docs/internals/rpc.html.in that describes these two new types? In the interest of well-documented code, I won't ack without a v2 with docs.
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0648e49..f798c4a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1178,6 +1178,8 @@ virFileFdopen; virNetMessageClear; virNetMessageEncodeHeader; virNetMessageEncodePayload; +virNetMessageEncodeNumFDs; +virNetMessageDecodeNumFDs;
Sorting?
@@ -135,7 +152,11 @@ enum virNetMessageType { /* either direction. async notification */ VIR_NET_MESSAGE = 2, /* either direction. stream data packet */ - VIR_NET_STREAM = 3 + VIR_NET_STREAM = 3, + /* client -> server. args from a method call, with passed FDs */ + VIR_NET_CALL_WITH_FDS = 4, + /* server -> client. reply/error from a method call, with passed FDs */ + VIR_NET_REPLY_WITH_FDS = 5
Have you tested the case where client sends 4 but server does not understand it? Likewise, what if server sends 5 but client does not understand it? Are those graceful failures? Do we risk stranding a leaked fd into the side that wasn't aware of how to handle the new protocol? Does this pass 'make check' with pdwtags (from the dwarves package) installed, or do you have to also tweak src/virnetprotocol-structs? What you have looks mostly okay, but needs a v2 at least for docs. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Oct 24, 2011 at 03:39:46PM -0600, Eric Blake wrote:
@@ -135,7 +152,11 @@ enum virNetMessageType { /* either direction. async notification */ VIR_NET_MESSAGE = 2, /* either direction. stream data packet */ - VIR_NET_STREAM = 3 + VIR_NET_STREAM = 3, + /* client -> server. args from a method call, with passed FDs */ + VIR_NET_CALL_WITH_FDS = 4, + /* server -> client. reply/error from a method call, with passed FDs */ + VIR_NET_REPLY_WITH_FDS = 5
Have you tested the case where client sends 4 but server does not understand it? Likewise, what if server sends 5 but client does not understand it? Are those graceful failures? Do we risk stranding a leaked fd into the side that wasn't aware of how to handle the new protocol?
IIUC, a remote application can send as many FDs as it likes. Until you actually do recvmsg() to accept the FD handle, the file handle does not exist in the process ? If that's correct, then the only leak scenario would be if an old client did a recvmsg for the FD and then just discarded it, which can't happen since old client never knew to do recvmsg. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 10/25/2011 06:26 AM, Daniel P. Berrange wrote:
On Mon, Oct 24, 2011 at 03:39:46PM -0600, Eric Blake wrote:
@@ -135,7 +152,11 @@ enum virNetMessageType { /* either direction. async notification */ VIR_NET_MESSAGE = 2, /* either direction. stream data packet */ - VIR_NET_STREAM = 3 + VIR_NET_STREAM = 3, + /* client -> server. args from a method call, with passed FDs */ + VIR_NET_CALL_WITH_FDS = 4, + /* server -> client. reply/error from a method call, with passed FDs */ + VIR_NET_REPLY_WITH_FDS = 5
Have you tested the case where client sends 4 but server does not understand it? Likewise, what if server sends 5 but client does not understand it? Are those graceful failures? Do we risk stranding a leaked fd into the side that wasn't aware of how to handle the new protocol?
IIUC, a remote application can send as many FDs as it likes. Until you actually do recvmsg() to accept the FD handle, the file handle does not exist in the process ?
OK, you're right that there is no fd leak. It is the recvmsg that tells the kernel to actually expose the fd into the client; if it were not so, then you could DoS an arbitrary process by sending it extra fds. However, I'm still wondering if the receiving end gracefully handles the unknown message type. And since VIR_NET_CALL_WITH_FDS is normally synchronous and expects a reply, does that mean the client has to reply with an explicit error, or does it mean that the server has to tolerate the lack of a reply, or will we have deadlock? Or do we need some sort of capability handshake where the client probes whether the server supports CALL_WITH_FDS before using it? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Extend the RPC client code to allow file descriptors to be sent to the server with calls, and received back with replies. * src/remote/remote_driver.c: Stub extra args * src/libvirt_private.syms, src/rpc/virnetclient.c, src/rpc/virnetclient.h, src/rpc/virnetclientprogram.c, src/rpc/virnetclientprogram.h: Extend APIs to allow FD passing --- src/libvirt_private.syms | 4 +++ src/remote/remote_driver.c | 1 + src/rpc/virnetclient.c | 25 +++++++++++++++++++++++ src/rpc/virnetclient.h | 2 + src/rpc/virnetclientprogram.c | 44 +++++++++++++++++++++++++++++++++++++++- src/rpc/virnetclientprogram.h | 4 +++ 6 files changed, 78 insertions(+), 2 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f798c4a..fd107e7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1174,6 +1174,10 @@ virFileFclose; virFileFdopen; +# virnetclient.h +virNetClientHasPassFD; + + # virnetmessage.h virNetMessageClear; virNetMessageEncodeHeader; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 1dea327..fb16994 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4116,6 +4116,7 @@ call (virConnectPtr conn ATTRIBUTE_UNUSED, client, counter, proc_nr, + 0, NULL, NULL, NULL, args_filter, args, ret_filter, ret); remoteDriverLock(priv); diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 085dc8d..e3ba157 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -258,6 +258,16 @@ int virNetClientDupFD(virNetClientPtr client, bool cloexec) } +bool virNetClientHasPassFD(virNetClientPtr client) +{ + int hasPassFD; + virNetClientLock(client); + hasPassFD = virNetSocketHasPassFD(client->sock); + virNetClientUnlock(client); + return hasPassFD; +} + + void virNetClientFree(virNetClientPtr client) { int i; @@ -684,6 +694,7 @@ static int virNetClientCallDispatchStream(virNetClientPtr client) static int virNetClientCallDispatch(virNetClientPtr client) { + size_t i; if (virNetMessageDecodeHeader(&client->msg) < 0) return -1; @@ -697,6 +708,15 @@ virNetClientCallDispatch(virNetClientPtr client) case VIR_NET_REPLY: /* Normal RPC replies */ return virNetClientCallDispatchReply(client); + case VIR_NET_REPLY_WITH_FDS: /* Normal RPC replies with FDs */ + if (virNetMessageDecodeNumFDs(&client->msg) < 0) + return -1; + for (i = 0 ; i < client->msg.nfds ; i++) { + if ((client->msg.fds[i] = virNetSocketRecvFD(client->sock)) < 0) + return -1; + } + return virNetClientCallDispatchReply(client); + case VIR_NET_MESSAGE: /* Async notifications */ return virNetClientCallDispatchMessage(client); @@ -728,6 +748,11 @@ virNetClientIOWriteMessage(virNetClientPtr client, thecall->msg->bufferOffset += ret; if (thecall->msg->bufferOffset == thecall->msg->bufferLength) { + size_t i; + for (i = 0 ; i < thecall->msg->nfds ; i++) { + if (virNetSocketSendFD(client->sock, thecall->msg->fds[i]) < 0) + return -1; + } thecall->msg->bufferOffset = thecall->msg->bufferLength = 0; if (thecall->expectReply) thecall->mode = VIR_NET_CLIENT_MODE_WAIT_RX; diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h index 1fabcfd..fb679e8 100644 --- a/src/rpc/virnetclient.h +++ b/src/rpc/virnetclient.h @@ -56,6 +56,8 @@ void virNetClientRef(virNetClientPtr client); int virNetClientGetFD(virNetClientPtr client); int virNetClientDupFD(virNetClientPtr client, bool cloexec); +bool virNetClientHasPassFD(virNetClientPtr client); + int virNetClientAddProgram(virNetClientPtr client, virNetClientProgramPtr prog); diff --git a/src/rpc/virnetclientprogram.c b/src/rpc/virnetclientprogram.c index 33fa507..4ff6cf3 100644 --- a/src/rpc/virnetclientprogram.c +++ b/src/rpc/virnetclientprogram.c @@ -22,6 +22,8 @@ #include <config.h> +#include <unistd.h> + #include "virnetclientprogram.h" #include "virnetclient.h" #include "virnetprotocol.h" @@ -267,10 +269,15 @@ int virNetClientProgramCall(virNetClientProgramPtr prog, virNetClientPtr client, unsigned serial, int proc, + size_t noutfds, + int *outfds, + size_t *ninfds, + int **infds, xdrproc_t args_filter, void *args, xdrproc_t ret_filter, void *ret) { virNetMessagePtr msg; + size_t i; if (!(msg = virNetMessageNew(false))) return -1; @@ -278,13 +285,30 @@ int virNetClientProgramCall(virNetClientProgramPtr prog, msg->header.prog = prog->program; msg->header.vers = prog->version; msg->header.status = VIR_NET_OK; - msg->header.type = VIR_NET_CALL; + msg->header.type = noutfds ? VIR_NET_CALL_WITH_FDS : VIR_NET_CALL; msg->header.serial = serial; msg->header.proc = proc; + msg->nfds = noutfds; + if (VIR_ALLOC_N(msg->fds, msg->nfds) < 0) { + virReportOOMError(); + goto error; + } + for (i = 0 ; i < msg->nfds ; i++) { + if ((msg->fds[i] = dup(outfds[i])) < 0) { + virReportSystemError(errno, + _("Cannot duplicate FD %d"), + outfds[i]); + goto error; + } + } if (virNetMessageEncodeHeader(msg) < 0) goto error; + if (msg->nfds && + virNetMessageEncodeNumFDs(msg) < 0) + goto error; + if (virNetMessageEncodePayload(msg, args_filter, args) < 0) goto error; @@ -295,7 +319,8 @@ int virNetClientProgramCall(virNetClientProgramPtr prog, * virNetClientSend should have validated the reply, * but it doesn't hurt to check again. */ - if (msg->header.type != VIR_NET_REPLY) { + if (msg->header.type != VIR_NET_REPLY && + msg->header.type != VIR_NET_REPLY_WITH_FDS) { virNetError(VIR_ERR_INTERNAL_ERROR, _("Unexpected message type %d"), msg->header.type); goto error; @@ -315,6 +340,21 @@ int virNetClientProgramCall(virNetClientProgramPtr prog, switch (msg->header.status) { case VIR_NET_OK: + if (infds && ninfds) { + *ninfds = msg->nfds; + if (VIR_ALLOC_N(*infds, *ninfds) < 0) { + virReportOOMError(); + goto error; + } + for (i = 0 ; i < *ninfds ; i++) { + if ((*infds[i] = dup(msg->fds[i])) < 0) { + virReportSystemError(errno, + _("Cannot duplicate FD %d"), + msg->fds[i]); + goto error; + } + } + } if (virNetMessageDecodePayload(msg, ret_filter, ret) < 0) goto error; break; diff --git a/src/rpc/virnetclientprogram.h b/src/rpc/virnetclientprogram.h index 82ae2c6..14a4c96 100644 --- a/src/rpc/virnetclientprogram.h +++ b/src/rpc/virnetclientprogram.h @@ -77,6 +77,10 @@ int virNetClientProgramCall(virNetClientProgramPtr prog, virNetClientPtr client, unsigned serial, int proc, + size_t noutfds, + int *outfds, + size_t *ninfds, + int **infds, xdrproc_t args_filter, void *args, xdrproc_t ret_filter, void *ret); -- 1.7.6.4
On 10/21/2011 06:55 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
Extend the RPC client code to allow file descriptors to be sent to the server with calls, and received back with replies.
* src/remote/remote_driver.c: Stub extra args * src/libvirt_private.syms, src/rpc/virnetclient.c, src/rpc/virnetclient.h, src/rpc/virnetclientprogram.c, src/rpc/virnetclientprogram.h: Extend APIs to allow FD passing --- src/libvirt_private.syms | 4 +++ src/remote/remote_driver.c | 1 + src/rpc/virnetclient.c | 25 +++++++++++++++++++++++ src/rpc/virnetclient.h | 2 + src/rpc/virnetclientprogram.c | 44 +++++++++++++++++++++++++++++++++++++++- src/rpc/virnetclientprogram.h | 4 +++ 6 files changed, 78 insertions(+), 2 deletions(-)
+bool virNetClientHasPassFD(virNetClientPtr client) +{ + int hasPassFD;
s/int/bool/
+ virNetClientLock(client); + hasPassFD = virNetSocketHasPassFD(client->sock); + virNetClientUnlock(client); + return hasPassFD;
so that your return value matches the signature off the bat.
@@ -697,6 +708,15 @@ virNetClientCallDispatch(virNetClientPtr client) case VIR_NET_REPLY: /* Normal RPC replies */ return virNetClientCallDispatchReply(client);
+ case VIR_NET_REPLY_WITH_FDS: /* Normal RPC replies with FDs */ + if (virNetMessageDecodeNumFDs(&client->msg)< 0) + return -1; + for (i = 0 ; i< client->msg.nfds ; i++) { + if ((client->msg.fds[i] = virNetSocketRecvFD(client->sock))< 0)
You do realize that gnulib's sendfd/recvfd pass a single byte alongside each out-of-band fd (since passing fds with 0-byte messages isn't portable). It looks like you were careful to ensure that fds are only sent and received in between complete messages; so hopefully we don't ever run into any problems where the extra byte payloads gets interleaved with real rpc traffic, since that could cause confusion on the current state of bytes going between endpoints. Is encryption ever used on UNIX sockets, or is that only for TCP connections?
@@ -278,13 +285,30 @@ int virNetClientProgramCall(virNetClientProgramPtr prog, msg->header.prog = prog->program; msg->header.vers = prog->version; msg->header.status = VIR_NET_OK; - msg->header.type = VIR_NET_CALL; + msg->header.type = noutfds ? VIR_NET_CALL_WITH_FDS : VIR_NET_CALL; msg->header.serial = serial; msg->header.proc = proc; + msg->nfds = noutfds; + if (VIR_ALLOC_N(msg->fds, msg->nfds)< 0) { + virReportOOMError(); + goto error; + } + for (i = 0 ; i< msg->nfds ; i++) { + if ((msg->fds[i] = dup(outfds[i]))< 0) {
Should we be using fcntl(outfds[i], F_DUP_CLOEXEC, 0) here, so that the dups don't leak out of this process? ACK with nits fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Oct 24, 2011 at 04:23:16PM -0600, Eric Blake wrote:
On 10/21/2011 06:55 AM, Daniel P. Berrange wrote:
@@ -697,6 +708,15 @@ virNetClientCallDispatch(virNetClientPtr client) case VIR_NET_REPLY: /* Normal RPC replies */ return virNetClientCallDispatchReply(client);
+ case VIR_NET_REPLY_WITH_FDS: /* Normal RPC replies with FDs */ + if (virNetMessageDecodeNumFDs(&client->msg)< 0) + return -1; + for (i = 0 ; i< client->msg.nfds ; i++) { + if ((client->msg.fds[i] = virNetSocketRecvFD(client->sock))< 0)
You do realize that gnulib's sendfd/recvfd pass a single byte alongside each out-of-band fd (since passing fds with 0-byte messages isn't portable). It looks like you were careful to ensure that fds are only sent and received in between complete messages; so hopefully we don't ever run into any problems where the extra byte payloads gets interleaved with real rpc traffic, since that could cause confusion on the current state of bytes going between endpoints. Is encryption ever used on UNIX sockets, or is that only for TCP connections?
In theory you can run SASL over the UNIX socket which could layer in encryption, but IIRC we always disable the encryption layer with SASL. It is kind of annoying that sendfd/recvfd actually put real data into the stream. This can certainly muck up this code, since I would need to be careful to always make sure to put the sendfd/recvfd calls in the right place wrt sending or recving the payload. My testing suggests I have this right, but will need to double check now for sanity Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
From: "Daniel P. Berrange" <berrange@redhat.com> The RPC server classes are extended to allow FDs to be received from clients with calls. THere is not currently any way for a procedure to pass FDs back to the client with replies * daemon/remote.c, src/rpc/gendispatch.pl: Change virNetMessageHeaderPtr param to virNetMessagePtr in dispatcher impls * src/rpc/virnetserver.c, src/rpc/virnetserverclient.c, src/rpc/virnetserverprogram.c, src/rpc/virnetserverprogram.h: Extend to support FD passing --- daemon/remote.c | 86 ++++++++++++++++++++-------------------- src/libvirt_private.syms | 1 + src/rpc/gendispatch.pl | 12 +++--- src/rpc/virnetserver.c | 3 +- src/rpc/virnetserverclient.c | 24 +++++++++++ src/rpc/virnetserverprogram.c | 29 ++++++++++++- src/rpc/virnetserverprogram.h | 2 +- 7 files changed, 103 insertions(+), 54 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 550bed4..2507e91 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -535,7 +535,7 @@ int remoteClientInitHook(virNetServerPtr srv ATTRIBUTE_UNUSED, static int remoteDispatchOpen(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, struct remote_open_args *args) { @@ -582,7 +582,7 @@ cleanup: static int remoteDispatchClose(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED) { virNetServerClientDelayedClose(client); @@ -593,7 +593,7 @@ remoteDispatchClose(virNetServerPtr server ATTRIBUTE_UNUSED, static int remoteDispatchDomainGetSchedulerType(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_get_scheduler_type_args *args, remote_domain_get_scheduler_type_ret *ret) @@ -768,7 +768,7 @@ cleanup: static int remoteDispatchDomainGetSchedulerParameters(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_get_scheduler_parameters_args *args, remote_domain_get_scheduler_parameters_ret *ret) @@ -821,7 +821,7 @@ no_memory: static int remoteDispatchDomainGetSchedulerParametersFlags(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_get_scheduler_parameters_flags_args *args, remote_domain_get_scheduler_parameters_flags_ret *ret) @@ -875,7 +875,7 @@ no_memory: static int remoteDispatchDomainMemoryStats(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_memory_stats_args *args, remote_domain_memory_stats_ret *ret) @@ -937,7 +937,7 @@ cleanup: static int remoteDispatchDomainBlockPeek(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_block_peek_args *args, remote_domain_block_peek_ret *ret) @@ -994,7 +994,7 @@ cleanup: static int remoteDispatchDomainBlockStatsFlags(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_block_stats_flags_args *args, remote_domain_block_stats_flags_ret *ret) @@ -1065,7 +1065,7 @@ cleanup: static int remoteDispatchDomainMemoryPeek(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_memory_peek_args *args, remote_domain_memory_peek_ret *ret) @@ -1120,7 +1120,7 @@ cleanup: static int remoteDispatchDomainGetSecurityLabel(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_get_security_label_args *args, remote_domain_get_security_label_ret *ret) @@ -1169,7 +1169,7 @@ cleanup: static int remoteDispatchNodeGetSecurityModel(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_node_get_security_model_ret *ret) { @@ -1212,7 +1212,7 @@ cleanup: static int remoteDispatchDomainGetVcpuPinInfo(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_get_vcpu_pin_info_args *args, remote_domain_get_vcpu_pin_info_ret *ret) @@ -1282,7 +1282,7 @@ no_memory: static int remoteDispatchDomainGetVcpus(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_get_vcpus_args *args, remote_domain_get_vcpus_ret *ret) @@ -1367,7 +1367,7 @@ no_memory: static int remoteDispatchDomainMigratePrepare(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_migrate_prepare_args *args, remote_domain_migrate_prepare_ret *ret) @@ -1424,7 +1424,7 @@ cleanup: static int remoteDispatchDomainMigratePrepare2(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_migrate_prepare2_args *args, remote_domain_migrate_prepare2_ret *ret) @@ -1476,7 +1476,7 @@ cleanup: static int remoteDispatchDomainGetMemoryParameters(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_get_memory_parameters_args *args, remote_domain_get_memory_parameters_ret *ret) @@ -1539,7 +1539,7 @@ cleanup: static int remoteDispatchDomainGetBlkioParameters(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_get_blkio_parameters_args *args, remote_domain_get_blkio_parameters_ret *ret) @@ -1602,7 +1602,7 @@ cleanup: static int remoteDispatchNodeGetCPUStats(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_node_get_cpu_stats_args *args, remote_node_get_cpu_stats_ret *ret) @@ -1680,7 +1680,7 @@ no_memory: static int remoteDispatchNodeGetMemoryStats(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_node_get_memory_stats_args *args, remote_node_get_memory_stats_ret *ret) @@ -1758,7 +1758,7 @@ no_memory: static int remoteDispatchDomainGetBlockJobInfo(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_get_block_job_info_args *args, remote_domain_get_block_job_info_ret *ret) @@ -1802,7 +1802,7 @@ cleanup: static int remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_auth_list_ret *ret) { @@ -1872,7 +1872,7 @@ cleanup: static int remoteDispatchAuthSaslInit(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_auth_sasl_init_ret *ret) { @@ -1996,7 +1996,7 @@ error: static int remoteDispatchAuthSaslStart(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_auth_sasl_start_args *args, remote_auth_sasl_start_ret *ret) @@ -2094,7 +2094,7 @@ error: static int remoteDispatchAuthSaslStep(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_auth_sasl_step_args *args, remote_auth_sasl_step_ret *ret) @@ -2191,7 +2191,7 @@ error: static int remoteDispatchAuthSaslInit(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_auth_sasl_init_ret *ret ATTRIBUTE_UNUSED) { @@ -2204,7 +2204,7 @@ remoteDispatchAuthSaslInit(virNetServerPtr server ATTRIBUTE_UNUSED, static int remoteDispatchAuthSaslStart(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_auth_sasl_start_args *args ATTRIBUTE_UNUSED, remote_auth_sasl_start_ret *ret ATTRIBUTE_UNUSED) @@ -2218,7 +2218,7 @@ remoteDispatchAuthSaslStart(virNetServerPtr server ATTRIBUTE_UNUSED, static int remoteDispatchAuthSaslStep(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_auth_sasl_step_args *args ATTRIBUTE_UNUSED, remote_auth_sasl_step_ret *ret ATTRIBUTE_UNUSED) @@ -2237,7 +2237,7 @@ remoteDispatchAuthSaslStep(virNetServerPtr server ATTRIBUTE_UNUSED, static int remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_auth_polkit_ret *ret) { @@ -2337,7 +2337,7 @@ authdeny: static int remoteDispatchAuthPolkit(virNetServerPtr server, virNetServerClientPtr client, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_auth_polkit_ret *ret) { @@ -2476,7 +2476,7 @@ authdeny: static int remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_auth_polkit_ret *ret ATTRIBUTE_UNUSED) { @@ -2496,7 +2496,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, static int remoteDispatchNodeDeviceGetParent(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_node_device_get_parent_args *args, remote_node_device_get_parent_ret *ret) @@ -2551,7 +2551,7 @@ cleanup: static int remoteDispatchDomainEventsRegister(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_domain_events_register_ret *ret ATTRIBUTE_UNUSED) { @@ -2593,7 +2593,7 @@ cleanup: static int remoteDispatchDomainEventsDeregister(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_domain_events_deregister_ret *ret ATTRIBUTE_UNUSED) { @@ -2667,7 +2667,7 @@ cleanup: static int remoteDispatchSecretGetValue(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_secret_get_value_args *args, remote_secret_get_value_ret *ret) @@ -2706,7 +2706,7 @@ cleanup: static int remoteDispatchDomainGetState(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_get_state_args *args, remote_domain_get_state_ret *ret) @@ -2740,7 +2740,7 @@ cleanup: static int remoteDispatchDomainEventsRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_domain_events_register_any_args *args) { @@ -2789,7 +2789,7 @@ cleanup: static int remoteDispatchDomainEventsDeregisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_domain_events_deregister_any_args *args) { @@ -2834,7 +2834,7 @@ cleanup: static int qemuDispatchMonitorCommand(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, qemu_monitor_command_args *args, qemu_monitor_command_ret *ret) @@ -2870,7 +2870,7 @@ cleanup: static int remoteDispatchDomainMigrateBegin3(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_migrate_begin3_args *args, remote_domain_migrate_begin3_ret *ret) @@ -2922,7 +2922,7 @@ cleanup: static int remoteDispatchDomainMigratePrepare3(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_migrate_prepare3_args *args, remote_domain_migrate_prepare3_ret *ret) @@ -2980,7 +2980,7 @@ cleanup: static int remoteDispatchDomainMigratePerform3(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_migrate_perform3_args *args, remote_domain_migrate_perform3_ret *ret) @@ -3036,7 +3036,7 @@ cleanup: static int remoteDispatchDomainMigrateFinish3(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_migrate_finish3_args *args, remote_domain_migrate_finish3_ret *ret) @@ -3090,7 +3090,7 @@ cleanup: static int remoteDispatchDomainMigrateConfirm3(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, remote_domain_migrate_confirm3_args *args) { diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fd107e7..3411a2e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1184,6 +1184,7 @@ virNetMessageEncodeHeader; virNetMessageEncodePayload; virNetMessageEncodeNumFDs; virNetMessageDecodeNumFDs; +virNetMessageDupFD; virNetMessageFree; virNetMessageNew; virNetMessageQueuePush; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index b7ac3c8..56af258 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -298,7 +298,7 @@ elsif ($opt_b) { print "static int ${name}(\n"; print " virNetServerPtr server,\n"; print " virNetServerClientPtr client,\n"; - print " virNetMessageHeaderPtr hdr,\n"; + print " virNetMessagePtr msg,\n"; print " virNetMessageErrorPtr rerr"; if ($argtype ne "void") { print ",\n $argtype *args"; @@ -315,13 +315,13 @@ elsif ($opt_b) { print "static int ${name}Helper(\n"; print " virNetServerPtr server,\n"; print " virNetServerClientPtr client,\n"; - print " virNetMessageHeaderPtr hdr,\n"; + print " virNetMessagePtr msg,\n"; print " virNetMessageErrorPtr rerr,\n"; print " void *args$argann,\n"; print " void *ret$retann)\n"; print "{\n"; - print " VIR_DEBUG(\"server=%p client=%p hdr=%p rerr=%p args=%p ret=%p\", server, client, hdr, rerr, args, ret);\n"; - print " return $name(server, client, hdr, rerr"; + print " VIR_DEBUG(\"server=%p client=%p msg=%p rerr=%p args=%p ret=%p\", server, client, msg, rerr, args, ret);\n"; + print " return $name(server, client, msg, rerr"; if ($argtype ne "void") { print ", args"; } @@ -750,7 +750,7 @@ elsif ($opt_b) { print "static int $name(\n"; print " virNetServerPtr server ATTRIBUTE_UNUSED,\n"; print " virNetServerClientPtr client,\n"; - print " virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED,\n"; + print " virNetMessagePtr msg ATTRIBUTE_UNUSED,\n"; print " virNetMessageErrorPtr rerr"; if ($argtype ne "void") { print ",\n $argtype *args"; @@ -809,7 +809,7 @@ elsif ($opt_b) { print " if (!(st = virStreamNew(priv->conn, VIR_STREAM_NONBLOCK)))\n"; print " goto cleanup;\n"; print "\n"; - print " if (!(stream = daemonCreateClientStream(client, st, remoteProgram, hdr)))\n"; + print " if (!(stream = daemonCreateClientStream(client, st, remoteProgram, &msg->header)))\n"; print " goto cleanup;\n"; print "\n"; } diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index f739743..6533b5a 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -138,7 +138,8 @@ static void virNetServerHandleJob(void *jobOpaque, void *opaque) * message types are not expecting replies, so we * must just log it & drop them */ - if (job->msg->header.type == VIR_NET_CALL) { + if (job->msg->header.type == VIR_NET_CALL || + job->msg->header.type == VIR_NET_CALL_WITH_FDS) { if (virNetServerProgramUnknownError(job->client, job->msg, &job->msg->header) < 0) diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 05077d6..1256f0f 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -770,6 +770,7 @@ readmore: /* Grab the completed message */ virNetMessagePtr msg = virNetMessageQueueServe(&client->rx); virNetServerClientFilterPtr filter; + size_t i; /* Decode the header so we can use it for routing decisions */ if (virNetMessageDecodeHeader(msg) < 0) { @@ -778,6 +779,20 @@ readmore: return; } + if (msg->header.type == VIR_NET_CALL_WITH_FDS && + virNetMessageDecodeNumFDs(msg) < 0) { + virNetMessageFree(msg); + client->wantClose = true; + return; + } + for (i = 0 ; i < msg->nfds ; i++) { + if ((msg->fds[i] = virNetSocketRecvFD(client->sock)) < 0) { + virNetMessageFree(msg); + client->wantClose = true; + return; + } + } + PROBE(RPC_SERVER_CLIENT_MSG_RX, "client=%p len=%zu prog=%u vers=%u proc=%u type=%u status=%u serial=%u", client, msg->bufferLength, @@ -883,6 +898,15 @@ virNetServerClientDispatchWrite(virNetServerClientPtr client) if (client->tx->bufferOffset == client->tx->bufferLength) { virNetMessagePtr msg; + size_t i; + + for (i = 0 ; i < client->tx->nfds ; i++) { + if (virNetSocketSendFD(client->sock, client->tx->fds[i]) < 0) { + client->wantClose = true; + return; + } + } + #if HAVE_SASL /* Completed this 'tx' operation, so now read for all * future rx/tx to be under a SASL SSF layer diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c index 47b7ded..bbd2d65 100644 --- a/src/rpc/virnetserverprogram.c +++ b/src/rpc/virnetserverprogram.c @@ -29,6 +29,7 @@ #include "memory.h" #include "virterror_internal.h" #include "logging.h" +#include "virfile.h" #define VIR_FROM_THIS VIR_FROM_RPC #define virNetError(code, ...) \ @@ -284,6 +285,7 @@ int virNetServerProgramDispatch(virNetServerProgramPtr prog, switch (msg->header.type) { case VIR_NET_CALL: + case VIR_NET_CALL_WITH_FDS: ret = virNetServerProgramDispatchCall(prog, server, client, msg); break; @@ -314,7 +316,8 @@ int virNetServerProgramDispatch(virNetServerProgramPtr prog, return ret; error: - if (msg->header.type == VIR_NET_CALL) { + if (msg->header.type == VIR_NET_CALL || + msg->header.type == VIR_NET_CALL_WITH_FDS) { ret = virNetServerProgramSendReplyError(prog, client, msg, &rerr, &msg->header); } else { /* Send a dummy reply to free up 'msg' & unblock client rx */ @@ -355,6 +358,7 @@ virNetServerProgramDispatchCall(virNetServerProgramPtr prog, int rv = -1; virNetServerProgramProcPtr dispatcher; virNetMessageError rerr; + size_t i; memset(&rerr, 0, sizeof(rerr)); @@ -409,7 +413,20 @@ virNetServerProgramDispatchCall(virNetServerProgramPtr prog, * * 'args and 'ret' */ - rv = (dispatcher->func)(server, client, &msg->header, &rerr, arg, ret); + rv = (dispatcher->func)(server, client, msg, &rerr, arg, ret); + + /* + * Clear out the FDs we got from the client, we don't + * want to send them back ! + * + * XXX we don't have a way to let dispatcher->func + * return any FDs. Fortunately we don't need this + * capability just yet + */ + for (i = 0 ; i < msg->nfds ; i++) + VIR_FORCE_CLOSE(msg->fds[i]); + VIR_FREE(msg->fds); + msg->nfds = 0; xdr_free(dispatcher->arg_filter, arg); @@ -421,7 +438,7 @@ virNetServerProgramDispatchCall(virNetServerProgramPtr prog, /*msg->header.prog = msg->header.prog;*/ /*msg->header.vers = msg->header.vers;*/ /*msg->header.proc = msg->header.proc;*/ - msg->header.type = VIR_NET_REPLY; + msg->header.type = msg->nfds ? VIR_NET_REPLY_WITH_FDS : VIR_NET_REPLY; /*msg->header.serial = msg->header.serial;*/ msg->header.status = VIR_NET_OK; @@ -430,6 +447,12 @@ virNetServerProgramDispatchCall(virNetServerProgramPtr prog, goto error; } + if (msg->nfds && + virNetMessageEncodeNumFDs(msg) < 0) { + xdr_free(dispatcher->ret_filter, ret); + goto error; + } + if (virNetMessageEncodePayload(msg, dispatcher->ret_filter, ret) < 0) { xdr_free(dispatcher->ret_filter, ret); goto error; diff --git a/src/rpc/virnetserverprogram.h b/src/rpc/virnetserverprogram.h index 9dabf92..aa9f3cf 100644 --- a/src/rpc/virnetserverprogram.h +++ b/src/rpc/virnetserverprogram.h @@ -41,7 +41,7 @@ typedef virNetServerProgramProc *virNetServerProgramProcPtr; typedef int (*virNetServerProgramDispatchFunc)(virNetServerPtr server, virNetServerClientPtr client, - virNetMessageHeaderPtr hdr, + virNetMessagePtr msg, virNetMessageErrorPtr rerr, void *args, void *ret); -- 1.7.6.4
On 10/21/2011 06:55 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
The RPC server classes are extended to allow FDs to be received from clients with calls. THere is not currently any way for a
s/THere/There/
procedure to pass FDs back to the client with replies
* daemon/remote.c, src/rpc/gendispatch.pl: Change virNetMessageHeaderPtr param to virNetMessagePtr in dispatcher impls * src/rpc/virnetserver.c, src/rpc/virnetserverclient.c, src/rpc/virnetserverprogram.c, src/rpc/virnetserverprogram.h: Extend to support FD passing
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Since it needs to access file descriptors pass in the msg, the RPC driver for virDomainOpenGraphics needs to be manually implemented. * daemon/remote.c: RPC server dispatcher * src/remote/remote_driver.c: RPC client dispatcher * src/remote/remote_protocol.x: Define protocol --- daemon/remote.c | 43 ++++++++++++++++++++++++ src/remote/remote_driver.c | 73 +++++++++++++++++++++++++++++++++++------ src/remote/remote_protocol.x | 9 ++++- 3 files changed, 113 insertions(+), 12 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 2507e91..291011a 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -44,6 +44,7 @@ #include "intprops.h" #include "virnetserverservice.h" #include "virnetserver.h" +#include "virfile.h" #include "remote_protocol.h" #include "qemu_protocol.h" @@ -3124,6 +3125,48 @@ cleanup: } +static int +remoteDispatchDomainOpenGraphics(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg, + virNetMessageErrorPtr rerr, + remote_domain_open_graphics_args *args) +{ + virDomainPtr dom = NULL; + int rv = -1; + int fd; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if ((fd = virNetMessageDupFD(msg, 0)) < 0) + goto cleanup; + + if (virDomainOpenGraphics(dom, + args->idx, + fd, + args->flags) < 0) + goto cleanup; + + rv = 0; + +cleanup: + VIR_FORCE_CLOSE(fd); + if (rv < 0) + virNetMessageSaveError(rerr); + if (dom) + virDomainFree(dom); + return rv; +} + + /*----- Helpers. -----*/ /* get_nonnull_domain and get_nonnull_network turn an on-wire diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index fb16994..c56ab55 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -103,10 +103,14 @@ static void remoteDriverUnlock(struct private_data *driver) virMutexUnlock(&driver->lock); } -static int call (virConnectPtr conn, struct private_data *priv, - unsigned int flags, int proc_nr, - xdrproc_t args_filter, char *args, - xdrproc_t ret_filter, char *ret); +static int call(virConnectPtr conn, struct private_data *priv, + unsigned int flags, int proc_nr, + xdrproc_t args_filter, char *args, + xdrproc_t ret_filter, char *ret); +static int callWithFD(virConnectPtr conn, struct private_data *priv, + unsigned int flags, int fd, int proc_nr, + xdrproc_t args_filter, char *args, + xdrproc_t ret_filter, char *ret); static int remoteAuthenticate (virConnectPtr conn, struct private_data *priv, virConnectAuthPtr auth, const char *authtype); #if HAVE_SASL @@ -4086,6 +4090,36 @@ done: } +static int +remoteDomainOpenGraphics(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; + + remoteDriverLock(priv); + + make_nonnull_domain (&args.dom, dom); + args.idx = idx; + args.flags = flags; + + if (callWithFD(dom->conn, priv, 0, fd, REMOTE_PROC_DOMAIN_OPEN_GRAPHICS, + (xdrproc_t) xdr_remote_domain_open_graphics_args, (char *) &args, + (xdrproc_t) xdr_void, NULL) == -1) + goto done; + + rv = 0; + +done: + remoteDriverUnlock(priv); + + return rv; +} + + #include "remote_client_bodies.h" #include "qemu_client_bodies.h" @@ -4094,17 +4128,20 @@ done: * send that to the server and wait for reply */ static int -call (virConnectPtr conn ATTRIBUTE_UNUSED, - struct private_data *priv, - unsigned int flags, - int proc_nr, - xdrproc_t args_filter, char *args, - xdrproc_t ret_filter, char *ret) +callWithFD(virConnectPtr conn ATTRIBUTE_UNUSED, + struct private_data *priv, + unsigned int flags, + int fd, + int proc_nr, + xdrproc_t args_filter, char *args, + xdrproc_t ret_filter, char *ret) { int rv; virNetClientProgramPtr prog = flags & REMOTE_CALL_QEMU ? priv->qemuProgram : priv->remoteProgram; int counter = priv->counter++; virNetClientPtr client = priv->client; + int fds[] = { fd }; + size_t nfds = fd == -1 ? 0 : 1; priv->localUses++; /* Unlock, so that if we get any async events/stream data @@ -4116,7 +4153,7 @@ call (virConnectPtr conn ATTRIBUTE_UNUSED, client, counter, proc_nr, - 0, NULL, NULL, NULL, + nfds, nfds ? fds : NULL, NULL, NULL, args_filter, args, ret_filter, ret); remoteDriverLock(priv); @@ -4125,6 +4162,19 @@ call (virConnectPtr conn ATTRIBUTE_UNUSED, return rv; } +static int +call (virConnectPtr conn, + struct private_data *priv, + unsigned int flags, + int proc_nr, + xdrproc_t args_filter, char *args, + xdrproc_t ret_filter, char *ret) +{ + return callWithFD(conn, priv, flags, -1, proc_nr, + args_filter, args, + ret_filter, ret); +} + static void remoteDomainEventDispatchFunc(virConnectPtr conn, virDomainEventPtr event, @@ -4427,6 +4477,7 @@ static virDriver remote_driver = { .qemuDomainMonitorCommand = remoteQemuDomainMonitorCommand, /* 0.8.3 */ .qemuDomainAttach = qemuDomainAttach, /* 0.9.4 */ .domainOpenConsole = remoteDomainOpenConsole, /* 0.8.6 */ + .domainOpenGraphics = remoteDomainOpenGraphics, /* 0.9.7 */ .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 f95253e..bde3baf 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2253,6 +2253,12 @@ struct remote_domain_get_control_info_ret { /* insert@1 */ unsigned hyper stateTime; }; +struct remote_domain_open_graphics_args { + remote_nonnull_domain dom; + unsigned int idx; + unsigned int flags; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -2546,7 +2552,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244, /* autogen autogen priority:high */ REMOTE_PROC_DOMAIN_RESET = 245, /* autogen autogen */ REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN = 246, /* autogen autogen priority:high */ - REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247 /* autogen autogen priority:high */ + REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247, /* autogen autogen priority:high */ + REMOTE_PROC_DOMAIN_OPEN_GRAPHICS = 248 /* skipgen skipgen */ /* * Notice how the entries are grouped in sets of 10 ? -- 1.7.6.4
On 10/21/2011 06:56 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@redhat.com>
Since it needs to access file descriptors pass in the msg,
s/pass/passed/
the RPC driver for virDomainOpenGraphics needs to be manually implemented.
* daemon/remote.c: RPC server dispatcher * src/remote/remote_driver.c: RPC client dispatcher * src/remote/remote_protocol.x: Define protocol --- daemon/remote.c | 43 ++++++++++++++++++++++++ src/remote/remote_driver.c | 73 +++++++++++++++++++++++++++++++++++------ src/remote/remote_protocol.x | 9 ++++- 3 files changed, 113 insertions(+), 12 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 2507e91..291011a 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -44,6 +44,7 @@ #include "intprops.h" #include "virnetserverservice.h" #include "virnetserver.h" +#include "virfile.h"
#include "remote_protocol.h" #include "qemu_protocol.h" @@ -3124,6 +3125,48 @@ cleanup: }
+static int +remoteDispatchDomainOpenGraphics(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg, + virNetMessageErrorPtr rerr, + remote_domain_open_graphics_args *args) +{ + virDomainPtr dom = NULL; + int rv = -1; + int fd;
Must set fd to -1 here; otherwise, the first goto cleanup will end up calling VIR_FORCE_CLOSE on an uninit value, which could close an unintended fd.
+++ b/src/remote/remote_protocol.x @@ -2253,6 +2253,12 @@ struct remote_domain_get_control_info_ret { /* insert@1 */ unsigned hyper stateTime; };
+struct remote_domain_open_graphics_args { + remote_nonnull_domain dom; + unsigned int idx; + unsigned int flags; +}; + /*----- Protocol. -----*/
/* Define the program number, protocol version and procedure numbers here. */ @@ -2546,7 +2552,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT = 244, /* autogen autogen priority:high */ REMOTE_PROC_DOMAIN_RESET = 245, /* autogen autogen */ REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN = 246, /* autogen autogen priority:high */ - REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247 /* autogen autogen priority:high */ + REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247, /* autogen autogen priority:high */ + REMOTE_PROC_DOMAIN_OPEN_GRAPHICS = 248 /* skipgen skipgen */
/* * Notice how the entries are grouped in sets of 10 ?
Missing src/remote_protocol-structs changes. ACK with those problems fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel P. Berrange -
Eric Blake