7:28 a.m.
This series is a bunch of fixes & improvements to the dbus
APIs needed for the forthcoming new firewall APIs.
Daniel P. Berrange (6):
Refactor dbus helper methods for method calls
Add DBus helper methods for creating reply messages
Remove bogus unref in virDBusMessageRead
Remove bogus call to dbus_set_error_from_message
Introduce alternate way to encode/decode arrays in DBus messages
Allow caller to handle DBus error messages
src/libvirt_private.syms | 5 +
src/util/virdbus.c | 426 ++++++++++++++++++++++++++++++++++++++++-------
src/util/virdbus.h | 27 +++
src/util/virdbuspriv.h | 1 -
src/util/virsystemd.c | 2 +
tests/virdbustest.c | 93 +++++++++++
tests/virsystemdmock.c | 13 +-
7 files changed, 499 insertions(+), 68 deletions(-)
--
1.8.5.3
7:28 a.m.
New subject: [libvirt] [PATCH 1/6] Refactor dbus helper methods for method calls
Split the virDBusMethodCall method into a couple of new methods
virDBusCall, virDBusCreateMethod and virDBusCreateMethodV.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/libvirt_private.syms | 3 +
src/util/virdbus.c | 207 +++++++++++++++++++++++++++++++++++++++--------
src/util/virdbus.h | 19 +++++
src/util/virdbuspriv.h | 1 -
4 files changed, 195 insertions(+), 35 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3baf766..6091c67 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1155,8 +1155,11 @@ virCryptoHashString;
# util/virdbus.h
+virDBusCall;
virDBusCallMethod;
virDBusCloseSystemBus;
+virDBusCreateMethod;
+virDBusCreateMethodV;
virDBusGetSessionBus;
virDBusGetSystemBus;
virDBusHasSystemBus;
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index e3236d8..d4e0381 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1049,23 +1049,21 @@ int virDBusMessageDecode(DBusMessage* msg,
# define VIR_DBUS_METHOD_CALL_TIMEOUT_MILLIS 30 * 1000
/**
- * virDBusCallMethod:
- * @conn: a DBus connection
- * @replyout: pointer to receive reply message, or NULL
+ * virDBusCreateMethodV:
+ * @call: pointer to be filled with a method call message
* @destination: bus identifier of the target service
* @path: object path of the target service
* @iface: the interface of the object
* @member: the name of the method in the interface
* @types: type signature for following method arguments
- * @...: method arguments
+ * @args: method arguments
*
- * This invokes a method on a remote service on the
- * DBus bus @conn. The @destination, @path, @iface
+ * This creates a DBus method call message and saves a
+ * pointer to it in @call. The @destination, @path, @iface
* and @member parameters identify the object method to
* be invoked. The optional @replyout parameter will be
- * filled with any reply to the method call. The
- * virDBusMethodReply method can be used to decode the
- * return values.
+ * filled with any reply to the method call. The method
+ * can be later invoked using virDBusCall.
*
* The @types parameter is a DBus signature describing
* the method call parameters which will be provided
@@ -1166,38 +1164,91 @@ int virDBusMessageDecode(DBusMessage* msg,
* (3, "email", "s", "joe(a)blogs.com", "age",
"i", 35,
* "address", "as", 3, "Some house", "Some
road", "some city")
*/
-
-int virDBusCallMethod(DBusConnection *conn,
- DBusMessage **replyout,
- const char *destination,
- const char *path,
- const char *iface,
- const char *member,
- const char *types, ...)
+int virDBusCreateMethodV(DBusMessage **call,
+ const char *destination,
+ const char *path,
+ const char *iface,
+ const char *member,
+ const char *types,
+ va_list args)
{
- DBusMessage *call = NULL;
- DBusMessage *reply = NULL;
- DBusError error;
int ret = -1;
- va_list args;
- dbus_error_init(&error);
-
- if (!(call = dbus_message_new_method_call(destination,
- path,
- iface,
- member))) {
+ if (!(*call = dbus_message_new_method_call(destination,
+ path,
+ iface,
+ member))) {
virReportOOMError();
goto cleanup;
}
+ if (virDBusMessageEncodeArgs(*call, types, args) < 0) {
+ dbus_message_unref(*call);
+ *call = NULL;
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
+ return ret;
+}
+
+
+/**
+ * virDBusCreateMethod:
+ * @call: pointer to be filled with a method call message
+ * @destination: bus identifier of the target service
+ * @path: object path of the target service
+ * @iface: the interface of the object
+ * @member: the name of the method in the interface
+ * @types: type signature for following method arguments
+ * @...: method arguments
+ *
+ * See virDBusCreateMethodV for a description of the
+ * behaviour of this method.
+ */
+int virDBusCreateMethod(DBusMessage **call,
+ const char *destination,
+ const char *path,
+ const char *iface,
+ const char *member,
+ const char *types, ...)
+{
+ va_list args;
+ int ret;
+
va_start(args, types);
- ret = virDBusMessageEncodeArgs(call, types, args);
+ ret = virDBusCreateMethodV(call, destination, path,
+ iface, member, types, args);
va_end(args);
- if (ret < 0)
- goto cleanup;
- ret = -1;
+ return ret;
+}
+
+
+/**
+ * virDBusCall:
+ * @conn: a DBus connection
+ * @call: pointer to a message to send
+ * @replyout: pointer to receive reply message, or NULL
+ *
+ * This invokes a method encoded in @call on a remote
+ * service on the DBus bus @conn. The optional @replyout
+ * parameter will be filled with any reply to the method
+ * call. The virDBusMethodReply method can be used to
+ * decode the return values.
+ *
+ * Returns 0 on success, or -1 upon error
+ */
+int virDBusCall(DBusConnection *conn,
+ DBusMessage *call,
+ DBusMessage **replyout)
+{
+ DBusMessage *reply = NULL;
+ DBusError error;
+ int ret = -1;
+
+ dbus_error_init(&error);
if (!(reply = dbus_connection_send_with_reply_and_block(conn,
call,
@@ -1217,10 +1268,8 @@ int virDBusCallMethod(DBusConnection *conn,
ret = 0;
-cleanup:
+ cleanup:
dbus_error_free(&error);
- if (call)
- dbus_message_unref(call);
if (reply) {
if (ret == 0 && replyout)
*replyout = reply;
@@ -1232,6 +1281,62 @@ cleanup:
/**
+ * virDBusCallMethod:
+ * @conn: a DBus connection
+ * @replyout: pointer to receive reply message, or NULL
+ * @destination: bus identifier of the target service
+ * @path: object path of the target service
+ * @iface: the interface of the object
+ * @member: the name of the method in the interface
+ * @types: type signature for following method arguments
+ * @...: method arguments
+ *
+ * This invokes a method on a remote service on the
+ * DBus bus @conn. The @destination, @path, @iface
+ * and @member parameters identify the object method to
+ * be invoked. The optional @replyout parameter will be
+ * filled with any reply to the method call. The
+ * virDBusMethodReply method can be used to decode the
+ * return values.
+ *
+ * The @types parameter is a DBus signature describing
+ * the method call parameters which will be provided
+ * as variadic args. See virDBusCreateMethodV for a
+ * description of this parameter.
+ *
+ * Returns: 0 on success, -1 on error
+ */
+int virDBusCallMethod(DBusConnection *conn,
+ DBusMessage **replyout,
+ const char *destination,
+ const char *path,
+ const char *iface,
+ const char *member,
+ const char *types, ...)
+{
+ DBusMessage *call = NULL;
+ int ret = -1;
+ va_list args;
+
+ va_start(args, types);
+ ret = virDBusCreateMethodV(&call, destination, path,
+ iface, member, types, args);
+ va_end(args);
+ if (ret < 0)
+ goto cleanup;
+
+ ret = -1;
+
+ ret = virDBusCall(conn, call, replyout);
+
+cleanup:
+ if (call)
+ dbus_message_unref(call);
+ return ret;
+}
+
+
+/**
* virDBusMessageRead:
* @msg: the reply to decode
* @types: type signature for following return values
@@ -1369,6 +1474,40 @@ DBusConnection *virDBusGetSessionBus(void)
return NULL;
}
+int virDBusCreateMethod(DBusMessage **call ATTRIBUTE_UNUSED,
+ const char *destination ATTRIBUTE_UNUSED,
+ const char *path ATTRIBUTE_UNUSED,
+ const char *iface ATTRIBUTE_UNUSED,
+ const char *member ATTRIBUTE_UNUSED,
+ const char *types ATTRIBUTE_UNUSED, ...)
+{
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("DBus support not compiled into this
binary"));
+ return -1;
+}
+
+int virDBusCreateMethodV(DBusMessage **call ATTRIBUTE_UNUSED,
+ const char *destination ATTRIBUTE_UNUSED,
+ const char *path ATTRIBUTE_UNUSED,
+ const char *iface ATTRIBUTE_UNUSED,
+ const char *member ATTRIBUTE_UNUSED,
+ const char *types ATTRIBUTE_UNUSED,
+ va_list args ATTRIBUTE_UNUSED)
+{
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("DBus support not compiled into this
binary"));
+ return -1;
+}
+
+int virDBusCall(DBusConnection *conn ATTRIBUTE_UNUSED,
+ DBusMessage *call ATTRIBUTE_UNUSED,
+ DBusMessage **reply ATTRIBUTE_UNUSED)
+{
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("DBus support not compiled into this
binary"));
+ return -1;
+}
+
int virDBusCallMethod(DBusConnection *conn ATTRIBUTE_UNUSED,
DBusMessage **reply ATTRIBUTE_UNUSED,
const char *destination ATTRIBUTE_UNUSED,
diff --git a/src/util/virdbus.h b/src/util/virdbus.h
index 1ca8641..e191a28 100644
--- a/src/util/virdbus.h
+++ b/src/util/virdbus.h
@@ -31,6 +31,8 @@
# endif
# include "internal.h"
+# include <stdarg.h>
+
void virDBusSetSharedBus(bool shared);
DBusConnection *virDBusGetSystemBus(void);
@@ -38,6 +40,20 @@ bool virDBusHasSystemBus(void);
void virDBusCloseSystemBus(void);
DBusConnection *virDBusGetSessionBus(void);
+int virDBusCreateMethod(DBusMessage **call,
+ const char *destination,
+ const char *path,
+ const char *iface,
+ const char *member,
+ const char *types, ...);
+int virDBusCreateMethodV(DBusMessage **call,
+ const char *destination,
+ const char *path,
+ const char *iface,
+ const char *member,
+ const char *types,
+ va_list args);
+
int virDBusCallMethod(DBusConnection *conn,
DBusMessage **reply,
const char *destination,
@@ -45,6 +61,9 @@ int virDBusCallMethod(DBusConnection *conn,
const char *iface,
const char *member,
const char *types, ...);
+int virDBusCall(DBusConnection *conn,
+ DBusMessage *call,
+ DBusMessage **reply);
int virDBusMessageRead(DBusMessage *msg,
const char *types, ...);
diff --git a/src/util/virdbuspriv.h b/src/util/virdbuspriv.h
index a4ff655..d45fb25 100644
--- a/src/util/virdbuspriv.h
+++ b/src/util/virdbuspriv.h
@@ -24,7 +24,6 @@
# include "virdbus.h"
-# include <stdarg.h>
int virDBusMessageEncodeArgs(DBusMessage* msg,
const char *types,
--
1.8.5.3
12:16 p.m.
New subject: [libvirt] [PATCH 1/6] Refactor dbus helper methods for method calls
On 20.03.2014 13:28, Daniel P. Berrange wrote:
Split the virDBusMethodCall method into a couple of new methods
virDBusCall, virDBusCreateMethod and virDBusCreateMethodV.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/libvirt_private.syms | 3 +
src/util/virdbus.c | 207 +++++++++++++++++++++++++++++++++++++++--------
src/util/virdbus.h | 19 +++++
src/util/virdbuspriv.h | 1 -
4 files changed, 195 insertions(+), 35 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3baf766..6091c67 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1155,8 +1155,11 @@ virCryptoHashString;
# util/virdbus.h
+virDBusCall;
virDBusCallMethod;
virDBusCloseSystemBus;
+virDBusCreateMethod;
+virDBusCreateMethodV;
virDBusGetSessionBus;
virDBusGetSystemBus;
virDBusHasSystemBus;
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index e3236d8..d4e0381 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1049,23 +1049,21 @@ int virDBusMessageDecode(DBusMessage* msg,
# define VIR_DBUS_METHOD_CALL_TIMEOUT_MILLIS 30 * 1000
/**
- * virDBusCallMethod:
- * @conn: a DBus connection
- * @replyout: pointer to receive reply message, or NULL
+ * virDBusCreateMethodV:
+ * @call: pointer to be filled with a method call message
* @destination: bus identifier of the target service
* @path: object path of the target service
* @iface: the interface of the object
* @member: the name of the method in the interface
* @types: type signature for following method arguments
- * @...: method arguments
+ * @args: method arguments
*
- * This invokes a method on a remote service on the
- * DBus bus @conn. The @destination, @path, @iface
+ * This creates a DBus method call message and saves a
+ * pointer to it in @call. The @destination, @path, @iface
* and @member parameters identify the object method to
* be invoked. The optional @replyout parameter will be
- * filled with any reply to the method call. The
- * virDBusMethodReply method can be used to decode the
- * return values.
+ * filled with any reply to the method call. The method
+ * can be later invoked using virDBusCall.
*
* The @types parameter is a DBus signature describing
* the method call parameters which will be provided
@@ -1166,38 +1164,91 @@ int virDBusMessageDecode(DBusMessage* msg,
* (3, "email", "s", "joe(a)blogs.com",
"age", "i", 35,
* "address", "as", 3, "Some house", "Some
road", "some city")
*/
-
-int virDBusCallMethod(DBusConnection *conn,
- DBusMessage **replyout,
- const char *destination,
- const char *path,
- const char *iface,
- const char *member,
- const char *types, ...)
+int virDBusCreateMethodV(DBusMessage **call,
+ const char *destination,
+ const char *path,
+ const char *iface,
+ const char *member,
+ const char *types,
+ va_list args)
{
- DBusMessage *call = NULL;
- DBusMessage *reply = NULL;
- DBusError error;
int ret = -1;
- va_list args;
- dbus_error_init(&error);
-
- if (!(call = dbus_message_new_method_call(destination,
- path,
- iface,
- member))) {
+ if (!(*call = dbus_message_new_method_call(destination,
+ path,
+ iface,
+ member))) {
virReportOOMError();
goto cleanup;
}
+ if (virDBusMessageEncodeArgs(*call, types, args) < 0) {
+ dbus_message_unref(*call);
+ *call = NULL;
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
Indentation's off
+ return ret;
+}
+
+
+/**
+ * virDBusCreateMethod:
+ * @call: pointer to be filled with a method call message
+ * @destination: bus identifier of the target service
+ * @path: object path of the target service
+ * @iface: the interface of the object
+ * @member: the name of the method in the interface
+ * @types: type signature for following method arguments
+ * @...: method arguments
+ *
+ * See virDBusCreateMethodV for a description of the
+ * behaviour of this method.
+ */
+int virDBusCreateMethod(DBusMessage **call,
+ const char *destination,
+ const char *path,
+ const char *iface,
+ const char *member,
+ const char *types, ...)
+{
+ va_list args;
+ int ret;
+
va_start(args, types);
- ret = virDBusMessageEncodeArgs(call, types, args);
+ ret = virDBusCreateMethodV(call, destination, path,
+ iface, member, types, args);
va_end(args);
- if (ret < 0)
- goto cleanup;
- ret = -1;
+ return ret;
+}
+
+
+/**
+ * virDBusCall:
+ * @conn: a DBus connection
+ * @call: pointer to a message to send
+ * @replyout: pointer to receive reply message, or NULL
+ *
+ * This invokes a method encoded in @call on a remote
+ * service on the DBus bus @conn. The optional @replyout
+ * parameter will be filled with any reply to the method
+ * call. The virDBusMethodReply method can be used to
+ * decode the return values.
+ *
+ * Returns 0 on success, or -1 upon error
+ */
+int virDBusCall(DBusConnection *conn,
+ DBusMessage *call,
+ DBusMessage **replyout)
+{
+ DBusMessage *reply = NULL;
+ DBusError error;
+ int ret = -1;
+
+ dbus_error_init(&error);
if (!(reply = dbus_connection_send_with_reply_and_block(conn,
call,
@@ -1217,10 +1268,8 @@ int virDBusCallMethod(DBusConnection *conn,
ret = 0;
-cleanup:
+ cleanup:
And here as well.
dbus_error_free(&error);
- if (call)
- dbus_message_unref(call);
if (reply) {
if (ret == 0 && replyout)
*replyout = reply;
ACK with that fixed.
Michal
7:28 a.m.
New subject: [libvirt] [PATCH 2/6] Add DBus helper methods for creating reply messages
The test suites often have to create DBus method reply messages
with payloads. Create two helpers for simplifying the process
of creating replies with payloads.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/libvirt_private.syms | 2 ++
src/util/virdbus.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
src/util/virdbus.h | 5 ++++
3 files changed, 67 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6091c67..6a89d1f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1160,6 +1160,8 @@ virDBusCallMethod;
virDBusCloseSystemBus;
virDBusCreateMethod;
virDBusCreateMethodV;
+virDBusCreateReply;
+virDBusCreateReplyV;
virDBusGetSessionBus;
virDBusGetSystemBus;
virDBusHasSystemBus;
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index d4e0381..d5d3e01 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1227,6 +1227,66 @@ int virDBusCreateMethod(DBusMessage **call,
/**
+ * virDBusCreateReplyV:
+ * @reply: pointer to be filled with a method reply message
+ * @types: type signature for following method arguments
+ * @args: method arguments
+ *
+ * This creates a DBus method reply message and saves a
+ * pointer to it in @reply.
+ *
+ * The @types parameter is a DBus signature describing
+ * the method call parameters which will be provided
+ * as variadic args. See virDBusCreateMethodV for a
+ * description of this parameter.
+ */
+int virDBusCreateReplyV(DBusMessage **reply,
+ const char *types,
+ va_list args)
+{
+ int ret = -1;
+
+ if (!(*reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (virDBusMessageEncodeArgs(*reply, types, args) < 0) {
+ dbus_message_unref(*reply);
+ *reply = NULL;
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
+ return ret;
+}
+
+
+/**
+ * virDBusCreateReply:
+ * @reply: pointer to be filled with a method reply message
+ * @types: type signature for following method arguments
+ * @...: method arguments
+ *
+ * See virDBusCreateReplyV for a description of the
+ * behaviour of this method.
+ */
+int virDBusCreateReply(DBusMessage **reply,
+ const char *types, ...)
+{
+ va_list args;
+ int ret;
+
+ va_start(args, types);
+ ret = virDBusCreateReplyV(reply, types, args);
+ va_end(args);
+
+ return ret;
+}
+
+
+/**
* virDBusCall:
* @conn: a DBus connection
* @call: pointer to a message to send
diff --git a/src/util/virdbus.h b/src/util/virdbus.h
index e191a28..4fbda87 100644
--- a/src/util/virdbus.h
+++ b/src/util/virdbus.h
@@ -53,6 +53,11 @@ int virDBusCreateMethodV(DBusMessage **call,
const char *member,
const char *types,
va_list args);
+int virDBusCreateReply(DBusMessage **reply,
+ const char *types, ...);
+int virDBusCreateReplyV(DBusMessage **reply,
+ const char *types,
+ va_list args);
int virDBusCallMethod(DBusConnection *conn,
DBusMessage **reply,
--
1.8.5.3
12:16 p.m.
New subject: [libvirt] [PATCH 2/6] Add DBus helper methods for creating reply messages
On 20.03.2014 13:28, Daniel P. Berrange wrote:
The test suites often have to create DBus method reply messages
with payloads. Create two helpers for simplifying the process
of creating replies with payloads.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/libvirt_private.syms | 2 ++
src/util/virdbus.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
src/util/virdbus.h | 5 ++++
3 files changed, 67 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6091c67..6a89d1f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1160,6 +1160,8 @@ virDBusCallMethod;
virDBusCloseSystemBus;
virDBusCreateMethod;
virDBusCreateMethodV;
+virDBusCreateReply;
+virDBusCreateReplyV;
virDBusGetSessionBus;
virDBusGetSystemBus;
virDBusHasSystemBus;
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index d4e0381..d5d3e01 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1227,6 +1227,66 @@ int virDBusCreateMethod(DBusMessage **call,
/**
+ * virDBusCreateReplyV:
+ * @reply: pointer to be filled with a method reply message
+ * @types: type signature for following method arguments
+ * @args: method arguments
+ *
+ * This creates a DBus method reply message and saves a
+ * pointer to it in @reply.
+ *
+ * The @types parameter is a DBus signature describing
+ * the method call parameters which will be provided
+ * as variadic args. See virDBusCreateMethodV for a
+ * description of this parameter.
+ */
+int virDBusCreateReplyV(DBusMessage **reply,
+ const char *types,
+ va_list args)
+{
+ int ret = -1;
+
+ if (!(*reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (virDBusMessageEncodeArgs(*reply, types, args) < 0) {
+ dbus_message_unref(*reply);
+ *reply = NULL;
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
Indentation's off.
+ return ret;
+}
+
+
ACK with that fixed.
Michal
12:28 p.m.
New subject: [libvirt] [PATCH 2/6] Add DBus helper methods for creating reply messages
On Thu, Mar 20, 2014 at 06:16:08PM +0100, Michal Privoznik wrote:
On 20.03.2014 13:28, Daniel P. Berrange wrote:
>The test suites often have to create DBus method reply messages
>with payloads. Create two helpers for simplifying the process
>of creating replies with payloads.
>
>Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
>---
> src/libvirt_private.syms | 2 ++
> src/util/virdbus.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/util/virdbus.h | 5 ++++
> 3 files changed, 67 insertions(+)
>+ ret = 0;
>+ cleanup:
Indentation's off.
Do we actually have an indentation rule for labels ?
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
12:39 p.m.
New subject: [libvirt] [PATCH 2/6] Add DBus helper methods for creating reply messages
On 03/20/2014 11:28 AM, Daniel P. Berrange wrote:
On Thu, Mar 20, 2014 at 06:16:08PM +0100, Michal Privoznik wrote:
> On 20.03.2014 13:28, Daniel P. Berrange wrote:
>> The test suites often have to create DBus method reply messages
>> with payloads. Create two helpers for simplifying the process
>> of creating replies with payloads.
>>
>> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
>> ---
>> src/libvirt_private.syms | 2 ++
>> src/util/virdbus.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
>> src/util/virdbus.h | 5 ++++
>> 3 files changed, 67 insertions(+)
>> + ret = 0;
>> + cleanup:
>
> Indentation's off.
Do we actually have an indentation rule for labels ?
Most code starts it in the first column, with no leading space.
In some respects, emacs doesn't handle it well (it assumes anything in
the first column is a function name, so it tries to treat the label as a
function name when generating changelog templates); on the other hand,
when I hit TAB on a label, emacs reindents it to the first column (that
is, our .dir-locals.el requests c-file-style "K&R", and apparently that
style includes putting labels at one indentation layer less than the
rest of the code; so if you are labelling something that indents four
spaces, the label gets indented 0 spaces).
I've been going by the general rule of thumb that if emacs reindents
something, then my style wasn't consistent with the bulk of the code;
but I agree that HACKING doesn't actually mention this, and not everyone
uses emacs.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
5:51 a.m.
New subject: [libvirt] [PATCH 2/6] Add DBus helper methods for creating reply messages
On Thu, Mar 20, 2014 at 11:39:18AM -0600, Eric Blake wrote:
On 03/20/2014 11:28 AM, Daniel P. Berrange wrote:
> On Thu, Mar 20, 2014 at 06:16:08PM +0100, Michal Privoznik wrote:
>> On 20.03.2014 13:28, Daniel P. Berrange wrote:
>>> The test suites often have to create DBus method reply messages
>>> with payloads. Create two helpers for simplifying the process
>>> of creating replies with payloads.
>>>
>>> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
>>> ---
>>> src/libvirt_private.syms | 2 ++
>>> src/util/virdbus.c | 60
++++++++++++++++++++++++++++++++++++++++++++++++
>>> src/util/virdbus.h | 5 ++++
>>> 3 files changed, 67 insertions(+)
>
>
>>> + ret = 0;
>>> + cleanup:
>>
>> Indentation's off.
>
> Do we actually have an indentation rule for labels ?
Most code starts it in the first column, with no leading space.
In some respects, emacs doesn't handle it well (it assumes anything in
the first column is a function name, so it tries to treat the label as a
function name when generating changelog templates); on the other hand,
when I hit TAB on a label, emacs reindents it to the first column (that
is, our .dir-locals.el requests c-file-style "K&R", and apparently that
style includes putting labels at one indentation layer less than the
rest of the code; so if you are labelling something that indents four
spaces, the label gets indented 0 spaces).
I've been going by the general rule of thumb that if emacs reindents
something, then my style wasn't consistent with the bulk of the code;
but I agree that HACKING doesn't actually mention this, and not everyone
uses emacs.
Yep, I'm going with what emacs does, which is indent by 1 space here.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
7:13 a.m.
New subject: [libvirt] [PATCH 2/6] Add DBus helper methods for creating reply messages
On Fri, Mar 21, 2014 at 10:51:58 +0000, Daniel Berrange wrote:
On Thu, Mar 20, 2014 at 11:39:18AM -0600, Eric Blake wrote:
> On 03/20/2014 11:28 AM, Daniel P. Berrange wrote:
> > On Thu, Mar 20, 2014 at 06:16:08PM +0100, Michal Privoznik wrote:
> >> On 20.03.2014 13:28, Daniel P. Berrange wrote:
> >>> The test suites often have to create DBus method reply messages
> >>> with payloads. Create two helpers for simplifying the process
> >>> of creating replies with payloads.
> >>>
> >>> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> >>> ---
> >>> src/libvirt_private.syms | 2 ++
> >>> src/util/virdbus.c | 60
++++++++++++++++++++++++++++++++++++++++++++++++
> >>> src/util/virdbus.h | 5 ++++
> >>> 3 files changed, 67 insertions(+)
> >
> >
> >>> + ret = 0;
> >>> + cleanup:
> >>
> >> Indentation's off.
> >
> > Do we actually have an indentation rule for labels ?
>
> Most code starts it in the first column, with no leading space.
>
> In some respects, emacs doesn't handle it well (it assumes anything in
> the first column is a function name, so it tries to treat the label as a
> function name when generating changelog templates); on the other hand,
> when I hit TAB on a label, emacs reindents it to the first column (that
> is, our .dir-locals.el requests c-file-style "K&R", and apparently
that
> style includes putting labels at one indentation layer less than the
> rest of the code; so if you are labelling something that indents four
> spaces, the label gets indented 0 spaces).
>
> I've been going by the general rule of thumb that if emacs reindents
> something, then my style wasn't consistent with the bulk of the code;
> but I agree that HACKING doesn't actually mention this, and not everyone
> uses emacs.
Yep, I'm going with what emacs does, which is indent by 1 space here.
And since it makes git diff produce better results too, I guess we could
just start doing that everytime. Even vim can be told to indent labels
by one space:
:set cinoptions=L3
If you think 3 != 1, it's because 4 - 3 = 1, where 4 is the normal
indent.
Jirka
7:38 a.m.
New subject: [libvirt] [PATCH 2/6] Add DBus helper methods for creating reply messages
On 03/21/2014 04:51 AM, Daniel P. Berrange wrote:
>
> I've been going by the general rule of thumb that if emacs reindents
> something, then my style wasn't consistent with the bulk of the code;
> but I agree that HACKING doesn't actually mention this, and not everyone
> uses emacs.
Yep, I'm going with what emacs does, which is indent by 1 space here.
Okay, then something is odd here, if your emacs indents by 1 space and
mine doesn't. I'm not sure what is different between our two settings,
but it would be nice to get it codified into .dir-locals.el so that it
works the same for all emacs users.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
7:28 a.m.
New subject: [libvirt] [PATCH 3/6] Remove bogus unref in virDBusMessageRead
The virDBusMessageRead method should not have side-effects on
the message parameter passed in, so unref'ing it is wrong.
The caller should unref only when they decided they are done
with it.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/util/virdbus.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index d5d3e01..85f8e29 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1419,7 +1419,6 @@ int virDBusMessageRead(DBusMessage *msg,
ret = virDBusMessageDecodeArgs(msg, types, args);
va_end(args);
- dbus_message_unref(msg);
return ret;
}
--
1.8.5.3
12:16 p.m.
New subject: [libvirt] [PATCH 3/6] Remove bogus unref in virDBusMessageRead
On 20.03.2014 13:28, Daniel P. Berrange wrote:
The virDBusMessageRead method should not have side-effects on
the message parameter passed in, so unref'ing it is wrong.
The caller should unref only when they decided they are done
with it.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/util/virdbus.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index d5d3e01..85f8e29 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1419,7 +1419,6 @@ int virDBusMessageRead(DBusMessage *msg,
ret = virDBusMessageDecodeArgs(msg, types, args);
va_end(args);
- dbus_message_unref(msg);
return ret;
}
ACK, and there's no new mem leak as this functions is not used anywhere yet.
Michal
7:28 a.m.
New subject: [libvirt] [PATCH 4/6] Remove bogus call to dbus_set_error_from_message
The dbus_connection_send_with_reply_and_block method will
automatically call dbus_set_error_from_message for us. We
mistakenly thought we had todo it because of a flaw in the
systemd unit test mock impl. The latter should have directly
set the error object, instead of creating an error message
object.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/util/virdbus.c | 7 -------
tests/virsystemdmock.c | 13 +++----------
2 files changed, 3 insertions(+), 17 deletions(-)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index 85f8e29..8a978e5 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1319,13 +1319,6 @@ int virDBusCall(DBusConnection *conn,
goto cleanup;
}
- if (dbus_set_error_from_message(&error,
- reply)) {
- virReportDBusServiceError(error.message ? error.message : "unknown
error",
- error.name);
- goto cleanup;
- }
-
ret = 0;
cleanup:
diff --git a/tests/virsystemdmock.c b/tests/virsystemdmock.c
index b4fcf6e..23167db 100644
--- a/tests/virsystemdmock.c
+++ b/tests/virsystemdmock.c
@@ -70,16 +70,9 @@ DBusMessage *dbus_connection_send_with_reply_and_block(DBusConnection
*connectio
if (STREQ(service, "org.freedesktop.machine1")) {
if (getenv("FAIL_BAD_SERVICE")) {
- DBusMessageIter iter;
- const char *error_message = "Something went wrong creating the
machine";
- if (!(reply = dbus_message_new(DBUS_MESSAGE_TYPE_ERROR)))
- return NULL;
- dbus_message_set_error_name(reply,
"org.freedesktop.systemd.badthing");
- dbus_message_iter_init_append(reply, &iter);
- if (!dbus_message_iter_append_basic(&iter,
- DBUS_TYPE_STRING,
- &error_message))
- goto error;
+ dbus_set_error_const(error,
+ "org.freedesktop.systemd.badthing",
+ "Something went wrong creating the machine");
} else {
reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN);
}
--
1.8.5.3
12:15 p.m.
New subject: [libvirt] [PATCH 4/6] Remove bogus call to dbus_set_error_from_message
On 20.03.2014 13:28, Daniel P. Berrange wrote:
The dbus_connection_send_with_reply_and_block method will
automatically call dbus_set_error_from_message for us. We
mistakenly thought we had todo it because of a flaw in the
systemd unit test mock impl. The latter should have directly
set the error object, instead of creating an error message
object.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/util/virdbus.c | 7 -------
tests/virsystemdmock.c | 13 +++----------
2 files changed, 3 insertions(+), 17 deletions(-)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index 85f8e29..8a978e5 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1319,13 +1319,6 @@ int virDBusCall(DBusConnection *conn,
goto cleanup;
}
- if (dbus_set_error_from_message(&error,
- reply)) {
- virReportDBusServiceError(error.message ? error.message : "unknown
error",
- error.name);
- goto cleanup;
- }
-
ret = 0;
cleanup:
diff --git a/tests/virsystemdmock.c b/tests/virsystemdmock.c
index b4fcf6e..23167db 100644
--- a/tests/virsystemdmock.c
+++ b/tests/virsystemdmock.c
@@ -70,16 +70,9 @@ DBusMessage *dbus_connection_send_with_reply_and_block(DBusConnection
*connectio
if (STREQ(service, "org.freedesktop.machine1")) {
if (getenv("FAIL_BAD_SERVICE")) {
- DBusMessageIter iter;
- const char *error_message = "Something went wrong creating the
machine";
- if (!(reply = dbus_message_new(DBUS_MESSAGE_TYPE_ERROR)))
- return NULL;
- dbus_message_set_error_name(reply,
"org.freedesktop.systemd.badthing");
- dbus_message_iter_init_append(reply, &iter);
- if (!dbus_message_iter_append_basic(&iter,
- DBUS_TYPE_STRING,
- &error_message))
- goto error;
+ dbus_set_error_const(error,
+ "org.freedesktop.systemd.badthing",
+ "Something went wrong creating the
machine");
} else {
reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN);
}
ACK
Michal
7:28 a.m.
New subject: [libvirt] [PATCH 5/6] Introduce alternate way to encode/decode arrays in DBus messages
Currently the DBus helper APIs require the values for an array
to be passed inline in the variadic argument list. This change
introduces support for passing arrays using a pointer to a plain
C array of the basic type. This is of particular benefit for
decoding messages when you don't know how many array elements
are being received.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/util/virdbus.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++------
tests/virdbustest.c | 93 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 201 insertions(+), 13 deletions(-)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index 8a978e5..d208646 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -456,7 +456,7 @@ static int virDBusTypeStackPush(virDBusTypeStack **stack,
(*stack)[(*nstack) - 1].types = types;
(*stack)[(*nstack) - 1].nstruct = nstruct;
(*stack)[(*nstack) - 1].narray = narray;
- VIR_DEBUG("Pushed '%s'", types);
+ VIR_DEBUG("Pushed types='%s' nstruct=%zu narray=%zu", types,
nstruct, narray);
return 0;
}
@@ -478,7 +478,7 @@ static int virDBusTypeStackPop(virDBusTypeStack **stack,
*types = (*stack)[(*nstack) - 1].types;
*nstruct = (*stack)[(*nstack) - 1].nstruct;
*narray = (*stack)[(*nstack) - 1].narray;
- VIR_DEBUG("Popped '%s'", *types);
+ VIR_DEBUG("Popped types='%s' nstruct=%zu narray=%zu", *types,
*nstruct, *narray);
VIR_SHRINK_N(*stack, *nstack, 1);
return 0;
@@ -501,16 +501,25 @@ static void virDBusTypeStackFree(virDBusTypeStack **stack,
# define SET_NEXT_VAL(dbustype, vargtype, sigtype, fmt) \
do { \
- dbustype x = (dbustype)va_arg(args, vargtype); \
+ dbustype x; \
+ if (arrayref) { \
+ vargtype *valarray = arrayptr; \
+ x = (dbustype)*valarray; \
+ valarray++; \
+ arrayptr = valarray; \
+ } else { \
+ x = (dbustype)va_arg(args, vargtype); \
+ } \
if (!dbus_message_iter_append_basic(iter, sigtype, &x)) { \
virReportError(VIR_ERR_INTERNAL_ERROR, \
- _("Cannot append basic type %s"), #vargtype); \
+ _("Cannot append basic type %s"), #vargtype);\
goto cleanup; \
} \
- VIR_DEBUG("Appended basic type '" #dbustype "' varg
'" #vargtype \
+ VIR_DEBUG("Appended basic type '" #dbustype "' varg
'" #vargtype\
"' sig '%c' val '" fmt "'",
sigtype, (vargtype)x); \
} while (0)
+
static int
virDBusMessageIterEncode(DBusMessageIter *rootiter,
const char *types,
@@ -519,6 +528,8 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
int ret = -1;
size_t narray;
size_t nstruct;
+ bool arrayref = false;
+ void *arrayptr = NULL;
virDBusTypeStack *stack = NULL;
size_t nstack = 0;
size_t siglen;
@@ -544,6 +555,8 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
(narray == (size_t)-1 &&
nstruct == 0)) {
DBusMessageIter *thisiter = iter;
+ arrayref = false;
+ arrayptr = NULL;
VIR_DEBUG("Popping iter=%p", iter);
if (nstack == 0)
break;
@@ -616,12 +629,32 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
break;
case DBUS_TYPE_ARRAY:
+ arrayptr = NULL;
+ if (t[1] == '&') {
+ VIR_DEBUG("Got array ref");
+ t++;
+ types++;
+ nstruct--;
+ arrayref = true;
+ } else {
+ VIR_DEBUG("Got array non-ref");
+ arrayref = false;
+ }
+
if (virDBusSignatureLength(t + 1, &siglen) < 0)
goto cleanup;
if (VIR_STRNDUP(contsig, t + 1, siglen) < 0)
goto cleanup;
+ if (arrayref && (strlen(contsig) > 1 ||
+ !virDBusIsBasicType(*contsig))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Got array ref but '%s' is not a single
basic type"),
+ contsig);
+ goto cleanup;
+ }
+
if (narray == (size_t)-1) {
types += siglen;
nstruct -= siglen;
@@ -644,7 +677,9 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
newiter = NULL;
types = t + 1;
nstruct = siglen;
- narray = va_arg(args, int);
+ narray = (size_t)va_arg(args, int);
+ if (arrayref)
+ arrayptr = va_arg(args, void *);
break;
case DBUS_TYPE_VARIANT:
@@ -710,8 +745,9 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unknown type in signature '%s'"),
- types);
+ _("Unknown type '%c' in signature
'%s'"),
+ *t, types);
+ goto cleanup;
}
}
@@ -739,7 +775,16 @@ cleanup:
# define GET_NEXT_VAL(dbustype, vargtype, fmt) \
do { \
- dbustype *x = (dbustype *)va_arg(args, vargtype *); \
+ dbustype *x; \
+ if (arrayref) { \
+ vargtype **xptrptr = arrayptr; \
+ if (VIR_EXPAND_N(*xptrptr, *narrayptr, 1) < 0) \
+ goto cleanup; \
+ x = (dbustype *)(*xptrptr + (*narrayptr - 1)); \
+ VIR_DEBUG("Expanded to %zu", *narrayptr); \
+ } else { \
+ x = (dbustype *)va_arg(args, vargtype *); \
+ } \
dbus_message_iter_get_basic(iter, x); \
VIR_DEBUG("Read basic type '" #dbustype "' varg
'" #vargtype \
"' val '" fmt "'", (vargtype)*x);
\
@@ -754,6 +799,9 @@ virDBusMessageIterDecode(DBusMessageIter *rootiter,
int ret = -1;
size_t narray;
size_t nstruct;
+ bool arrayref = false;
+ void *arrayptr = NULL;
+ size_t *narrayptr = 0;
virDBusTypeStack *stack = NULL;
size_t nstack = 0;
size_t siglen;
@@ -780,6 +828,8 @@ virDBusMessageIterDecode(DBusMessageIter *rootiter,
(narray == (size_t)-1 &&
nstruct == 0)) {
DBusMessageIter *thisiter = iter;
+ arrayref = false;
+ arrayptr = NULL;
VIR_DEBUG("Popping iter=%p", iter);
if (nstack == 0)
break;
@@ -849,7 +899,16 @@ virDBusMessageIterDecode(DBusMessageIter *rootiter,
case DBUS_TYPE_OBJECT_PATH:
case DBUS_TYPE_SIGNATURE:
do {
- char **x = (char **)va_arg(args, char **);
+ char **x;
+ if (arrayref) {
+ char ***xptrptr = arrayptr;
+ if (VIR_EXPAND_N(*xptrptr, *narrayptr, 1) < 0)
+ goto cleanup;
+ x = (char **)(*xptrptr + (*narrayptr - 1));
+ VIR_DEBUG("Expanded to %zu", *narrayptr);
+ } else {
+ x = (char **)va_arg(args, char **);
+ }
char *s;
dbus_message_iter_get_basic(iter, &s);
if (VIR_STRDUP(*x, s) < 0)
@@ -860,6 +919,18 @@ virDBusMessageIterDecode(DBusMessageIter *rootiter,
break;
case DBUS_TYPE_ARRAY:
+ arrayptr = NULL;
+ if (t[1] == '&') {
+ VIR_DEBUG("Got array ref");
+ t++;
+ types++;
+ nstruct--;
+ arrayref = true;
+ } else {
+ VIR_DEBUG("Got array non-ref");
+ arrayref = false;
+ }
+
advanceiter = false;
if (virDBusSignatureLength(t + 1, &siglen) < 0)
goto cleanup;
@@ -867,6 +938,14 @@ virDBusMessageIterDecode(DBusMessageIter *rootiter,
if (VIR_STRNDUP(contsig, t + 1, siglen) < 0)
goto cleanup;
+ if (arrayref && (strlen(contsig) > 1 ||
+ !virDBusIsBasicType(*contsig))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Got array ref but '%s' is not a single
basic type"),
+ contsig);
+ goto cleanup;
+ }
+
if (narray == (size_t)-1) {
types += siglen;
nstruct -= siglen;
@@ -885,7 +964,14 @@ virDBusMessageIterDecode(DBusMessageIter *rootiter,
newiter = NULL;
types = t + 1;
nstruct = siglen;
- narray = va_arg(args, int);
+ if (arrayref) {
+ narrayptr = va_arg(args, size_t *);
+ arrayptr = va_arg(args, void *);
+ *narrayptr = 0;
+ *(char **)arrayptr = NULL;
+ } else {
+ narray = va_arg(args, int);
+ }
break;
case DBUS_TYPE_VARIANT:
@@ -945,8 +1031,17 @@ virDBusMessageIterDecode(DBusMessageIter *rootiter,
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unknown type in signature '%s'"),
- types);
+ _("Unknown type '%c' in signature
'%s'"),
+ *t, types);
+ goto cleanup;
+ }
+
+ if (arrayref) {
+ if (*t == '&' ||
+ dbus_message_iter_has_next(iter))
+ narray = 1;
+ else
+ narray = 0;
}
VIR_DEBUG("After stack=%zu array=%zu struct=%zu type='%s'",
diff --git a/tests/virdbustest.c b/tests/virdbustest.c
index 9a6c4c6..2269c8d 100644
--- a/tests/virdbustest.c
+++ b/tests/virdbustest.c
@@ -228,6 +228,97 @@ cleanup:
return ret;
}
+static int testMessageArrayRef(const void *args ATTRIBUTE_UNUSED)
+{
+ DBusMessage *msg = NULL;
+ int ret = -1;
+ const char *in_str1 = "Hello";
+ int in_int32[] = {
+ 100000000, 2000000000, -2000000000
+ };
+ const char *in_strv1[] = {
+ "Fishfood",
+ };
+ const char *in_strv2[] = {
+ "Hello", "World",
+ };
+ int *out_int32 = NULL;
+ size_t out_nint32 = 0;
+ char **out_strv1 = NULL;
+ char **out_strv2 = NULL;
+ size_t out_nstrv1 = 0;
+ size_t out_nstrv2 = 0;
+ const char *in_str2 = "World";
+ char *out_str1 = NULL, *out_str2 = NULL;
+
+ if (!(msg = dbus_message_new_method_call("org.libvirt.test",
+ "/org/libvirt/test",
+ "org.libvirt.test.astrochicken",
+ "cluck"))) {
+ VIR_DEBUG("Failed to allocate method call");
+ goto cleanup;
+ }
+
+ if (virDBusMessageEncode(msg,
+ "sa&sa&ia&ss",
+ in_str1,
+ 1, in_strv1,
+ 3, in_int32,
+ 2, in_strv2,
+ in_str2) < 0) {
+ VIR_DEBUG("Failed to encode arguments");
+ goto cleanup;
+ }
+
+ if (virDBusMessageDecode(msg,
+ "sa&sa&ia&ss",
+ &out_str1,
+ &out_nstrv1, &out_strv1,
+ &out_nint32, &out_int32,
+ &out_nstrv2, &out_strv2,
+ &out_str2) < 0) {
+ VIR_DEBUG("Failed to decode arguments");
+ goto cleanup;
+ }
+
+
+ VERIFY_STR("str1", in_str1, out_str1, "%s");
+ if (out_nstrv1 != 1) {
+ fprintf(stderr, "Expected 1 string, but got %zu\n",
+ out_nstrv1);
+ goto cleanup;
+ }
+ VERIFY_STR("strv1[0]", in_strv1[0], out_strv1[0], "%s");
+
+ if (out_nint32 != 3) {
+ fprintf(stderr, "Expected 3 integers, but got %zu\n",
+ out_nint32);
+ goto cleanup;
+ }
+ VERIFY("int32a", in_int32[0], out_int32[0], "%d");
+ VERIFY("int32b", in_int32[1], out_int32[1], "%d");
+ VERIFY("int32c", in_int32[2], out_int32[2], "%d");
+
+ if (out_nstrv2 != 2) {
+ fprintf(stderr, "Expected 2 strings, but got %zu\n",
+ out_nstrv2);
+ goto cleanup;
+ }
+ VERIFY_STR("strv2[0]", in_strv2[0], out_strv2[0], "%s");
+ VERIFY_STR("strv2[1]", in_strv2[1], out_strv2[1], "%s");
+
+ VERIFY_STR("str2", in_str2, out_str2, "%s");
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(out_int32);
+ VIR_FREE(out_str1);
+ VIR_FREE(out_str2);
+ dbus_message_unref(msg);
+ return ret;
+}
+
static int testMessageStruct(const void *args ATTRIBUTE_UNUSED)
{
DBusMessage *msg = NULL;
@@ -385,6 +476,8 @@ mymain(void)
ret = -1;
if (virtTestRun("Test message array ", testMessageArray, NULL) < 0)
ret = -1;
+ if (virtTestRun("Test message array ref ", testMessageArrayRef, NULL) <
0)
+ ret = -1;
if (virtTestRun("Test message struct ", testMessageStruct, NULL) < 0)
ret = -1;
if (virtTestRun("Test message dict ", testMessageDict, NULL) < 0)
--
1.8.5.3
12:15 p.m.
New subject: [libvirt] [PATCH 5/6] Introduce alternate way to encode/decode arrays in DBus messages
On 20.03.2014 13:28, Daniel P. Berrange wrote:
Currently the DBus helper APIs require the values for an array
to be passed inline in the variadic argument list. This change
introduces support for passing arrays using a pointer to a plain
C array of the basic type. This is of particular benefit for
decoding messages when you don't know how many array elements
are being received.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/util/virdbus.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++------
tests/virdbustest.c | 93 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 201 insertions(+), 13 deletions(-)
Nice, we have a test too!
ACK
Michal
12:18 p.m.
New subject: [libvirt] [PATCH 5/6] Introduce alternate way to encode/decode arrays in DBus messages
On Thu, Mar 20, 2014 at 06:15:48PM +0100, Michal Privoznik wrote:
On 20.03.2014 13:28, Daniel P. Berrange wrote:
>Currently the DBus helper APIs require the values for an array
>to be passed inline in the variadic argument list. This change
>introduces support for passing arrays using a pointer to a plain
>C array of the basic type. This is of particular benefit for
>decoding messages when you don't know how many array elements
>are being received.
>
>Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
>---
> src/util/virdbus.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++------
> tests/virdbustest.c | 93 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 201 insertions(+), 13 deletions(-)
Nice, we have a test too!
Mostly because I learnt the hard way that it is damn near impossible
to get this DBus code right without a test suite for it :-)
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
7:28 a.m.
New subject: [libvirt] [PATCH 6/6] Allow caller to handle DBus error messages
The caller may not want all DBus error conditions to be turned
into libvirt errors, so provide a way for the caller to get
back the full DBusError object. They can then check the errors
and only report those that they consider to be fatal.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/util/virdbus.c | 46 ++++++++++++++++++++++++++++++++++++----------
src/util/virdbus.h | 5 ++++-
src/util/virsystemd.c | 2 ++
3 files changed, 42 insertions(+), 11 deletions(-)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index d208646..1eaac23 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1386,6 +1386,7 @@ int virDBusCreateReply(DBusMessage **reply,
* @conn: a DBus connection
* @call: pointer to a message to send
* @replyout: pointer to receive reply message, or NULL
+ * @error: pointer to receive error message
*
* This invokes a method encoded in @call on a remote
* service on the DBus bus @conn. The optional @replyout
@@ -1393,31 +1394,43 @@ int virDBusCreateReply(DBusMessage **reply,
* call. The virDBusMethodReply method can be used to
* decode the return values.
*
+ * If @error is NULL then a libvirt error will be raised
+ * when a DBus error is received and the return value will
+ * be -1. If @error is non-NULL then any DBus error will
+ * be saved into that object and the return value will
+ * be 0.
+ *
* Returns 0 on success, or -1 upon error
*/
int virDBusCall(DBusConnection *conn,
DBusMessage *call,
- DBusMessage **replyout)
+ DBusMessage **replyout,
+ DBusError *error)
{
DBusMessage *reply = NULL;
- DBusError error;
+ DBusError localerror;
int ret = -1;
- dbus_error_init(&error);
+ if (!error)
+ dbus_error_init(&localerror);
if (!(reply = dbus_connection_send_with_reply_and_block(conn,
call,
VIR_DBUS_METHOD_CALL_TIMEOUT_MILLIS,
- &error))) {
- virReportDBusServiceError(error.message ? error.message : "unknown
error",
- error.name);
+ error ? error :
&localerror))) {
+ if (error)
+ ret = 0;
+ else
+ virReportDBusServiceError(localerror.message ? localerror.message :
"unknown error",
+ localerror.name);
goto cleanup;
}
ret = 0;
cleanup:
- dbus_error_free(&error);
+ if (!error)
+ dbus_error_free(&localerror);
if (reply) {
if (ret == 0 && replyout)
*replyout = reply;
@@ -1452,10 +1465,20 @@ int virDBusCall(DBusConnection *conn,
* as variadic args. See virDBusCreateMethodV for a
* description of this parameter.
*
- * Returns: 0 on success, -1 on error
+ *
+ * If @error is NULL then a libvirt error will be raised
+ * when a DBus error is received and the return value will
+ * be -1. If @error is non-NULL then any DBus error will
+ * be saved into that object and the return value will
+ * be 0. If an error occurs while encoding method args
+ * the return value will always be -1 regardless of whether
+ * @error is set.
+ *
+ * Returns 0 on success, or -1 upon error
*/
int virDBusCallMethod(DBusConnection *conn,
DBusMessage **replyout,
+ DBusError *error,
const char *destination,
const char *path,
const char *iface,
@@ -1475,7 +1498,7 @@ int virDBusCallMethod(DBusConnection *conn,
ret = -1;
- ret = virDBusCall(conn, call, replyout);
+ ret = virDBusCall(conn, call, replyout, error);
cleanup:
if (call)
@@ -1525,6 +1548,7 @@ static int virDBusIsServiceInList(const char *listMethod, const char
*name)
if (virDBusCallMethod(conn,
&reply,
+ NULL,
"org.freedesktop.DBus",
"/org/freedesktop/DBus",
"org.freedesktop.DBus",
@@ -1648,7 +1672,8 @@ int virDBusCreateMethodV(DBusMessage **call ATTRIBUTE_UNUSED,
int virDBusCall(DBusConnection *conn ATTRIBUTE_UNUSED,
DBusMessage *call ATTRIBUTE_UNUSED,
- DBusMessage **reply ATTRIBUTE_UNUSED)
+ DBusMessage **reply ATTRIBUTE_UNUSED,
+ DBusError *error ATTRIBUTE_UNUSED)
{
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("DBus support not compiled into this
binary"));
@@ -1657,6 +1682,7 @@ int virDBusCall(DBusConnection *conn ATTRIBUTE_UNUSED,
int virDBusCallMethod(DBusConnection *conn ATTRIBUTE_UNUSED,
DBusMessage **reply ATTRIBUTE_UNUSED,
+ DBusError *error ATTRIBUTE_UNUSED,
const char *destination ATTRIBUTE_UNUSED,
const char *path ATTRIBUTE_UNUSED,
const char *iface ATTRIBUTE_UNUSED,
diff --git a/src/util/virdbus.h b/src/util/virdbus.h
index 4fbda87..0f21821 100644
--- a/src/util/virdbus.h
+++ b/src/util/virdbus.h
@@ -28,6 +28,7 @@
# else
# define DBusConnection void
# define DBusMessage void
+# define DBusError void
# endif
# include "internal.h"
@@ -61,6 +62,7 @@ int virDBusCreateReplyV(DBusMessage **reply,
int virDBusCallMethod(DBusConnection *conn,
DBusMessage **reply,
+ DBusError *error,
const char *destination,
const char *path,
const char *iface,
@@ -68,7 +70,8 @@ int virDBusCallMethod(DBusConnection *conn,
const char *types, ...);
int virDBusCall(DBusConnection *conn,
DBusMessage *call,
- DBusMessage **reply);
+ DBusMessage **reply,
+ DBusError *error);
int virDBusMessageRead(DBusMessage *msg,
const char *types, ...);
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index f2eeb8c..0281158 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -236,6 +236,7 @@ int virSystemdCreateMachine(const char *name,
VIR_DEBUG("Attempting to create machine via systemd");
if (virDBusCallMethod(conn,
NULL,
+ NULL,
"org.freedesktop.machine1",
"/org/freedesktop/machine1",
"org.freedesktop.machine1.Manager",
@@ -301,6 +302,7 @@ int virSystemdTerminateMachine(const char *name,
VIR_DEBUG("Attempting to terminate machine via systemd");
if (virDBusCallMethod(conn,
NULL,
+ NULL,
"org.freedesktop.machine1",
"/org/freedesktop/machine1",
"org.freedesktop.machine1.Manager",
--
1.8.5.3
12:15 p.m.
New subject: [libvirt] [PATCH 6/6] Allow caller to handle DBus error messages
On 20.03.2014 13:28, Daniel P. Berrange wrote:
The caller may not want all DBus error conditions to be turned
into libvirt errors, so provide a way for the caller to get
back the full DBusError object. They can then check the errors
and only report those that they consider to be fatal.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/util/virdbus.c | 46 ++++++++++++++++++++++++++++++++++++----------
src/util/virdbus.h | 5 ++++-
src/util/virsystemd.c | 2 ++
3 files changed, 42 insertions(+), 11 deletions(-)
ACK
Michal
3929
days inactive
3930
days old
18 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Jiri Denemark -
Michal Privoznik