On Tue, Apr 13, 2010 at 02:36:48PM -0400, Chris Lalancette wrote:
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
daemon/Makefile.am | 25 +++++-
daemon/dispatch.c | 171 +++++++++++++++++++------------------
daemon/libvirtd.h | 1 +
daemon/qemu_dispatch_args.h | 5 +
daemon/qemu_dispatch_prototypes.h | 12 +++
daemon/qemu_dispatch_ret.h | 5 +
daemon/qemu_dispatch_table.h | 14 +++
daemon/qemu_generate_stubs.pl | 170 ++++++++++++++++++++++++++++++++++++
daemon/remote.c | 43 +++++++++
daemon/remote.h | 8 ++
src/Makefile.am | 37 ++++++++-
src/remote/qemu_protocol.c | 60 +++++++++++++
src/remote/qemu_protocol.h | 69 +++++++++++++++
src/remote/qemu_protocol.x | 55 ++++++++++++
src/remote/remote_driver.c | 105 +++++++++++++++++------
15 files changed, 665 insertions(+), 115 deletions(-)
create mode 100644 daemon/qemu_dispatch_args.h
create mode 100644 daemon/qemu_dispatch_prototypes.h
create mode 100644 daemon/qemu_dispatch_ret.h
create mode 100644 daemon/qemu_dispatch_table.h
create mode 100755 daemon/qemu_generate_stubs.pl
create mode 100644 src/remote/qemu_protocol.c
create mode 100644 src/remote/qemu_protocol.h
create mode 100644 src/remote/qemu_protocol.x
diff --git a/daemon/dispatch.c b/daemon/dispatch.c
index f024900..d36d1a1 100644
--- a/daemon/dispatch.c
+++ b/daemon/dispatch.c
@@ -336,85 +336,6 @@ 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
- * @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
- */
-int
-remoteDispatchClientRequest (struct qemud_server *server,
- struct qemud_client *client,
- struct qemud_client_message *msg)
-{
- int ret;
- remote_error rerr;
-
- DEBUG("prog=%d ver=%d type=%d satus=%d serial=%d proc=%d",
- msg->hdr.prog, msg->hdr.vers, msg->hdr.type,
- msg->hdr.status, msg->hdr.serial, msg->hdr.proc);
-
- 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 error;
- }
- if (msg->hdr.vers != REMOTE_PROTOCOL_VERSION) {
- remoteDispatchFormatError (&rerr,
- _("version mismatch (actual %x, expected
%x)"),
- msg->hdr.vers, REMOTE_PROTOCOL_VERSION);
- goto error;
- }
-
- switch (msg->hdr.type) {
- case REMOTE_CALL:
- return remoteDispatchClientCall(server, client, msg);
-
- case REMOTE_STREAM:
- /* Since stream data is non-acked, async, we may continue to received
- * stream packets after we closed down a stream. Just drop & ignore
- * these.
- */
- VIR_INFO("Ignoring unexpected stream data serial=%d proc=%d
status=%d",
- msg->hdr.serial, msg->hdr.proc, msg->hdr.status);
- qemudClientMessageRelease(client, msg);
- break;
-
- default:
- remoteDispatchFormatError (&rerr, _("type (%d) != REMOTE_CALL"),
- (int) msg->hdr.type);
- goto error;
- }
-
- return 0;
-
-error:
- ret = remoteSerializeReplyError(client, &rerr, &msg->hdr);
-
- if (ret >= 0)
- VIR_FREE(msg);
-
- return ret;
-}
-
-
/*
* @server: the unlocked server object
* @client: the locked client object
@@ -427,10 +348,11 @@ error:
*
* Returns 0 if the reply was sent, or -1 upon fatal error
*/
-int
+static int
remoteDispatchClientCall (struct qemud_server *server,
struct qemud_client *client,
- struct qemud_client_message *msg)
+ struct qemud_client_message *msg,
+ int qemu_protocol)
{
XDR xdr;
remote_error rerr;
@@ -469,7 +391,10 @@ remoteDispatchClientCall (struct qemud_server *server,
}
}
- data = remoteGetDispatchData(msg->hdr.proc);
+ if (qemu_protocol)
+ data = qemuGetDispatchData(msg->hdr.proc);
+ else
+ data = remoteGetDispatchData(msg->hdr.proc);
if (!data) {
remoteDispatchFormatError (&rerr, _("unknown procedure: %d"),
@@ -584,6 +509,88 @@ fatal_error:
return -1;
}
+/*
+ * @server: the unlocked server object
+ * @client: the locked client object
+ * @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
+ */
+int remoteDispatchClientRequest(struct qemud_server *server,
+ struct qemud_client *client,
+ struct qemud_client_message *msg)
+{
+ int ret;
+ remote_error rerr;
+ int qemu_call;
+
+ DEBUG("prog=%d ver=%d type=%d satus=%d serial=%d proc=%d",
+ msg->hdr.prog, msg->hdr.vers, msg->hdr.type,
+ msg->hdr.status, msg->hdr.serial, msg->hdr.proc);
+
+ memset(&rerr, 0, sizeof rerr);
+
+ /* Check version, etc. */
+ if (msg->hdr.prog == REMOTE_PROGRAM)
+ qemu_call = 0;
+ else if (msg->hdr.prog == QEMU_PROGRAM)
+ qemu_call = 1;
+ else {
+ remoteDispatchFormatError (&rerr,
+ _("program mismatch (actual %x, expected %x or
%x)"),
+ msg->hdr.prog, REMOTE_PROGRAM, QEMU_PROGRAM);
+ goto error;
+ }
+
+ if (!qemu_call && msg->hdr.vers != REMOTE_PROTOCOL_VERSION) {
+ remoteDispatchFormatError (&rerr,
+ _("version mismatch (actual %x, expected
%x)"),
+ msg->hdr.vers, REMOTE_PROTOCOL_VERSION);
+ goto error;
+ }
+ else if (qemu_call && msg->hdr.vers != QEMU_PROTOCOL_VERSION) {
+ remoteDispatchFormatError (&rerr,
+ _("version mismatch (actual %x, expected
%x)"),
+ msg->hdr.vers, QEMU_PROTOCOL_VERSION);
+ goto error;
+ }
+
+ switch (msg->hdr.type) {
+ case REMOTE_CALL:
+ return remoteDispatchClientCall(server, client, msg, qemu_call);
+
+ case REMOTE_STREAM:
+ /* Since stream data is non-acked, async, we may continue to received
+ * stream packets after we closed down a stream. Just drop & ignore
+ * these.
+ */
+ VIR_INFO("Ignoring unexpected stream data serial=%d proc=%d
status=%d",
+ msg->hdr.serial, msg->hdr.proc, msg->hdr.status);
+ qemudClientMessageRelease(client, msg);
+ break;
+
+ default:
+ remoteDispatchFormatError (&rerr, _("type (%d) != REMOTE_CALL"),
+ (int) msg->hdr.type);
+ goto error;
+ }
+
+ return 0;
+
+error:
+ ret = remoteSerializeReplyError(client, &rerr, &msg->hdr);
+
+ if (ret >= 0)
+ VIR_FREE(msg);
+
+ return ret;
+}
Can we avoid moving this function down the file - there doesn't appear to
be any compile reason for moving it & I can't see from the diff whether
you changed any code in it
--- a/daemon/libvirtd.h
+++ b/daemon/libvirtd.h
@@ -45,6 +45,7 @@
# include <rpc/types.h>
# include <rpc/xdr.h>
# include "remote_protocol.h"
+# include "qemu_protocol.h"
I'm thinking this is possibly redundant - I feel it should be possible
to just include it in either the remote.c or dispatch.c file where
it is needed. Indeed the same probably applies for remote_protocol.h
too
diff --git a/daemon/qemu_generate_stubs.pl
b/daemon/qemu_generate_stubs.pl
new file mode 100755
index 0000000..870b5c5
--- /dev/null
+++ b/daemon/qemu_generate_stubs.pl
@@ -0,0 +1,170 @@
+#!/usr/bin/perl -w
+#
+# This script parses remote_protocol.x and produces lots of boilerplate
+# code for both ends of the remote connection.
+#
+# By Richard Jones <rjones(a)redhat.com>
IIUC, the only difference between this script & remote_generate_stubs.pl
is the method name prefix they're matching on. Could we just pass in the
desired method name prfix from the Makefile.am rule, avoiding the need
to duplicate the generator code ?
+#endif /* !_RP_QEMU_H_RPCGEN */
diff --git a/src/remote/qemu_protocol.x b/src/remote/qemu_protocol.x
new file mode 100644
index 0000000..e0593a8
--- /dev/null
+++ b/src/remote/qemu_protocol.x
@@ -0,0 +1,55 @@
+/* -*- c -*-
+ * qemu_protocol.x: private protocol for communicating between
+ * remote_internal driver and libvirtd. This protocol is
+ * internal and may change at any time.
+ *
+ * Copyright (C) 2010 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: Chris Lalancette <clalance(a)redhat.com>
+ */
+
+%#include "internal.h"
+%#include "remote_protocol.h"
+%#include <arpa/inet.h>
+
+/*----- Protocol. -----*/
+struct qemu_monitor_command_args {
+ remote_nonnull_domain domain;
+ remote_nonnull_string cmd;
+ int flags;
+};
+
+struct qemu_monitor_command_ret {
+ remote_nonnull_string result;
+};
+
+/* Define the program number, protocol version and procedure numbers here. */
+const QEMU_PROGRAM = 0x20008087;
+const QEMU_PROTOCOL_VERSION = 1;
+
+enum qemu_procedure {
+ QEMU_PROC_MONITOR_COMMAND = 1
+};
+
+struct qemu_message_header {
+ unsigned prog; /* QEMU_PROGRAM */
+ unsigned vers; /* QEMU_PROTOCOL_VERSION */
+ qemu_procedure proc; /* QEMU_PROC_x */
+ remote_message_type type;
+ unsigned serial; /* Serial number of message. */
+ remote_message_status status;
+};
I don't think we need, or should have, a separate message header definition
for QEMU. The daemon has to read & look at the complete header before it
can determine that this is the QEMU protocol vs remote protocol. Thus they
must be identical, or bad stuff will happen.
I guess the reason was for the 'qemu_procedure' enum, but could we just
turn that field into a plain 'unsigned' (which is the same byte size).
Despite all the comments, this all looks architecturally sound to me.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://deltacloud.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|