5:33 p.m.
On 07/18/2013 07:27 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Doing DBus method calls using libdbus.so is tedious in the
extreme. systemd developers came up with a nice high level
API for DBus method calls (sd_bus_call_method). While
systemd doesn't use libdbus.so, their API design can easily
be ported to libdbus.so.
This patch thus introduces methods virDBusCallMethod &
virDBusMessageRead, which are based on the code used for
sd_bus_call_method and sd_bus_message_read. This code in
systemd is under the LGPLv2+, so we're license compatible.
This code is probably pretty unintelligible unless you are
familiar with the DBus type system. So I added some API
docs trying to explain how to use them, as well as test
cases to validate that I didn't screw up the adaptation
from the original systemd code.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
Failed to build.
util/virdbus.c:1119:9: error: implicit declaration of function
'virReportDBusServiceError' [-Werror=implicit-function-declaration]
virReportDBusServiceError(error.message ? error.message :
"unknown error",
^
util/virdbus.c:1119:9: error: nested extern declaration of
'virReportDBusServiceError' [-Werror=nested-externs]
---
.gitignore | 1 +
src/libvirt_private.syms | 4 +
src/util/virdbus.c | 966 ++++++++++++++++++++++++++++++++++++++++++++++-
src/util/virdbus.h | 11 +
src/util/virdbuspriv.h | 45 +++
tests/Makefile.am | 13 +
tests/virdbustest.c | 389 +++++++++++++++++++
7 files changed, 1428 insertions(+), 1 deletion(-)
create mode 100644 src/util/virdbuspriv.h
create mode 100644 tests/virdbustest.c
+++ b/.gitignore
@@ -191,6 +191,7 @@
/tests/virbitmaptest
/tests/virbuftest
/tests/vircgrouptest
+/tests/virdbustest
Yay - an added test is reassuring (although I didn't actually run it,
due to the compile error).
+
+ if (virDBusSignatureLengthInternal(s + 1, true, arrayDepth+1, structDepth,
&t) < 0)
Inconsistent spacing around '+'.
+
+ while (*p != DBUS_STRUCT_END_CHAR) {
+ size_t t;
+
+ if (virDBusSignatureLengthInternal(p, false, arrayDepth, structDepth+1,
&t) < 0)
and again, probably also a long line worth wrapping.
+
+ if (virDBusSignatureLengthInternal(p, false, arrayDepth, structDepth+1,
&t) < 0)
Recurring theme on '+'; I'll quit pointing them out.
+
+/* Ideally, we'd just call ourselves recursively on every
+ * complex type. However, the state of a va_list that is
+ * passed to a function is undefined after that function
+ * returns. This means we need to docode the va_list linearly
d/docode/decode/
+ * in a single stackframe. We hence implement our own
+ * home-grown stack in an array. */
Is it also possible to use va_copy, or is that risking stack explosion
because we'd have to copy the list for each recursion?
+/**
+ * @conn: a DBus connection
Missing the method name 'virDBusCallMethod:' before the parameters.
+ * @replyout: pointer to receive reply message, or NULL
+ * @destination: bus identifier of the target service
+ * @path: object path of the target service
+ * @interface: the interface of the object
+ * @member: the name of the method in the interface
+ * @types: type signature for following method arguments
+ * @...: method arguments
+ * 'd' - 8-byte floating point, passed as a
'double'
+ * 's' - NULL terminated string, in UTF-8
+ * 'o' - NULL terminated string, representing a valid object path
+ * 'g' - NULL terminated string, representing a valid type signature
Technically, it's NUL-terminated (terminated by the one-byte character
NUL, not by the 4/8 byte pointer NULL), but we're inconsistent on this
so I don't care if you leave it.
+ * Example signatures, with their corresponding variadic
+ * args:
Thanks; this helps.
+ *
+ * - "a{sv}" - a hash table (aka array + dict entry)
+ *
+ * (3, "email", "s", "joe(a)blogs.com", "age",
"i", 35,
+ * "address", "as", 3, "Some house", "Some
road", "some city")
Wow, that adds up fast.
+ ret = -1;
+
+ if (!(reply = dbus_connection_send_with_reply_and_block(conn,
+ call,
+ 30 * 1000,
Worth a symbolic name for this magic number?
+
+/**
+ * virDBusMessageRead:
+ * @msg: the reply to decode
+ * @types: type signature for following return values
+ * @...: pointers in which to store return values
+ *
+ * The @types type signature is the same format as
+ * that used for the virDBusCallMethod. The difference
+ * is that each variadic parameter must be a pointer to
+ * be filled with the values. eg instead of passing an
+ * 'int', pass an 'int *'.
Are variants/structs/dicts allowed on decoding? If so, an example of
how to interleave intermediate types alongside pointers for receiving
contents may make sense.
+++ b/tests/virdbustest.c
+#include "testutils.h"
+
+#define VERIFY(t, a, b, f) \
These names are a bit cryptic, would it hurt to make them words?
+
+ if (!(msg = dbus_message_new_method_call("org.libvirt.test",
+ "/org/libvirt/test",
+ "org.libvirt.test.astrochicken",
+ "cluck"))) {
bawk bawk b-gaawkk :)
+
+ if (virDBusMessageDecode(msg,
+ "svs",
+ &out_str1,
+ "i", &out_int32,
+ &out_str2) < 0) {
Ah, so you CAN pass a variant to decode. I guess DBus is smart enough
to error out if any type signature fails to match the actual message
being decoded (that is, if I pass ["v", "i", &out_int32], but the
message was encoded with ["v", "s", "hello"], then I get a
proper
message about type mismatch rather than some random integer which
happens to represent 32-bits of the pointer value of "hello")? In fact,
it is worth a negative test, to prove that we handle mismatch in the
client's arguments vs. the message being decoded?
+
+ if (virDBusMessageDecode(msg,
+ "sais",
+ &out_str1,
+ 3, &out_int32a, &out_int32b, &out_int32c,
+ &out_str2) < 0) {
Likewise, what happens if our array is too small or too large compared
to what we are decoding? Too small is probably an error; does too large
work and just leave the extra pointers unmodified?
Overall, it looks pretty clean; I'm okay if you just post an interdiff
for the change you make to fix compilation instead of a full-blown v2.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org