On 07/18/2013 07:27 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@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@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@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