* qemud/dispatch.c, qemud/dispatch.h: Generic code handling dispatch of RPC messages. * qemud/Makefile.am: Add dispatch.c to build * qemud/qemud.c: Include dispatch.h * qemud/qemud.h: Remove remoteDispatchClientRequest, remoteRelayDomainEvent now in dispatch.h * qemud/remote.c: Remove remoteDispatchClientRequest, remoteRelayDomainEvent now in dispatch.c, and dispatch_args, dispatch_ret, dispatch_fn & dispatch_data now in remote.h * qemud/remote.h: Add typedefs for dispatch_args, dispatch_ret, dispatch_fn, dispath_data. Add remoteGetDispatchData() API Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- qemud/Makefile.am | 3 +- qemud/dispatch.c | 302 +++++++++++++++++++++++++++++++++++++++++++++++++ qemud/dispatch.h | 58 ++++++++++ qemud/qemud.c | 4 +- qemud/qemud.h | 11 +-- qemud/remote.c | 325 +++-------------------------------------------------- qemud/remote.h | 72 ++++++++++++ 7 files changed, 454 insertions(+), 321 deletions(-) create mode 100644 qemud/dispatch.c create mode 100644 qemud/dispatch.h create mode 100644 qemud/remote.h diff --git a/qemud/Makefile.am b/qemud/Makefile.am index 403846a..74dfd22 100644 --- a/qemud/Makefile.am +++ b/qemud/Makefile.am @@ -3,7 +3,8 @@ DAEMON_SOURCES = \ event.c event.h \ qemud.c qemud.h \ - remote.c \ + remote.c remote.h \ + dispatch.c dispatch.h \ remote_dispatch_prototypes.h \ remote_dispatch_table.h \ remote_dispatch_args.h \ diff --git a/qemud/dispatch.c b/qemud/dispatch.c new file mode 100644 index 0000000..d2338fb --- /dev/null +++ b/qemud/dispatch.c @@ -0,0 +1,302 @@ +/* + * dispatch.h: RPC message dispatching infrastructure + * + * Copyright (C) 2007, 2008, 2009 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Richard W.M. Jones <rjones@redhat.com> + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> + + +#include "dispatch.h" +#include "remote.h" + +/* Convert a libvirt virError object into wire format */ +static void +remoteDispatchCopyError (remote_error *rerr, + virErrorPtr verr) +{ + rerr->code = verr->code; + rerr->domain = verr->domain; + rerr->message = verr->message ? malloc(sizeof(char*)) : NULL; + if (rerr->message) *rerr->message = strdup(verr->message); + rerr->level = verr->level; + rerr->str1 = verr->str1 ? malloc(sizeof(char*)) : NULL; + if (rerr->str1) *rerr->str1 = strdup(verr->str1); + rerr->str2 = verr->str2 ? malloc(sizeof(char*)) : NULL; + if (rerr->str2) *rerr->str2 = strdup(verr->str2); + rerr->str3 = verr->str3 ? malloc(sizeof(char*)) : NULL; + if (rerr->str3) *rerr->str3 = strdup(verr->str3); + rerr->int1 = verr->int1; + rerr->int2 = verr->int2; +} + + +/* A set of helpers for sending back errors to client + in various ways .... */ + +static void +remoteDispatchStringError (remote_error *rerr, + int code, const char *msg) +{ + virError verr; + + memset(&verr, 0, sizeof verr); + + /* Construct the dummy libvirt virError. */ + verr.code = code; + verr.domain = VIR_FROM_REMOTE; + verr.message = (char *)msg; + verr.level = VIR_ERR_ERROR; + verr.str1 = (char *)msg; + + remoteDispatchCopyError(rerr, &verr); +} + + +void remoteDispatchAuthError (remote_error *rerr) +{ + remoteDispatchStringError (rerr, VIR_ERR_AUTH_FAILED, "authentication failed"); +} + + +void remoteDispatchFormatError (remote_error *rerr, + const char *fmt, ...) +{ + va_list args; + char msgbuf[1024]; + char *msg = msgbuf; + + va_start (args, fmt); + vsnprintf (msgbuf, sizeof msgbuf, fmt, args); + va_end (args); + + remoteDispatchStringError (rerr, VIR_ERR_RPC, msg); +} + + +void remoteDispatchGenericError (remote_error *rerr) +{ + remoteDispatchStringError(rerr, + VIR_ERR_INTERNAL_ERROR, + "library function returned error but did not set virterror"); +} + + +void remoteDispatchOOMError (remote_error *rerr) +{ + remoteDispatchStringError(rerr, + VIR_ERR_NO_MEMORY, + NULL); +} + + +void remoteDispatchConnError (remote_error *rerr, + virConnectPtr conn) +{ + virErrorPtr verr; + + if (conn) + verr = virConnGetLastError(conn); + else + verr = virGetLastError(); + if (verr) + remoteDispatchCopyError(rerr, verr); + else + remoteDispatchGenericError(rerr); +} + + +/* + * @server: the unlocked server object + * @client: the locked client object + * @msg: the complete incoming message packet + * + * This function gets called from qemud when it pulls a incoming + * remote protocol messsage off the dispatch queue for processing. + * + * + * Returns 0 if the message was dispatched, -1 upon fatal error + */ +int +remoteDispatchClientRequest (struct qemud_server *server, + struct qemud_client *client, + struct qemud_client_message *msg) +{ + XDR xdr; + remote_message_header req, rep; + remote_error rerr; + dispatch_args args; + dispatch_ret ret; + const dispatch_data *data = NULL; + int rv = -1; + unsigned int len; + virConnectPtr conn = NULL; + + memset(&args, 0, sizeof args); + memset(&ret, 0, sizeof ret); + memset(&rerr, 0, sizeof rerr); + + /* Parse the header. */ + xdrmem_create (&xdr, + msg->buffer + REMOTE_MESSAGE_HEADER_XDR_LEN, + msg->bufferLength - REMOTE_MESSAGE_HEADER_XDR_LEN, + XDR_DECODE); + + if (!xdr_remote_message_header (&xdr, &req)) + goto fatal_error; + + /* Check version, etc. */ + if (req.prog != REMOTE_PROGRAM) { + remoteDispatchFormatError (&rerr, + _("program mismatch (actual %x, expected %x)"), + req.prog, REMOTE_PROGRAM); + goto rpc_error; + } + if (req.vers != REMOTE_PROTOCOL_VERSION) { + remoteDispatchFormatError (&rerr, + _("version mismatch (actual %x, expected %x)"), + req.vers, REMOTE_PROTOCOL_VERSION); + goto rpc_error; + } + if (req.direction != REMOTE_CALL) { + remoteDispatchFormatError (&rerr, _("direction (%d) != REMOTE_CALL"), + (int) req.direction); + goto rpc_error; + } + if (req.status != REMOTE_OK) { + remoteDispatchFormatError (&rerr, _("status (%d) != REMOTE_OK"), + (int) req.status); + goto rpc_error; + } + + /* If client is marked as needing auth, don't allow any RPC ops, + * except for authentication ones + */ + if (client->auth) { + if (req.proc != REMOTE_PROC_AUTH_LIST && + req.proc != REMOTE_PROC_AUTH_SASL_INIT && + req.proc != REMOTE_PROC_AUTH_SASL_START && + req.proc != REMOTE_PROC_AUTH_SASL_STEP && + req.proc != REMOTE_PROC_AUTH_POLKIT + ) { + /* Explicitly *NOT* calling remoteDispatchAuthError() because + we want back-compatability with libvirt clients which don't + support the VIR_ERR_AUTH_FAILED error code */ + remoteDispatchFormatError (&rerr, "%s", _("authentication required")); + goto rpc_error; + } + } + + data = remoteGetDispatchData(req.proc); + + if (!data) { + remoteDispatchFormatError (&rerr, _("unknown procedure: %d"), + req.proc); + goto rpc_error; + } + + /* De-serialize args off the wire */ + if (!((data->args_filter)(&xdr, &args))) { + remoteDispatchFormatError (&rerr, "%s", _("parse args failed")); + goto rpc_error; + } + + /* Call function. */ + conn = client->conn; + virMutexUnlock(&client->lock); + + /* + * When the RPC handler is called: + * + * - Server object is unlocked + * - Client object is unlocked + * + * Without locking, it is safe to use: + * + * 'conn', 'rerr', 'args and 'ret' + */ + rv = (data->fn)(server, client, conn, &rerr, &args, &ret); + + virMutexLock(&server->lock); + virMutexLock(&client->lock); + virMutexUnlock(&server->lock); + + xdr_free (data->args_filter, (char*)&args); + +rpc_error: + xdr_destroy (&xdr); + + /* Return header. */ + rep.prog = req.prog; + rep.vers = req.vers; + rep.proc = req.proc; + rep.direction = REMOTE_REPLY; + rep.serial = req.serial; + rep.status = rv < 0 ? REMOTE_ERROR : REMOTE_OK; + + /* Serialise the return header. */ + xdrmem_create (&xdr, msg->buffer, sizeof msg->buffer, XDR_ENCODE); + + len = 0; /* We'll come back and write this later. */ + if (!xdr_u_int (&xdr, &len)) { + if (rv == 0) xdr_free (data->ret_filter, (char*)&ret); + goto fatal_error; + } + + if (!xdr_remote_message_header (&xdr, &rep)) { + if (rv == 0) xdr_free (data->ret_filter, (char*)&ret); + goto fatal_error; + } + + /* If OK, serialise return structure, if error serialise error. */ + if (rv >= 0) { + if (!((data->ret_filter) (&xdr, &ret))) + goto fatal_error; + xdr_free (data->ret_filter, (char*)&ret); + } else /* error */ { + /* Error was NULL so synthesize an error. */ + if (rerr.code == 0) + remoteDispatchGenericError(&rerr); + if (!xdr_remote_error (&xdr, &rerr)) + goto fatal_error; + xdr_free((xdrproc_t)xdr_remote_error, (char *)&rerr); + } + + /* Write the length word. */ + len = xdr_getpos (&xdr); + if (xdr_setpos (&xdr, 0) == 0) + goto fatal_error; + + if (!xdr_u_int (&xdr, &len)) + goto fatal_error; + + xdr_destroy (&xdr); + + msg->bufferLength = len; + msg->bufferOffset = 0; + + return 0; + +fatal_error: + /* Seriously bad stuff happened, so we'll kill off this client + and not send back any RPC error */ + xdr_destroy (&xdr); + return -1; +} diff --git a/qemud/dispatch.h b/qemud/dispatch.h new file mode 100644 index 0000000..9ab6148 --- /dev/null +++ b/qemud/dispatch.h @@ -0,0 +1,58 @@ +/* + * dispatch.h: RPC message dispatching infrastructure + * + * Copyright (C) 2007, 2008, 2009 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Richard W.M. Jones <rjones@redhat.com> + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#ifndef __LIBVIRTD_DISPATCH_H__ +#define __LIBVIRTD_DISPATCH_H__ + + +#include "qemud.h" + + +int +remoteDispatchClientRequest (struct qemud_server *server, + struct qemud_client *client, + struct qemud_client_message *req); + + +void remoteDispatchFormatError (remote_error *rerr, + const char *fmt, ...) + ATTRIBUTE_FORMAT(printf, 2, 3); + +void remoteDispatchAuthError (remote_error *rerr); +void remoteDispatchGenericError (remote_error *rerr); +void remoteDispatchOOMError (remote_error *rerr); +void remoteDispatchConnError (remote_error *rerr, + virConnectPtr conn); + +/* Having this here is dubious. It should be in remote.h + * but qemud.c shouldn't depend on that header directly. + * Refactor this later to deal with this properly. + */ +int remoteRelayDomainEvent (virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + int event, + int detail, + void *opaque); + + +#endif /* __LIBVIRTD_DISPATCH_H__ */ diff --git a/qemud/qemud.c b/qemud/qemud.c index da20aa9..d300c56 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -54,6 +54,8 @@ #define VIR_FROM_THIS VIR_FROM_QEMU #include "qemud.h" +#include "dispatch.h" + #include "util.h" #include "remote_internal.h" #include "conf.h" @@ -219,7 +221,7 @@ qemudClientMessageQueuePush(struct qemud_client_message **queue, } } -static struct qemud_client_message * +struct qemud_client_message * qemudClientMessageQueueServe(struct qemud_client_message **queue) { struct qemud_client_message *tmp = *queue; diff --git a/qemud/qemud.h b/qemud/qemud.h index 8880337..c8273cb 100644 --- a/qemud/qemud.h +++ b/qemud/qemud.h @@ -200,10 +200,6 @@ void qemudLog(int priority, const char *fmt, ...) ATTRIBUTE_FORMAT(printf,2,3); -int -remoteDispatchClientRequest (struct qemud_server *server, - struct qemud_client *client, - struct qemud_client_message *req); int qemudRegisterClientEvent(struct qemud_server *server, struct qemud_client *client, @@ -214,12 +210,9 @@ void qemudDispatchClientFailure(struct qemud_client *client); void qemudClientMessageQueuePush(struct qemud_client_message **queue, struct qemud_client_message *msg); +struct qemud_client_message * +qemudClientMessageQueueServe(struct qemud_client_message **queue); -int remoteRelayDomainEvent (virConnectPtr conn ATTRIBUTE_UNUSED, - virDomainPtr dom, - int event, - int detail, - void *opaque); #if HAVE_POLKIT diff --git a/qemud/remote.c b/qemud/remote.c index 1071c21..d0bc677 100644 --- a/qemud/remote.c +++ b/qemud/remote.c @@ -1,5 +1,5 @@ /* - * remote.c: code handling remote requests (from remote_internal.c) + * remote.c: handlers for RPC method calls * * Copyright (C) 2007, 2008, 2009 Red Hat, Inc. * @@ -48,18 +48,17 @@ #include <polkit-dbus/polkit-dbus.h> #endif +#include "remote.h" +#include "dispatch.h" + #include "libvirt_internal.h" #include "datatypes.h" -#include "qemud.h" #include "memory.h" #include "util.h" #define VIR_FROM_THIS VIR_FROM_REMOTE #define REMOTE_DEBUG(fmt, ...) DEBUG(fmt, __VA_ARGS__) -static void remoteDispatchFormatError (remote_error *rerr, - const char *fmt, ...) - ATTRIBUTE_FORMAT(printf, 2, 3); static virDomainPtr get_nonnull_domain (virConnectPtr conn, remote_nonnull_domain domain); static virNetworkPtr get_nonnull_network (virConnectPtr conn, remote_nonnull_network network); static virInterfacePtr get_nonnull_interface (virConnectPtr conn, remote_nonnull_interface interface); @@ -72,47 +71,23 @@ static void make_nonnull_storage_pool (remote_nonnull_storage_pool *pool_dst, vi static void make_nonnull_storage_vol (remote_nonnull_storage_vol *vol_dst, virStorageVolPtr vol_src); static void make_nonnull_node_device (remote_nonnull_node_device *dev_dst, virNodeDevicePtr dev_src); -#include "remote_dispatch_prototypes.h" - -typedef union { -#include "remote_dispatch_args.h" -} dispatch_args; - -typedef union { -#include "remote_dispatch_ret.h" -} dispatch_ret; - - -/** - * When the RPC handler is called: - * - * - Server object is unlocked - * - Client object is unlocked - * - * Both must be locked before use. Server lock must - * be held before attempting to lock client. - * - * Without any locking, it is safe to use: - * - * 'conn', 'rerr', 'args and 'ret' - */ -typedef int (*dispatch_fn) (struct qemud_server *server, - struct qemud_client *client, - virConnectPtr conn, - remote_error *err, - dispatch_args *args, - dispatch_ret *ret); -typedef struct { - dispatch_fn fn; - xdrproc_t args_filter; - xdrproc_t ret_filter; -} dispatch_data; +#include "remote_dispatch_prototypes.h" static const dispatch_data const dispatch_table[] = { #include "remote_dispatch_table.h" }; +const dispatch_data const *remoteGetDispatchData(int proc) +{ + if (proc >= ARRAY_CARDINALITY(dispatch_table) || + dispatch_table[proc].fn == NULL) { + return NULL; + } + + return &(dispatch_table[proc]); +} + /* Prototypes */ static void remoteDispatchDomainEventSend (struct qemud_client *client, @@ -122,276 +97,6 @@ remoteDispatchDomainEventSend (struct qemud_client *client, int detail); -/* Convert a libvirt virError object into wire format */ -static void -remoteDispatchCopyError (remote_error *rerr, - virErrorPtr verr) -{ - rerr->code = verr->code; - rerr->domain = verr->domain; - rerr->message = verr->message ? malloc(sizeof(char*)) : NULL; - if (rerr->message) *rerr->message = strdup(verr->message); - rerr->level = verr->level; - rerr->str1 = verr->str1 ? malloc(sizeof(char*)) : NULL; - if (rerr->str1) *rerr->str1 = strdup(verr->str1); - rerr->str2 = verr->str2 ? malloc(sizeof(char*)) : NULL; - if (rerr->str2) *rerr->str2 = strdup(verr->str2); - rerr->str3 = verr->str3 ? malloc(sizeof(char*)) : NULL; - if (rerr->str3) *rerr->str3 = strdup(verr->str3); - rerr->int1 = verr->int1; - rerr->int2 = verr->int2; -} - - -/* A set of helpers for sending back errors to client - in various ways .... */ - -static void -remoteDispatchStringError (remote_error *rerr, - int code, const char *msg) -{ - virError verr; - - memset(&verr, 0, sizeof verr); - - /* Construct the dummy libvirt virError. */ - verr.code = code; - verr.domain = VIR_FROM_REMOTE; - verr.message = (char *)msg; - verr.level = VIR_ERR_ERROR; - verr.str1 = (char *)msg; - - remoteDispatchCopyError(rerr, &verr); -} - -static void -remoteDispatchAuthError (remote_error *rerr) -{ - remoteDispatchStringError (rerr, VIR_ERR_AUTH_FAILED, "authentication failed"); -} - -static void -remoteDispatchFormatError (remote_error *rerr, - const char *fmt, ...) -{ - va_list args; - char msgbuf[1024]; - char *msg = msgbuf; - - va_start (args, fmt); - vsnprintf (msgbuf, sizeof msgbuf, fmt, args); - va_end (args); - - remoteDispatchStringError (rerr, VIR_ERR_RPC, msg); -} - -static void -remoteDispatchGenericError (remote_error *rerr) -{ - remoteDispatchStringError(rerr, - VIR_ERR_INTERNAL_ERROR, - "library function returned error but did not set virterror"); -} - -static void -remoteDispatchOOMError (remote_error *rerr) -{ - remoteDispatchStringError(rerr, - VIR_ERR_NO_MEMORY, - NULL); -} - -static void -remoteDispatchConnError (remote_error *rerr, - virConnectPtr conn) -{ - virErrorPtr verr; - - if (conn) - verr = virConnGetLastError(conn); - else - verr = virGetLastError(); - if (verr) - remoteDispatchCopyError(rerr, verr); - else - remoteDispatchGenericError(rerr); -} - - -/* This function gets called from qemud when it detects an incoming - * remote protocol message. At this point, client->buffer contains - * the full call message (including length word which we skip). - * - * Server object is unlocked - * Client object is locked - */ -int -remoteDispatchClientRequest (struct qemud_server *server, - struct qemud_client *client, - struct qemud_client_message *msg) -{ - XDR xdr; - remote_message_header req, rep; - remote_error rerr; - dispatch_args args; - dispatch_ret ret; - const dispatch_data *data = NULL; - int rv = -1; - unsigned int len; - virConnectPtr conn = NULL; - - memset(&args, 0, sizeof args); - memset(&ret, 0, sizeof ret); - memset(&rerr, 0, sizeof rerr); - - /* Parse the header. */ - xdrmem_create (&xdr, - msg->buffer + REMOTE_MESSAGE_HEADER_XDR_LEN, - msg->bufferLength - REMOTE_MESSAGE_HEADER_XDR_LEN, - XDR_DECODE); - - if (!xdr_remote_message_header (&xdr, &req)) - goto fatal_error; - - /* Check version, etc. */ - if (req.prog != REMOTE_PROGRAM) { - remoteDispatchFormatError (&rerr, - _("program mismatch (actual %x, expected %x)"), - req.prog, REMOTE_PROGRAM); - goto rpc_error; - } - if (req.vers != REMOTE_PROTOCOL_VERSION) { - remoteDispatchFormatError (&rerr, - _("version mismatch (actual %x, expected %x)"), - req.vers, REMOTE_PROTOCOL_VERSION); - goto rpc_error; - } - if (req.direction != REMOTE_CALL) { - remoteDispatchFormatError (&rerr, _("direction (%d) != REMOTE_CALL"), - (int) req.direction); - goto rpc_error; - } - if (req.status != REMOTE_OK) { - remoteDispatchFormatError (&rerr, _("status (%d) != REMOTE_OK"), - (int) req.status); - goto rpc_error; - } - - /* If client is marked as needing auth, don't allow any RPC ops, - * except for authentication ones - */ - if (client->auth) { - if (req.proc != REMOTE_PROC_AUTH_LIST && - req.proc != REMOTE_PROC_AUTH_SASL_INIT && - req.proc != REMOTE_PROC_AUTH_SASL_START && - req.proc != REMOTE_PROC_AUTH_SASL_STEP && - req.proc != REMOTE_PROC_AUTH_POLKIT - ) { - /* Explicitly *NOT* calling remoteDispatchAuthError() because - we want back-compatability with libvirt clients which don't - support the VIR_ERR_AUTH_FAILED error code */ - remoteDispatchFormatError (&rerr, "%s", _("authentication required")); - goto rpc_error; - } - } - - if (req.proc >= ARRAY_CARDINALITY(dispatch_table) || - dispatch_table[req.proc].fn == NULL) { - remoteDispatchFormatError (&rerr, _("unknown procedure: %d"), - req.proc); - goto rpc_error; - } - - data = &(dispatch_table[req.proc]); - - /* De-serialize args off the wire */ - if (!((data->args_filter)(&xdr, &args))) { - remoteDispatchFormatError (&rerr, "%s", _("parse args failed")); - goto rpc_error; - } - - /* Call function. */ - conn = client->conn; - virMutexUnlock(&client->lock); - - /* - * When the RPC handler is called: - * - * - Server object is unlocked - * - Client object is unlocked - * - * Without locking, it is safe to use: - * - * 'conn', 'rerr', 'args and 'ret' - */ - rv = (data->fn)(server, client, conn, &rerr, &args, &ret); - - virMutexLock(&server->lock); - virMutexLock(&client->lock); - virMutexUnlock(&server->lock); - - xdr_free (data->args_filter, (char*)&args); - -rpc_error: - xdr_destroy (&xdr); - - /* Return header. */ - rep.prog = req.prog; - rep.vers = req.vers; - rep.proc = req.proc; - rep.direction = REMOTE_REPLY; - rep.serial = req.serial; - rep.status = rv < 0 ? REMOTE_ERROR : REMOTE_OK; - - /* Serialise the return header. */ - xdrmem_create (&xdr, msg->buffer, sizeof msg->buffer, XDR_ENCODE); - - len = 0; /* We'll come back and write this later. */ - if (!xdr_u_int (&xdr, &len)) { - if (rv == 0) xdr_free (data->ret_filter, (char*)&ret); - goto fatal_error; - } - - if (!xdr_remote_message_header (&xdr, &rep)) { - if (rv == 0) xdr_free (data->ret_filter, (char*)&ret); - goto fatal_error; - } - - /* If OK, serialise return structure, if error serialise error. */ - if (rv >= 0) { - if (!((data->ret_filter) (&xdr, &ret))) - goto fatal_error; - xdr_free (data->ret_filter, (char*)&ret); - } else /* error */ { - /* Error was NULL so synthesize an error. */ - if (rerr.code == 0) - remoteDispatchGenericError(&rerr); - if (!xdr_remote_error (&xdr, &rerr)) - goto fatal_error; - xdr_free((xdrproc_t)xdr_remote_error, (char *)&rerr); - } - - /* Write the length word. */ - len = xdr_getpos (&xdr); - if (xdr_setpos (&xdr, 0) == 0) - goto fatal_error; - - if (!xdr_u_int (&xdr, &len)) - goto fatal_error; - - xdr_destroy (&xdr); - - msg->bufferLength = len; - msg->bufferOffset = 0; - - return 0; - -fatal_error: - /* Seriously bad stuff happened, so we'll kill off this client - and not send back any RPC error */ - xdr_destroy (&xdr); - return -1; -} int remoteRelayDomainEvent (virConnectPtr conn ATTRIBUTE_UNUSED, virDomainPtr dom, diff --git a/qemud/remote.h b/qemud/remote.h new file mode 100644 index 0000000..e3ee696 --- /dev/null +++ b/qemud/remote.h @@ -0,0 +1,72 @@ +/* + * remote.h: handlers for RPC method calls + * + * Copyright (C) 2007, 2008, 2009 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Richard W.M. Jones <rjones@redhat.com> + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#ifndef __LIBVIRTD_REMOTE_H__ +#define __LIBVIRTD_REMOTE_H__ + + +#include "qemud.h" + +typedef union { +#include "remote_dispatch_args.h" +} dispatch_args; + +typedef union { +#include "remote_dispatch_ret.h" +} dispatch_ret; + + + + +/** + * When the RPC handler is called: + * + * - Server object is unlocked + * - Client object is unlocked + * + * Both must be locked before use. Server lock must + * be held before attempting to lock client. + * + * Without any locking, it is safe to use: + * + * 'conn', 'rerr', 'args and 'ret' + */ +typedef int (*dispatch_fn) (struct qemud_server *server, + struct qemud_client *client, + virConnectPtr conn, + remote_error *err, + dispatch_args *args, + dispatch_ret *ret); + +typedef struct { + dispatch_fn fn; + xdrproc_t args_filter; + xdrproc_t ret_filter; +} dispatch_data; + + +const dispatch_data const *remoteGetDispatchData(int proc); + + + +#endif /* __LIBVIRTD_REMOTE_H__ */ -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Separate the decoding of incoming request header out from the dispatch code. This will allow later code to making dispatcher routing decisions based on the header field data. * qemud/dispatch.c, qemud/dispatch.h: Add remoteDecodeClientMessageHeader API for decoding the header of a client message. Update the remoteDispatchClientRequest method to assume a pre-decoded header. * qemud/qemud.h: Include a 'remote_message_header' field in 'struct qemud_client_message' for pre-decoded header data * qemud/qemud.c: Decode the incoming client message header before invoking remoteDispatchClientRequest Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- qemud/dispatch.c | 96 ++++++++++++++++++++++++++++++++++++----------------- qemud/dispatch.h | 3 ++ qemud/qemud.c | 3 +- qemud/qemud.h | 2 + 4 files changed, 72 insertions(+), 32 deletions(-) diff --git a/qemud/dispatch.c b/qemud/dispatch.c index d2338fb..127f93b 100644 --- a/qemud/dispatch.c +++ b/qemud/dispatch.c @@ -124,13 +124,50 @@ void remoteDispatchConnError (remote_error *rerr, /* + * @msg: the complete incoming message, whose header to decode + * + * Decodes the header part of the client message, but does not + * validate the decoded fields in the header. + * + * returns 0 if successfully decoded, -1 upon fatal error + */ +int +remoteDecodeClientMessageHeader (struct qemud_client_message *msg) +{ + XDR xdr; + int ret = -1; + + msg->bufferOffset = REMOTE_MESSAGE_HEADER_XDR_LEN; + + /* Parse the header. */ + xdrmem_create (&xdr, + msg->buffer + msg->bufferOffset, + msg->bufferLength - msg->bufferOffset, + XDR_DECODE); + + if (!xdr_remote_message_header (&xdr, &msg->hdr)) + goto cleanup; + + msg->bufferOffset += xdr_getpos(&xdr); + + ret = 0; + +cleanup: + xdr_destroy(&xdr); + return ret; +} + + +/* * @server: the unlocked server object * @client: the locked client object - * @msg: the complete incoming message packet + * @msg: the complete incoming message packet, with header already decoded * * This function gets called from qemud when it pulls a incoming * remote protocol messsage off the dispatch queue for processing. * + * The @msg parameter must have had its header decoded already by + * calling remoteDecodeClientMessageHeader * * Returns 0 if the message was dispatched, -1 upon fatal error */ @@ -140,7 +177,7 @@ remoteDispatchClientRequest (struct qemud_server *server, struct qemud_client_message *msg) { XDR xdr; - remote_message_header req, rep; + remote_message_header rep; remote_error rerr; dispatch_args args; dispatch_ret ret; @@ -153,36 +190,28 @@ remoteDispatchClientRequest (struct qemud_server *server, memset(&ret, 0, sizeof ret); memset(&rerr, 0, sizeof rerr); - /* Parse the header. */ - xdrmem_create (&xdr, - msg->buffer + REMOTE_MESSAGE_HEADER_XDR_LEN, - msg->bufferLength - REMOTE_MESSAGE_HEADER_XDR_LEN, - XDR_DECODE); - - if (!xdr_remote_message_header (&xdr, &req)) - goto fatal_error; /* Check version, etc. */ - if (req.prog != REMOTE_PROGRAM) { + if (msg->hdr.prog != REMOTE_PROGRAM) { remoteDispatchFormatError (&rerr, _("program mismatch (actual %x, expected %x)"), - req.prog, REMOTE_PROGRAM); + msg->hdr.prog, REMOTE_PROGRAM); goto rpc_error; } - if (req.vers != REMOTE_PROTOCOL_VERSION) { + if (msg->hdr.vers != REMOTE_PROTOCOL_VERSION) { remoteDispatchFormatError (&rerr, _("version mismatch (actual %x, expected %x)"), - req.vers, REMOTE_PROTOCOL_VERSION); + msg->hdr.vers, REMOTE_PROTOCOL_VERSION); goto rpc_error; } - if (req.direction != REMOTE_CALL) { + if (msg->hdr.direction != REMOTE_CALL) { remoteDispatchFormatError (&rerr, _("direction (%d) != REMOTE_CALL"), - (int) req.direction); + (int) msg->hdr.direction); goto rpc_error; } - if (req.status != REMOTE_OK) { + if (msg->hdr.status != REMOTE_OK) { remoteDispatchFormatError (&rerr, _("status (%d) != REMOTE_OK"), - (int) req.status); + (int) msg->hdr.status); goto rpc_error; } @@ -190,11 +219,11 @@ remoteDispatchClientRequest (struct qemud_server *server, * except for authentication ones */ if (client->auth) { - if (req.proc != REMOTE_PROC_AUTH_LIST && - req.proc != REMOTE_PROC_AUTH_SASL_INIT && - req.proc != REMOTE_PROC_AUTH_SASL_START && - req.proc != REMOTE_PROC_AUTH_SASL_STEP && - req.proc != REMOTE_PROC_AUTH_POLKIT + if (msg->hdr.proc != REMOTE_PROC_AUTH_LIST && + msg->hdr.proc != REMOTE_PROC_AUTH_SASL_INIT && + msg->hdr.proc != REMOTE_PROC_AUTH_SASL_START && + msg->hdr.proc != REMOTE_PROC_AUTH_SASL_STEP && + msg->hdr.proc != REMOTE_PROC_AUTH_POLKIT ) { /* Explicitly *NOT* calling remoteDispatchAuthError() because we want back-compatability with libvirt clients which don't @@ -204,19 +233,25 @@ remoteDispatchClientRequest (struct qemud_server *server, } } - data = remoteGetDispatchData(req.proc); + data = remoteGetDispatchData(msg->hdr.proc); if (!data) { remoteDispatchFormatError (&rerr, _("unknown procedure: %d"), - req.proc); + msg->hdr.proc); goto rpc_error; } - /* De-serialize args off the wire */ + /* De-serialize payload with args from the wire message */ + xdrmem_create (&xdr, + msg->buffer + msg->bufferOffset, + msg->bufferLength - msg->bufferOffset, + XDR_DECODE); if (!((data->args_filter)(&xdr, &args))) { + xdr_destroy (&xdr); remoteDispatchFormatError (&rerr, "%s", _("parse args failed")); goto rpc_error; } + xdr_destroy (&xdr); /* Call function. */ conn = client->conn; @@ -241,14 +276,13 @@ remoteDispatchClientRequest (struct qemud_server *server, xdr_free (data->args_filter, (char*)&args); rpc_error: - xdr_destroy (&xdr); /* Return header. */ - rep.prog = req.prog; - rep.vers = req.vers; - rep.proc = req.proc; + rep.prog = msg->hdr.prog; + rep.vers = msg->hdr.vers; + rep.proc = msg->hdr.proc; rep.direction = REMOTE_REPLY; - rep.serial = req.serial; + rep.serial = msg->hdr.serial; rep.status = rv < 0 ? REMOTE_ERROR : REMOTE_OK; /* Serialise the return header. */ diff --git a/qemud/dispatch.h b/qemud/dispatch.h index 9ab6148..db372f1 100644 --- a/qemud/dispatch.h +++ b/qemud/dispatch.h @@ -29,6 +29,9 @@ int +remoteDecodeClientMessageHeader (struct qemud_client_message *req); + +int remoteDispatchClientRequest (struct qemud_server *server, struct qemud_client *client, struct qemud_client_message *req); diff --git a/qemud/qemud.c b/qemud/qemud.c index d300c56..c577d88 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -1458,7 +1458,8 @@ static void *qemudWorker(void *data) /* This function drops the lock during dispatch, * and re-acquires it before returning */ - if (remoteDispatchClientRequest (server, client, reply) < 0) { + if (remoteDecodeClientMessageHeader(reply) < 0 || + remoteDispatchClientRequest (server, client, reply) < 0) { VIR_FREE(reply); qemudDispatchClientFailure(client); client->refs--; diff --git a/qemud/qemud.h b/qemud/qemud.h index c8273cb..6597429 100644 --- a/qemud/qemud.h +++ b/qemud/qemud.h @@ -85,6 +85,8 @@ struct qemud_client_message { int async : 1; + remote_message_header hdr; + struct qemud_client_message *next; }; -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Introduces an API for encoding the header field for outgoing messages allowing some duplicated code to be eliminated * qemud/dispatch.c, qemud/dispatch.h: add remoteEncodeClientMessageHeader for encoding message header. Update remoteDispatchClientRequest to use this method. * qemud/remote.c: Update remoteDispatchDomainEventSend to use the generic remoteEncodeClientMessageHeader() for encoding event message hedaders. Push some logic from remoteRelayDomainEvent down into remoteDispatchDomainEventSend. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- qemud/dispatch.c | 99 +++++++++++++++++++++++++++++++++++++++++++---------- qemud/dispatch.h | 2 + qemud/remote.c | 79 ++++++++++++++++++------------------------- 3 files changed, 115 insertions(+), 65 deletions(-) diff --git a/qemud/dispatch.c b/qemud/dispatch.c index 127f93b..29970e4 100644 --- a/qemud/dispatch.c +++ b/qemud/dispatch.c @@ -127,7 +127,10 @@ void remoteDispatchConnError (remote_error *rerr, * @msg: the complete incoming message, whose header to decode * * Decodes the header part of the client message, but does not - * validate the decoded fields in the header. + * validate the decoded fields in the header. It expects + * bufferLength to refer to length of the data packet. Upon + * return bufferOffset will refer to the amount of the packet + * consumed by decoding of the header. * * returns 0 if successfully decoded, -1 upon fatal error */ @@ -159,6 +162,61 @@ cleanup: /* + * @msg: the outgoing message, whose header to encode + * + * Encodes the header part of the client message, setting the + * message offset ready to encode the payload. Leaves space + * for the length field later. Upon return bufferLength will + * refer to the total available space for message, while + * bufferOffset will refer to current space used by header + * + * returns 0 if successfully encoded, -1 upon fatal error + */ +int +remoteEncodeClientMessageHeader (struct qemud_client_message *msg) +{ + XDR xdr; + int ret = -1; + unsigned int len = 0; + + msg->bufferLength = sizeof(msg->buffer); + msg->bufferOffset = 0; + + /* Format the header. */ + xdrmem_create (&xdr, + msg->buffer, + msg->bufferLength, + XDR_ENCODE); + + /* The real value is filled in shortly */ + if (!xdr_u_int (&xdr, &len)) { + goto cleanup; + } + + if (!xdr_remote_message_header (&xdr, &msg->hdr)) + goto cleanup; + + len = xdr_getpos(&xdr); + xdr_setpos(&xdr, 0); + + /* Fill in current length - may be re-written later + * if a payload is added + */ + if (!xdr_u_int (&xdr, &len)) { + goto cleanup; + } + + msg->bufferOffset += len; + + ret = 0; + +cleanup: + xdr_destroy(&xdr); + return ret; +} + + +/* * @server: the unlocked server object * @client: the locked client object * @msg: the complete incoming message packet, with header already decoded @@ -177,7 +235,6 @@ remoteDispatchClientRequest (struct qemud_server *server, struct qemud_client_message *msg) { XDR xdr; - remote_message_header rep; remote_error rerr; dispatch_args args; dispatch_ret ret; @@ -277,27 +334,29 @@ remoteDispatchClientRequest (struct qemud_server *server, rpc_error: - /* Return header. */ - rep.prog = msg->hdr.prog; - rep.vers = msg->hdr.vers; - rep.proc = msg->hdr.proc; - rep.direction = REMOTE_REPLY; - rep.serial = msg->hdr.serial; - rep.status = rv < 0 ? REMOTE_ERROR : REMOTE_OK; + /* Return header. We're re-using same message object, so + * only need to tweak direction/status fields */ + /*msg->hdr.prog = msg->hdr.prog;*/ + /*msg->hdr.vers = msg->hdr.vers;*/ + /*msg->hdr.proc = msg->hdr.proc;*/ + msg->hdr.direction = REMOTE_REPLY; + /*msg->hdr.serial = msg->hdr.serial;*/ + msg->hdr.status = rv < 0 ? REMOTE_ERROR : REMOTE_OK; - /* Serialise the return header. */ - xdrmem_create (&xdr, msg->buffer, sizeof msg->buffer, XDR_ENCODE); - - len = 0; /* We'll come back and write this later. */ - if (!xdr_u_int (&xdr, &len)) { + if (remoteEncodeClientMessageHeader(msg) < 0) { if (rv == 0) xdr_free (data->ret_filter, (char*)&ret); goto fatal_error; } - if (!xdr_remote_message_header (&xdr, &rep)) { - if (rv == 0) xdr_free (data->ret_filter, (char*)&ret); + + /* Now for the payload */ + xdrmem_create (&xdr, + msg->buffer, + msg->bufferLength, + XDR_ENCODE); + + if (xdr_setpos(&xdr, msg->bufferOffset) == 0) goto fatal_error; - } /* If OK, serialise return structure, if error serialise error. */ if (rv >= 0) { @@ -313,8 +372,9 @@ rpc_error: xdr_free((xdrproc_t)xdr_remote_error, (char *)&rerr); } - /* Write the length word. */ - len = xdr_getpos (&xdr); + /* Update the length word. */ + msg->bufferOffset += xdr_getpos (&xdr); + len = msg->bufferOffset; if (xdr_setpos (&xdr, 0) == 0) goto fatal_error; @@ -323,6 +383,7 @@ rpc_error: xdr_destroy (&xdr); + /* Reset ready for I/O */ msg->bufferLength = len; msg->bufferOffset = 0; diff --git a/qemud/dispatch.h b/qemud/dispatch.h index db372f1..ab45b19 100644 --- a/qemud/dispatch.h +++ b/qemud/dispatch.h @@ -30,6 +30,8 @@ int remoteDecodeClientMessageHeader (struct qemud_client_message *req); +int +remoteEncodeClientMessageHeader (struct qemud_client_message *req); int remoteDispatchClientRequest (struct qemud_server *server, diff --git a/qemud/remote.c b/qemud/remote.c index d0bc677..fb06dc3 100644 --- a/qemud/remote.c +++ b/qemud/remote.c @@ -91,7 +91,6 @@ const dispatch_data const *remoteGetDispatchData(int proc) /* Prototypes */ static void remoteDispatchDomainEventSend (struct qemud_client *client, - struct qemud_client_message *msg, virDomainPtr dom, int event, int detail); @@ -108,14 +107,9 @@ int remoteRelayDomainEvent (virConnectPtr conn ATTRIBUTE_UNUSED, REMOTE_DEBUG("Relaying domain event %d %d", event, detail); if (client) { - struct qemud_client_message *ev; - - if (VIR_ALLOC(ev) < 0) - return -1; - virMutexLock(&client->lock); - remoteDispatchDomainEventSend (client, ev, dom, event, detail); + remoteDispatchDomainEventSend (client, dom, event, detail); if (qemudRegisterClientEvent(client->server, client, 1) < 0) qemudDispatchClientFailure(client); @@ -4433,72 +4427,65 @@ remoteDispatchDomainEventsDeregister (struct qemud_server *server ATTRIBUTE_UNUS static void remoteDispatchDomainEventSend (struct qemud_client *client, - struct qemud_client_message *msg, virDomainPtr dom, int event, int detail) { - remote_message_header rep; + struct qemud_client_message *msg = NULL; XDR xdr; unsigned int len; remote_domain_event_ret data; - if (!client) + if (VIR_ALLOC(msg) < 0) return; - rep.prog = REMOTE_PROGRAM; - rep.vers = REMOTE_PROTOCOL_VERSION; - rep.proc = REMOTE_PROC_DOMAIN_EVENT; - rep.direction = REMOTE_MESSAGE; - rep.serial = 1; - rep.status = REMOTE_OK; + msg->hdr.prog = REMOTE_PROGRAM; + msg->hdr.vers = REMOTE_PROTOCOL_VERSION; + msg->hdr.proc = REMOTE_PROC_DOMAIN_EVENT; + msg->hdr.direction = REMOTE_MESSAGE; + msg->hdr.serial = 1; + msg->hdr.status = REMOTE_OK; - /* Serialise the return header and event. */ - xdrmem_create (&xdr, msg->buffer, sizeof msg->buffer, XDR_ENCODE); - - len = 0; /* We'll come back and write this later. */ - if (!xdr_u_int (&xdr, &len)) { - /*remoteDispatchError (client, NULL, "%s", _("xdr_u_int failed (1)"));*/ - xdr_destroy (&xdr); - return; - } + if (remoteEncodeClientMessageHeader(msg) < 0) + goto error; - if (!xdr_remote_message_header (&xdr, &rep)) { - xdr_destroy (&xdr); - return; - } + /* Serialise the return header and event. */ + xdrmem_create (&xdr, + msg->buffer + msg->bufferOffset, + msg->bufferLength - msg->bufferOffset, + XDR_ENCODE); /* build return data */ make_nonnull_domain (&data.dom, dom); data.event = event; data.detail = detail; - if (!xdr_remote_domain_event_ret(&xdr, &data)) { - /*remoteDispatchError (client, NULL, "%s", _("serialise return struct"));*/ - xdr_destroy (&xdr); - return; - } + if (!xdr_remote_domain_event_ret(&xdr, &data)) + goto xdr_error; - len = xdr_getpos (&xdr); - if (xdr_setpos (&xdr, 0) == 0) { - /*remoteDispatchError (client, NULL, "%s", _("xdr_setpos failed"));*/ - xdr_destroy (&xdr); - return; - } - if (!xdr_u_int (&xdr, &len)) { - /*remoteDispatchError (client, NULL, "%s", _("xdr_u_int failed (2)"));*/ - xdr_destroy (&xdr); - return; - } + /* Update length word */ + msg->bufferOffset += xdr_getpos (&xdr); + len = msg->bufferOffset; + if (xdr_setpos (&xdr, 0) == 0) + goto xdr_error; - xdr_destroy (&xdr); + if (!xdr_u_int (&xdr, &len)) + goto xdr_error; /* Send it. */ msg->async = 1; msg->bufferLength = len; msg->bufferOffset = 0; qemudClientMessageQueuePush(&client->tx, msg); + + xdr_destroy (&xdr); + return; + +xdr_error: + xdr_destroy(&xdr); +error: + VIR_FREE(msg); } /*----- Helpers. -----*/ -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

The naming convention for structs used in the RPC layer is for incoming requests to be called XXXX_args, and the associated outgoing reply to be called XXXX_ret. Asynchronously emitted messages (eg events) are re-using the XXXX_ret naming scheme. This patch changes that such that async messages are XXXX_msg, and stops adding entries for them in the dispatch table, avoiding the need for a dummy no-op implementation. * qemud/remote.c: Remove dummy remoteDispatchDomainEvent, no longer required. Update to replace remote_domain_event_ret with xdr_remote_domain_event_msg * qemud/remote_protocol.x: Rename remote_domain_event_ret to remote_domain_event_msg * qemud/remote_generate_stubs.pl: Adding handling for new XXX_msg structs. * src/remote_internal.c: Rename remote_domain_event_ret to remote_domain_event_msg * qemud/remote_dispatch_prototypes.h, qemud/remote_dispatch_ret.h, qemud/remote_dispatch_table.h, qemud/remote_protocol.h, qemud/remote_protocol.c: auto-regenerate Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- qemud/remote.c | 22 ++-------------------- qemud/remote_dispatch_prototypes.h | 7 ------- qemud/remote_dispatch_ret.h | 1 - qemud/remote_dispatch_table.h | 6 +++--- qemud/remote_generate_stubs.pl | 23 +++++++++++++++++++++-- qemud/remote_protocol.c | 2 +- qemud/remote_protocol.h | 8 ++++---- qemud/remote_protocol.x | 2 +- src/remote_internal.c | 12 ++++++------ 9 files changed, 38 insertions(+), 45 deletions(-) diff --git a/qemud/remote.c b/qemud/remote.c index fb06dc3..bb49c29 100644 --- a/qemud/remote.c +++ b/qemud/remote.c @@ -4365,24 +4365,6 @@ remoteDispatchNodeDeviceDestroy(struct qemud_server *server ATTRIBUTE_UNUSED, } -/************************** - * Async Events - **************************/ -static int -remoteDispatchDomainEvent (struct qemud_server *server ATTRIBUTE_UNUSED, - struct qemud_client *client ATTRIBUTE_UNUSED, - virConnectPtr conn ATTRIBUTE_UNUSED, - remote_error *rerr ATTRIBUTE_UNUSED, - void *args ATTRIBUTE_UNUSED, - remote_domain_event_ret *ret ATTRIBUTE_UNUSED) -{ - /* This call gets dispatched from a client call. - * This does not make sense, as this should not be intiated - * from the client side in generated code. - */ - remoteDispatchFormatError(rerr, "%s", _("unexpected async event method call")); - return -1; -} /*************************** * Register / deregister events @@ -4434,7 +4416,7 @@ remoteDispatchDomainEventSend (struct qemud_client *client, struct qemud_client_message *msg = NULL; XDR xdr; unsigned int len; - remote_domain_event_ret data; + remote_domain_event_msg data; if (VIR_ALLOC(msg) < 0) return; @@ -4460,7 +4442,7 @@ remoteDispatchDomainEventSend (struct qemud_client *client, data.event = event; data.detail = detail; - if (!xdr_remote_domain_event_ret(&xdr, &data)) + if (!xdr_remote_domain_event_msg(&xdr, &data)) goto xdr_error; diff --git a/qemud/remote_dispatch_prototypes.h b/qemud/remote_dispatch_prototypes.h index a20ac4e..aa9c3fc 100644 --- a/qemud/remote_dispatch_prototypes.h +++ b/qemud/remote_dispatch_prototypes.h @@ -114,13 +114,6 @@ static int remoteDispatchDomainDumpXml( remote_error *err, remote_domain_dump_xml_args *args, remote_domain_dump_xml_ret *ret); -static int remoteDispatchDomainEvent( - struct qemud_server *server, - struct qemud_client *client, - virConnectPtr conn, - remote_error *err, - void *args, - remote_domain_event_ret *ret); static int remoteDispatchDomainEventsDeregister( struct qemud_server *server, struct qemud_client *client, diff --git a/qemud/remote_dispatch_ret.h b/qemud/remote_dispatch_ret.h index d83ffd5..fb4a86e 100644 --- a/qemud/remote_dispatch_ret.h +++ b/qemud/remote_dispatch_ret.h @@ -75,7 +75,6 @@ remote_domain_memory_peek_ret val_remote_domain_memory_peek_ret; remote_domain_events_register_ret val_remote_domain_events_register_ret; remote_domain_events_deregister_ret val_remote_domain_events_deregister_ret; - remote_domain_event_ret val_remote_domain_event_ret; remote_domain_migrate_prepare2_ret val_remote_domain_migrate_prepare2_ret; remote_domain_migrate_finish2_ret val_remote_domain_migrate_finish2_ret; remote_get_uri_ret val_remote_get_uri_ret; diff --git a/qemud/remote_dispatch_table.h b/qemud/remote_dispatch_table.h index ae0af28..f3a0b10 100644 --- a/qemud/remote_dispatch_table.h +++ b/qemud/remote_dispatch_table.h @@ -537,10 +537,10 @@ .args_filter = (xdrproc_t) xdr_void, .ret_filter = (xdrproc_t) xdr_remote_domain_events_deregister_ret, }, -{ /* DomainEvent => 107 */ - .fn = (dispatch_fn) remoteDispatchDomainEvent, +{ /* Async event DomainEvent => 107 */ + .fn = NULL, .args_filter = (xdrproc_t) xdr_void, - .ret_filter = (xdrproc_t) xdr_remote_domain_event_ret, + .ret_filter = (xdrproc_t) xdr_void, }, { /* DomainMigratePrepare2 => 108 */ .fn = (dispatch_fn) remoteDispatchDomainMigratePrepare2, diff --git a/qemud/remote_generate_stubs.pl b/qemud/remote_generate_stubs.pl index 44e1552..9bca0cc 100755 --- a/qemud/remote_generate_stubs.pl +++ b/qemud/remote_generate_stubs.pl @@ -65,6 +65,16 @@ while (<>) { ret => "remote_${name}_ret" } } + } elsif (/^struct remote_(.*)_msg/) { + $name = $1; + $ProcName = name_to_ProcName ($name); + + $calls{$name} = { + name => $name, + ProcName => $ProcName, + UC_NAME => uc $name, + msg => "remote_${name}_msg" + } } elsif (/^\s*REMOTE_PROC_(.*?)\s+=\s+(\d+),?$/) { $name = lc $1; $id = $2; @@ -98,6 +108,9 @@ if ($opt_d) { elsif ($opt_p) { my @keys = sort (keys %calls); foreach (@keys) { + # Skip things which are REMOTE_MESSAGE + next if $calls{$_}->{msg}; + print "static int remoteDispatch$calls{$_}->{ProcName}(\n"; print " struct qemud_server *server,\n"; print " struct qemud_client *client,\n"; @@ -113,6 +126,7 @@ elsif ($opt_p) { elsif ($opt_a) { for ($id = 0 ; $id <= $#calls ; $id++) { if (defined $calls[$id] && + !$calls[$id]->{msg} && $calls[$id]->{args} ne "void") { print " $calls[$id]->{args} val_$calls[$id]->{args};\n"; } @@ -124,6 +138,7 @@ elsif ($opt_a) { elsif ($opt_r) { for ($id = 0 ; $id <= $#calls ; $id++) { if (defined $calls[$id] && + !$calls[$id]->{msg} && $calls[$id]->{ret} ne "void") { print " $calls[$id]->{ret} val_$calls[$id]->{ret};\n"; } @@ -134,7 +149,7 @@ elsif ($opt_r) { # ("remote_dispatch_table.h"). elsif ($opt_t) { for ($id = 0 ; $id <= $#calls ; $id++) { - if (defined $calls[$id]) { + if (defined $calls[$id] && !$calls[$id]->{msg}) { print "{ /* $calls[$id]->{ProcName} => $id */\n"; print " .fn = (dispatch_fn) remoteDispatch$calls[$id]->{ProcName},\n"; if ($calls[$id]->{args} ne "void") { @@ -149,7 +164,11 @@ elsif ($opt_t) { } print "},\n"; } else { - print "{ /* (unused) => $id */\n"; + if ($calls[$id]->{msg}) { + print "{ /* Async event $calls[$id]->{ProcName} => $id */\n"; + } else { + print "{ /* (unused) => $id */\n"; + } print " .fn = NULL,\n"; print " .args_filter = (xdrproc_t) xdr_void,\n"; print " .ret_filter = (xdrproc_t) xdr_void,\n"; diff --git a/qemud/remote_protocol.c b/qemud/remote_protocol.c index 5d09167..f6dac7a 100644 --- a/qemud/remote_protocol.c +++ b/qemud/remote_protocol.c @@ -2448,7 +2448,7 @@ xdr_remote_domain_events_deregister_ret (XDR *xdrs, remote_domain_events_deregis } bool_t -xdr_remote_domain_event_ret (XDR *xdrs, remote_domain_event_ret *objp) +xdr_remote_domain_event_msg (XDR *xdrs, remote_domain_event_msg *objp) { if (!xdr_remote_nonnull_domain (xdrs, &objp->dom)) diff --git a/qemud/remote_protocol.h b/qemud/remote_protocol.h index 87a5925..21eae07 100644 --- a/qemud/remote_protocol.h +++ b/qemud/remote_protocol.h @@ -1378,12 +1378,12 @@ struct remote_domain_events_deregister_ret { }; typedef struct remote_domain_events_deregister_ret remote_domain_events_deregister_ret; -struct remote_domain_event_ret { +struct remote_domain_event_msg { remote_nonnull_domain dom; int event; int detail; }; -typedef struct remote_domain_event_ret remote_domain_event_ret; +typedef struct remote_domain_event_msg remote_domain_event_msg; struct remote_domain_xml_from_native_args { remote_nonnull_string nativeFormat; @@ -1802,7 +1802,7 @@ extern bool_t xdr_remote_node_device_create_xml_ret (XDR *, remote_node_device_ extern bool_t xdr_remote_node_device_destroy_args (XDR *, remote_node_device_destroy_args*); extern bool_t xdr_remote_domain_events_register_ret (XDR *, remote_domain_events_register_ret*); extern bool_t xdr_remote_domain_events_deregister_ret (XDR *, remote_domain_events_deregister_ret*); -extern bool_t xdr_remote_domain_event_ret (XDR *, remote_domain_event_ret*); +extern bool_t xdr_remote_domain_event_msg (XDR *, remote_domain_event_msg*); extern bool_t xdr_remote_domain_xml_from_native_args (XDR *, remote_domain_xml_from_native_args*); extern bool_t xdr_remote_domain_xml_from_native_ret (XDR *, remote_domain_xml_from_native_ret*); extern bool_t xdr_remote_domain_xml_to_native_args (XDR *, remote_domain_xml_to_native_args*); @@ -2037,7 +2037,7 @@ extern bool_t xdr_remote_node_device_create_xml_ret (); extern bool_t xdr_remote_node_device_destroy_args (); extern bool_t xdr_remote_domain_events_register_ret (); extern bool_t xdr_remote_domain_events_deregister_ret (); -extern bool_t xdr_remote_domain_event_ret (); +extern bool_t xdr_remote_domain_event_msg (); extern bool_t xdr_remote_domain_xml_from_native_args (); extern bool_t xdr_remote_domain_xml_from_native_ret (); extern bool_t xdr_remote_domain_xml_to_native_args (); diff --git a/qemud/remote_protocol.x b/qemud/remote_protocol.x index 56385f4..1fb826b 100644 --- a/qemud/remote_protocol.x +++ b/qemud/remote_protocol.x @@ -1222,7 +1222,7 @@ struct remote_domain_events_deregister_ret { int cb_registered; }; -struct remote_domain_event_ret { +struct remote_domain_event_msg { remote_nonnull_domain dom; int event; int detail; diff --git a/src/remote_internal.c b/src/remote_internal.c index 6df0282..e7beb49 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -7093,23 +7093,23 @@ cleanup: static virDomainEventPtr remoteDomainReadEvent(virConnectPtr conn, XDR *xdr) { - remote_domain_event_ret ret; + remote_domain_event_msg msg; virDomainPtr dom; virDomainEventPtr event = NULL; - memset (&ret, 0, sizeof ret); + memset (&msg, 0, sizeof msg); /* unmarshall parameters, and process it*/ - if (! xdr_remote_domain_event_ret(xdr, &ret) ) { + if (! xdr_remote_domain_event_msg(xdr, &msg) ) { error (conn, VIR_ERR_RPC, - _("remoteDomainProcessEvent: unmarshalling ret")); + _("remoteDomainProcessEvent: unmarshalling msg")); return NULL; } - dom = get_nonnull_domain(conn,ret.dom); + dom = get_nonnull_domain(conn,msg.dom); if (!dom) return NULL; - event = virDomainEventNewFromDom(dom, ret.event, ret.detail); + event = virDomainEventNewFromDom(dom, msg.event, msg.detail); virDomainFree(dom); return event; -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

This removes an assumption from qemudWorker() code that every incoming message will generate a reply. * qemud/dispatch.c: remoteDispatchClientRequest now has responsibility for queuing the reply message to the RPC call * qemud/qemud.c: Do not queue the RPC call reply in qemudWorker(), allowing remoteDispatchClientRequest() to take care of it Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- qemud/dispatch.c | 6 ++++++ qemud/qemud.c | 16 +++++----------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/qemud/dispatch.c b/qemud/dispatch.c index 29970e4..ce8dbc9 100644 --- a/qemud/dispatch.c +++ b/qemud/dispatch.c @@ -387,6 +387,12 @@ rpc_error: msg->bufferLength = len; msg->bufferOffset = 0; + /* Put reply on end of tx queue to send out */ + qemudClientMessageQueuePush(&client->tx, msg); + + if (qemudRegisterClientEvent(server, client, 1) < 0) + qemudDispatchClientFailure(client); + return 0; fatal_error: diff --git a/qemud/qemud.c b/qemud/qemud.c index c577d88..4952d0b 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -1431,7 +1431,7 @@ static void *qemudWorker(void *data) while (1) { struct qemud_client *client = NULL; - struct qemud_client_message *reply; + struct qemud_client_message *msg; virMutexLock(&server->lock); while (((client = qemudPendingJob(server)) == NULL) && @@ -1454,25 +1454,19 @@ static void *qemudWorker(void *data) client->refs++; /* Remove our message from dispatch queue while we use it */ - reply = qemudClientMessageQueueServe(&client->dx); + msg = qemudClientMessageQueueServe(&client->dx); /* This function drops the lock during dispatch, * and re-acquires it before returning */ - if (remoteDecodeClientMessageHeader(reply) < 0 || - remoteDispatchClientRequest (server, client, reply) < 0) { - VIR_FREE(reply); + if (remoteDecodeClientMessageHeader(msg) < 0 || + remoteDispatchClientRequest (server, client, msg) < 0) { + VIR_FREE(msg); qemudDispatchClientFailure(client); client->refs--; virMutexUnlock(&client->lock); continue; } - /* Put reply on end of tx queue to send out */ - qemudClientMessageQueuePush(&client->tx, reply); - - if (qemudRegisterClientEvent(server, client, 1) < 0) - qemudDispatchClientFailure(client); - client->refs--; virMutexUnlock(&client->lock); -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

The current qemudRegisterClientEvent() code is used both for registering the initial socket watch, and updating the already registered watch. This causes unneccessary complexity in alot of code which only cares about updating existing watches. The updating of a watch cannot ever fail, nor is a reference to the 'qemud_server' object required. This introduces a new qemudUpdateClientEvent() method for that case, allowing the elimination of unneccessary error checking and removal of the server back-reference in struct qemud_client. * qemud/qemud.h: Remove 'server' field from struct qemud_client. Add qemudUpdateClientEvent() method. Remove 'update' param from qemudRegisterClientEvent method * qemud/dispatch.c, qemud/qemud.c, qemud/remote.c: Update alot of code to use qemudUpdateClientEvent() instead of qemudRegisterClientEvent(). Move more logic from remoteRelayDomainEvent into remoteDispatchDomainEventSend. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- qemud/dispatch.c | 4 +-- qemud/qemud.c | 93 ++++++++++++++++++++++++++++++++---------------------- qemud/qemud.h | 6 +-- qemud/remote.c | 30 +++++++---------- 4 files changed, 70 insertions(+), 63 deletions(-) diff --git a/qemud/dispatch.c b/qemud/dispatch.c index ce8dbc9..a4e6c3e 100644 --- a/qemud/dispatch.c +++ b/qemud/dispatch.c @@ -389,9 +389,7 @@ rpc_error: /* Put reply on end of tx queue to send out */ qemudClientMessageQueuePush(&client->tx, msg); - - if (qemudRegisterClientEvent(server, client, 1) < 0) - qemudDispatchClientFailure(client); + qemudUpdateClientEvent(client); return 0; diff --git a/qemud/qemud.c b/qemud/qemud.c index 4952d0b..42bc00e 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -1276,7 +1276,6 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket client->auth = sock->auth; memcpy (&client->addr, &addr, sizeof addr); client->addrlen = addrlen; - client->server = server; /* Prepare one for packet receive */ if (VIR_ALLOC(client->rx) < 0) @@ -1306,7 +1305,7 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket if (client->type != QEMUD_SOCK_TYPE_TLS) { /* Plain socket, so prepare to read first message */ - if (qemudRegisterClientEvent (server, client, 0) < 0) + if (qemudRegisterClientEvent (server, client) < 0) goto cleanup; } else { int ret; @@ -1328,13 +1327,13 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket goto cleanup; /* Handshake & cert check OK, so prepare to read first message */ - if (qemudRegisterClientEvent(server, client, 0) < 0) + if (qemudRegisterClientEvent(server, client) < 0) goto cleanup; } else if (ret == GNUTLS_E_INTERRUPTED || ret == GNUTLS_E_AGAIN) { /* Most likely, need to do more handshake data */ client->handshake = 1; - if (qemudRegisterClientEvent (server, client, 0) < 0) + if (qemudRegisterClientEvent (server, client) < 0) goto cleanup; } else { VIR_ERROR(_("TLS handshake failed: %s"), @@ -1699,10 +1698,7 @@ readmore: /* Prepare to read rest of message */ client->rx->bufferLength += len; - if (qemudRegisterClientEvent(server, client, 1) < 0) { - qemudDispatchClientFailure(client); - return; - } + qemudUpdateClientEvent(client); /* Try and read payload immediately instead of going back into poll() because chances are the data is already @@ -1722,11 +1718,10 @@ readmore: if (client->rx) client->rx->bufferLength = REMOTE_MESSAGE_HEADER_XDR_LEN; - if (qemudRegisterClientEvent(server, client, 1) < 0) - qemudDispatchClientFailure(client); - else - /* Tell one of the workers to get on with it... */ - virCondSignal(&server->job); + qemudUpdateClientEvent(client); + + /* Tell one of the workers to get on with it... */ + virCondSignal(&server->job); } } } @@ -1872,8 +1867,7 @@ static ssize_t qemudClientWrite(struct qemud_client *client) { * we would block on I/O */ static void -qemudDispatchClientWrite(struct qemud_server *server, - struct qemud_client *client) { +qemudDispatchClientWrite(struct qemud_client *client) { while (client->tx) { ssize_t ret; @@ -1907,16 +1901,16 @@ qemudDispatchClientWrite(struct qemud_server *server, VIR_FREE(reply); } - if (client->closing || - qemudRegisterClientEvent (server, client, 1) < 0) - qemudDispatchClientFailure(client); + if (client->closing) + qemudDispatchClientFailure(client); + else + qemudUpdateClientEvent(client); } } } static void -qemudDispatchClientHandshake(struct qemud_server *server, - struct qemud_client *client) { +qemudDispatchClientHandshake(struct qemud_client *client) { int ret; /* Continue the handshake. */ ret = gnutls_handshake (client->tlssession); @@ -1926,15 +1920,14 @@ qemudDispatchClientHandshake(struct qemud_server *server, /* Finished. Next step is to check the certificate. */ if (remoteCheckAccess (client) == -1) qemudDispatchClientFailure(client); - else if (qemudRegisterClientEvent (server, client, 1)) - qemudDispatchClientFailure(client); + else + qemudUpdateClientEvent(client); } else if (ret == GNUTLS_E_AGAIN || ret == GNUTLS_E_INTERRUPTED) { /* Carry on waiting for more handshake. Update the events just in case handshake data flow direction has changed */ - if (qemudRegisterClientEvent (server, client, 1)) - qemudDispatchClientFailure(client); + qemudUpdateClientEvent (client); } else { /* Fatal error in handshake */ VIR_ERROR(_("TLS handshake failed: %s"), @@ -1974,10 +1967,10 @@ qemudDispatchClientEvent(int watch, int fd, int events, void *opaque) { if (events & (VIR_EVENT_HANDLE_WRITABLE | VIR_EVENT_HANDLE_READABLE)) { if (client->handshake) { - qemudDispatchClientHandshake(server, client); + qemudDispatchClientHandshake(client); } else { if (events & VIR_EVENT_HANDLE_WRITABLE) - qemudDispatchClientWrite(server, client); + qemudDispatchClientWrite(client); if (events & VIR_EVENT_HANDLE_READABLE) qemudDispatchClientRead(server, client); } @@ -1992,9 +1985,12 @@ qemudDispatchClientEvent(int watch, int fd, int events, void *opaque) { virMutexUnlock(&client->lock); } -int qemudRegisterClientEvent(struct qemud_server *server, - struct qemud_client *client, - int update) { + +/* + * @client: a locked client object + */ +static int +qemudCalculateHandleMode(struct qemud_client *client) { int mode = 0; if (client->handshake) { @@ -2014,19 +2010,40 @@ int qemudRegisterClientEvent(struct qemud_server *server, mode |= VIR_EVENT_HANDLE_WRITABLE; } - if (update) { - virEventUpdateHandleImpl(client->watch, mode); - } else { - if ((client->watch = virEventAddHandleImpl(client->fd, - mode, - qemudDispatchClientEvent, - server, NULL)) < 0) - return -1; - } + return mode; +} + +/* + * @server: a locked or unlocked server object + * @client: a locked client object + */ +int qemudRegisterClientEvent(struct qemud_server *server, + struct qemud_client *client) { + int mode; + + mode = qemudCalculateHandleMode(client); + + if ((client->watch = virEventAddHandleImpl(client->fd, + mode, + qemudDispatchClientEvent, + server, NULL)) < 0) + return -1; return 0; } +/* + * @client: a locked client object + */ +void qemudUpdateClientEvent(struct qemud_client *client) { + int mode; + + mode = qemudCalculateHandleMode(client); + + virEventUpdateHandleImpl(client->watch, mode); +} + + static void qemudDispatchServerEvent(int watch, int fd, int events, void *opaque) { struct qemud_server *server = (struct qemud_server *)opaque; diff --git a/qemud/qemud.h b/qemud/qemud.h index 6597429..86b893d 100644 --- a/qemud/qemud.h +++ b/qemud/qemud.h @@ -142,8 +142,6 @@ struct qemud_client { virConnectPtr conn; int refs; - /* back-pointer to our server */ - struct qemud_server *server; }; #define QEMUD_CLIENT_MAGIC 0x7788aaee @@ -204,8 +202,8 @@ void qemudLog(int priority, const char *fmt, ...) int qemudRegisterClientEvent(struct qemud_server *server, - struct qemud_client *client, - int update); + struct qemud_client *client); +void qemudUpdateClientEvent(struct qemud_client *client); void qemudDispatchClientFailure(struct qemud_client *client); diff --git a/qemud/remote.c b/qemud/remote.c index bb49c29..4d6ddef 100644 --- a/qemud/remote.c +++ b/qemud/remote.c @@ -91,11 +91,7 @@ const dispatch_data const *remoteGetDispatchData(int proc) /* Prototypes */ static void remoteDispatchDomainEventSend (struct qemud_client *client, - virDomainPtr dom, - int event, - int detail); - - + remote_domain_event_msg *data); int remoteRelayDomainEvent (virConnectPtr conn ATTRIBUTE_UNUSED, virDomainPtr dom, @@ -107,12 +103,17 @@ int remoteRelayDomainEvent (virConnectPtr conn ATTRIBUTE_UNUSED, REMOTE_DEBUG("Relaying domain event %d %d", event, detail); if (client) { + remote_domain_event_msg data; + virMutexLock(&client->lock); - remoteDispatchDomainEventSend (client, dom, event, detail); + /* build return data */ + memset(&data, 0, sizeof data); + make_nonnull_domain (&data.dom, dom); + data.event = event; + data.detail = detail; - if (qemudRegisterClientEvent(client->server, client, 1) < 0) - qemudDispatchClientFailure(client); + remoteDispatchDomainEventSend (client, &data); virMutexUnlock(&client->lock); } @@ -4409,14 +4410,11 @@ remoteDispatchDomainEventsDeregister (struct qemud_server *server ATTRIBUTE_UNUS static void remoteDispatchDomainEventSend (struct qemud_client *client, - virDomainPtr dom, - int event, - int detail) + remote_domain_event_msg *data) { struct qemud_client_message *msg = NULL; XDR xdr; unsigned int len; - remote_domain_event_msg data; if (VIR_ALLOC(msg) < 0) return; @@ -4437,12 +4435,7 @@ remoteDispatchDomainEventSend (struct qemud_client *client, msg->bufferLength - msg->bufferOffset, XDR_ENCODE); - /* build return data */ - make_nonnull_domain (&data.dom, dom); - data.event = event; - data.detail = detail; - - if (!xdr_remote_domain_event_msg(&xdr, &data)) + if (!xdr_remote_domain_event_msg(&xdr, data)) goto xdr_error; @@ -4460,6 +4453,7 @@ remoteDispatchDomainEventSend (struct qemud_client *client, msg->bufferLength = len; msg->bufferOffset = 0; qemudClientMessageQueuePush(&client->tx, msg); + qemudUpdateClientEvent(client); xdr_destroy (&xdr); return; -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

The remoteDispatchClientRequest() method is currently hardwired to assume there is only one type of incoming message, a method call. To allow for alternate types of incoming messags, the code that is specific to method calls is being split into a separate method remoteDispatchClientCall * qemud/dispatch.c: Move method call specific code out into remoteDispatchClientCall. Add a helper remoteSerializeError for returning error messages to client Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- qemud/dispatch.c | 208 ++++++++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 170 insertions(+), 38 deletions(-) diff --git a/qemud/dispatch.c b/qemud/dispatch.c index a4e6c3e..1ccca10 100644 --- a/qemud/dispatch.c +++ b/qemud/dispatch.c @@ -23,10 +23,11 @@ #include <config.h> - #include "dispatch.h" #include "remote.h" +#include "memory.h" + /* Convert a libvirt virError object into wire format */ static void remoteDispatchCopyError (remote_error *rerr, @@ -122,6 +123,101 @@ void remoteDispatchConnError (remote_error *rerr, remoteDispatchGenericError(rerr); } +static int +remoteSerializeError(struct qemud_client *client, + remote_error *rerr, + int program, + int version, + int procedure, + int direction, + int serial) +{ + XDR xdr; + unsigned int len; + struct qemud_client_message *msg = NULL; + + if (VIR_ALLOC(msg) < 0) + goto fatal_error; + + /* Return header. */ + msg->hdr.prog = program; + msg->hdr.vers = version; + msg->hdr.proc = procedure; + msg->hdr.direction = direction; + msg->hdr.serial = serial; + msg->hdr.status = REMOTE_ERROR; + + msg->bufferLength = sizeof(msg->buffer); + + /* Serialise the return header. */ + xdrmem_create (&xdr, + msg->buffer, + msg->bufferLength, + XDR_ENCODE); + + len = 0; /* We'll come back and write this later. */ + if (!xdr_u_int (&xdr, &len)) + goto xdr_error; + + if (!xdr_remote_message_header (&xdr, &msg->hdr)) + goto xdr_error; + + /* Error was not set, so synthesize a generic error message. */ + if (rerr->code == 0) + remoteDispatchGenericError(rerr); + + if (!xdr_remote_error (&xdr, rerr)) + goto xdr_error; + + /* Write the length word. */ + len = xdr_getpos (&xdr); + if (xdr_setpos (&xdr, 0) == 0) + goto xdr_error; + + if (!xdr_u_int (&xdr, &len)) + goto xdr_error; + + xdr_destroy (&xdr); + + msg->bufferLength = len; + msg->bufferOffset = 0; + + /* Put reply on end of tx queue to send out */ + qemudClientMessageQueuePush(&client->tx, msg); + qemudUpdateClientEvent(client); + xdr_free((xdrproc_t)xdr_remote_error, (char *)rerr); + + return 0; + +xdr_error: + xdr_destroy(&xdr); +fatal_error: + xdr_free((xdrproc_t)xdr_remote_error, (char *)rerr); + return -1; +} + + +/* + * @client: the client to send the error to + * @rerr: the error object to send + * @req: the message this error is in reply to + * + * Send an error message to the client + * + * Returns 0 if the error was sent, -1 upon fatal error + */ +static int +remoteSerializeReplyError(struct qemud_client *client, + remote_error *rerr, + remote_message_header *req) { + return remoteSerializeError(client, + rerr, + req->prog, + req->vers, + req->proc, + REMOTE_REPLY, + req->serial); +} /* * @msg: the complete incoming message, whose header to decode @@ -216,6 +312,12 @@ cleanup: } +int +remoteDispatchClientCall (struct qemud_server *server, + struct qemud_client *client, + struct qemud_client_message *msg); + + /* * @server: the unlocked server object * @client: the locked client object @@ -234,44 +336,73 @@ remoteDispatchClientRequest (struct qemud_server *server, struct qemud_client *client, struct qemud_client_message *msg) { - XDR xdr; remote_error rerr; - dispatch_args args; - dispatch_ret ret; - const dispatch_data *data = NULL; - int rv = -1; - unsigned int len; - virConnectPtr conn = NULL; - memset(&args, 0, sizeof args); - memset(&ret, 0, sizeof ret); memset(&rerr, 0, sizeof rerr); - /* Check version, etc. */ if (msg->hdr.prog != REMOTE_PROGRAM) { remoteDispatchFormatError (&rerr, _("program mismatch (actual %x, expected %x)"), msg->hdr.prog, REMOTE_PROGRAM); - goto rpc_error; + goto error; } if (msg->hdr.vers != REMOTE_PROTOCOL_VERSION) { remoteDispatchFormatError (&rerr, _("version mismatch (actual %x, expected %x)"), msg->hdr.vers, REMOTE_PROTOCOL_VERSION); - goto rpc_error; - } - if (msg->hdr.direction != REMOTE_CALL) { - remoteDispatchFormatError (&rerr, _("direction (%d) != REMOTE_CALL"), - (int) msg->hdr.direction); - goto rpc_error; + goto error; } if (msg->hdr.status != REMOTE_OK) { remoteDispatchFormatError (&rerr, _("status (%d) != REMOTE_OK"), (int) msg->hdr.status); - goto rpc_error; + goto error; + } + + switch (msg->hdr.direction) { + case REMOTE_CALL: + return remoteDispatchClientCall(server, client, msg); + + default: + remoteDispatchFormatError (&rerr, _("direction (%d) != REMOTE_CALL"), + (int) msg->hdr.direction); } +error: + return remoteSerializeReplyError(client, &rerr, &msg->hdr); +} + + +/* + * @server: the unlocked server object + * @client: the locked client object + * @msg: the complete incoming method call, with header already decoded + * + * This method is used to dispatch an message representing an + * incoming method call from a client. It decodes the payload + * to obtain method call arguments, invokves the method and + * then sends a reply packet with the return values + * + * Returns 0 if the reply was sent, or -1 upon fatal error + */ +int +remoteDispatchClientCall (struct qemud_server *server, + struct qemud_client *client, + struct qemud_client_message *msg) +{ + XDR xdr; + remote_error rerr; + dispatch_args args; + dispatch_ret ret; + const dispatch_data *data = NULL; + int rv = -1; + unsigned int len; + virConnectPtr conn = NULL; + + memset(&args, 0, sizeof args); + memset(&ret, 0, sizeof ret); + memset(&rerr, 0, sizeof rerr); + /* If client is marked as needing auth, don't allow any RPC ops, * except for authentication ones */ @@ -332,7 +463,8 @@ remoteDispatchClientRequest (struct qemud_server *server, xdr_free (data->args_filter, (char*)&args); -rpc_error: + if (rv < 0) + goto rpc_error; /* Return header. We're re-using same message object, so * only need to tweak direction/status fields */ @@ -341,10 +473,10 @@ rpc_error: /*msg->hdr.proc = msg->hdr.proc;*/ msg->hdr.direction = REMOTE_REPLY; /*msg->hdr.serial = msg->hdr.serial;*/ - msg->hdr.status = rv < 0 ? REMOTE_ERROR : REMOTE_OK; + msg->hdr.status = REMOTE_OK; if (remoteEncodeClientMessageHeader(msg) < 0) { - if (rv == 0) xdr_free (data->ret_filter, (char*)&ret); + xdr_free (data->ret_filter, (char*)&ret); goto fatal_error; } @@ -356,32 +488,24 @@ rpc_error: XDR_ENCODE); if (xdr_setpos(&xdr, msg->bufferOffset) == 0) - goto fatal_error; + goto xdr_error; /* If OK, serialise return structure, if error serialise error. */ - if (rv >= 0) { - if (!((data->ret_filter) (&xdr, &ret))) - goto fatal_error; - xdr_free (data->ret_filter, (char*)&ret); - } else /* error */ { - /* Error was NULL so synthesize an error. */ - if (rerr.code == 0) - remoteDispatchGenericError(&rerr); - if (!xdr_remote_error (&xdr, &rerr)) - goto fatal_error; - xdr_free((xdrproc_t)xdr_remote_error, (char *)&rerr); - } + /* Serialise reply data */ + if (!((data->ret_filter) (&xdr, &ret))) + goto xdr_error; /* Update the length word. */ msg->bufferOffset += xdr_getpos (&xdr); len = msg->bufferOffset; if (xdr_setpos (&xdr, 0) == 0) - goto fatal_error; + goto xdr_error; if (!xdr_u_int (&xdr, &len)) - goto fatal_error; + goto xdr_error; xdr_destroy (&xdr); + xdr_free (data->ret_filter, (char*)&ret); /* Reset ready for I/O */ msg->bufferLength = len; @@ -393,9 +517,17 @@ rpc_error: return 0; -fatal_error: +rpc_error: + /* Semi-bad stuff happened, we can still try to send back + * an RPC error message to client */ + return remoteSerializeReplyError(client, &rerr, &msg->hdr); + + +xdr_error: /* Seriously bad stuff happened, so we'll kill off this client and not send back any RPC error */ + xdr_free (data->ret_filter, (char*)&ret); xdr_destroy (&xdr); +fatal_error: return -1; } -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

All incoming messages currently get routed to the generic method remoteDispatchClientRequest() for processing. To allow incoming data stream messages to bypass this and be routed to a specific location, a concept of dispatch filters is introduced. * qemud/qemud.h: Add a qemud_client_filter struct and a callback qemud_client_filter_func. Maintain a list of filters on every struct qemud_client * qemud/qemud.c: Move remoteDecodeClientMessageHeader() out of qemudWorker() into qemudDispatchClientRead(). Check registered message filters in qemudDispatchClientRead() to decide where to send incoming messages for dispatch. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- qemud/qemud.c | 28 ++++++++++++++++++++++++---- qemud/qemud.h | 16 ++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/qemud/qemud.c b/qemud/qemud.c index 42bc00e..e393db4 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -1457,8 +1457,7 @@ static void *qemudWorker(void *data) /* This function drops the lock during dispatch, * and re-acquires it before returning */ - if (remoteDecodeClientMessageHeader(msg) < 0 || - remoteDispatchClientRequest (server, client, msg) < 0) { + if (remoteDispatchClientRequest (server, client, msg) < 0) { VIR_FREE(msg); qemudDispatchClientFailure(client); client->refs--; @@ -1705,9 +1704,30 @@ readmore: waiting for us */ goto readmore; } else { + /* Grab the completed message */ + struct qemud_client_message *msg = qemudClientMessageQueueServe(&client->rx); + struct qemud_client_filter *filter; + + /* Decode the header so we can use it for routing decisions */ + if (remoteDecodeClientMessageHeader(msg) < 0) { + VIR_FREE(msg); + qemudDispatchClientFailure(client); + } + + /* Check if any filters match this message */ + filter = client->filters; + while (filter) { + if ((filter->query)(msg, filter->opaque)) { + qemudClientMessageQueuePush(&filter->dx, msg); + msg = NULL; + break; + } + filter = filter->next; + } + /* Move completed message to the end of the dispatch queue */ - qemudClientMessageQueuePush(&client->dx, client->rx); - client->rx = NULL; + if (msg) + qemudClientMessageQueuePush(&client->dx, msg); client->nrequests++; /* Possibly need to create another receive buffer */ diff --git a/qemud/qemud.h b/qemud/qemud.h index 86b893d..abacbbb 100644 --- a/qemud/qemud.h +++ b/qemud/qemud.h @@ -90,6 +90,19 @@ struct qemud_client_message { struct qemud_client_message *next; }; +/* Allow for filtering of incoming messages to a custom + * dispatch processing queue, instead of client->dx. + */ +typedef int (*qemud_client_filter_func)(struct qemud_client_message *msg, void *opaque); +struct qemud_client_filter { + qemud_client_filter_func query; + void *opaque; + + struct qemud_client_message *dx; + + struct qemud_client_filter *next; +}; + /* Stores the per-client connection state */ struct qemud_client { virMutex lock; @@ -134,6 +147,9 @@ struct qemud_client { /* Zero or many messages waiting for transmit * back to client, including async events */ struct qemud_client_message *tx; + /* Filters to capture messages that would otherwise + * end up on the 'dx' queue */ + struct qemud_client_filter *filters; /* This is only valid if a remote open call has been made on this * connection, otherwise it will be NULL. Also if remote close is -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|