Hi On Fri, Sep 6, 2019 at 7:37 PM Michal Privoznik <mprivozn@redhat.com> wrote:

This reverts commit 39dded7bb61444bb608fadd3f82f6fe93d08fd0e.

This commit broke virpolkittest on Ubuntu 18 which has an old dbus (v1.12.2). Any other distro with the recent one works (v1.12.16) which hints its a bug in dbus somewhere. Revert the commit to stop tickling it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

This patch is no longer necessary for dbus-vmstate in this series. I found this "bug" when implementing an alternative solution. So reverting it is fine by me. However, it may be revealing a transient bug in virDBusMessageIterDecode() rather than libdbus. It would be worth to have VIR_DEBUG() output from the failing test. For now: Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

--- src/util/virdbus.c | 18 ++++++------------ src/util/virdbus.h | 6 ++---- tests/virfirewalltest.c | 9 +++------ tests/virpolkittest.c | 3 +-- 4 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/src/util/virdbus.c b/src/util/virdbus.c index 64513eef14..b0ac8d7055 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -1456,7 +1456,6 @@ int virDBusCreateMethod(DBusMessage **call,

/** * virDBusCreateReplyV: - * @msg: the message to reply to * @reply: pointer to be filled with a method reply message * @types: type signature for following method arguments * @args: method arguments @@ -1469,14 +1468,13 @@ int virDBusCreateMethod(DBusMessage **call, * as variadic args. See virDBusCreateMethodV for a * description of this parameter. */ -int virDBusCreateReplyV(DBusMessage *msg, - DBusMessage **reply, +int virDBusCreateReplyV(DBusMessage **reply, const char *types, va_list args) { int ret = -1;

- if (!(*reply = dbus_message_new_method_return(msg))) { + if (!(*reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN))) { virReportOOMError(); goto cleanup; } @@ -1495,7 +1493,6 @@ int virDBusCreateReplyV(DBusMessage *msg,

/** * virDBusCreateReply: - * @msg: the message to reply to * @reply: pointer to be filled with a method reply message * @types: type signature for following method arguments * @...: method arguments @@ -1503,15 +1500,14 @@ int virDBusCreateReplyV(DBusMessage *msg, * See virDBusCreateReplyV for a description of the * behaviour of this method. */ -int virDBusCreateReply(DBusMessage *msg, - DBusMessage **reply, +int virDBusCreateReply(DBusMessage **reply, const char *types, ...) { va_list args; int ret;

va_start(args, types); - ret = virDBusCreateReplyV(msg, reply, types, args); + ret = virDBusCreateReplyV(reply, types, args); va_end(args);

return ret; @@ -1815,8 +1811,7 @@ int virDBusCreateMethodV(DBusMessage **call ATTRIBUTE_UNUSED, return -1; }

-int virDBusCreateReplyV(DBusMessage *msg ATTRIBUTE_UNUSED, - DBusMessage **reply ATTRIBUTE_UNUSED, +int virDBusCreateReplyV(DBusMessage **reply ATTRIBUTE_UNUSED, const char *types ATTRIBUTE_UNUSED, va_list args ATTRIBUTE_UNUSED) { @@ -1825,8 +1820,7 @@ int virDBusCreateReplyV(DBusMessage *msg ATTRIBUTE_UNUSED, return -1; }

-int virDBusCreateReply(DBusMessage *msg ATTRIBUTE_UNUSED, - DBusMessage **reply ATTRIBUTE_UNUSED, +int virDBusCreateReply(DBusMessage **reply ATTRIBUTE_UNUSED, const char *types ATTRIBUTE_UNUSED, ...) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virdbus.h b/src/util/virdbus.h index 0303e91045..083c074d59 100644 --- a/src/util/virdbus.h +++ b/src/util/virdbus.h @@ -52,11 +52,9 @@ int virDBusCreateMethodV(DBusMessage **call, const char *member, const char *types, va_list args); -int virDBusCreateReply(DBusMessage *msg, - DBusMessage **reply, +int virDBusCreateReply(DBusMessage **reply, const char *types, ...); -int virDBusCreateReplyV(DBusMessage *msg, - DBusMessage **reply, +int virDBusCreateReplyV(DBusMessage **reply, const char *types, va_list args);

diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c index e5eeb52175..78685a3bf4 100644 --- a/tests/virfirewalltest.c +++ b/tests/virfirewalltest.c @@ -150,8 +150,7 @@ VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block, if (nargs == 1 && STREQ(type, "ipv4") && STREQ(args[0], "-L")) { - if (virDBusCreateReply(message, - &reply, + if (virDBusCreateReply(&reply, "s", TEST_FILTER_TABLE_LIST) < 0) goto error; } else if (nargs == 3 && @@ -159,13 +158,11 @@ VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block, STREQ(args[0], "-t") && STREQ(args[1], "nat") && STREQ(args[2], "-L")) { - if (virDBusCreateReply(message, - &reply, + if (virDBusCreateReply(&reply, "s", TEST_NAT_TABLE_LIST) < 0) goto error; } else { - if (virDBusCreateReply(message, - &reply, + if (virDBusCreateReply(&reply, "s", "success") < 0) goto error; } diff --git a/tests/virpolkittest.c b/tests/virpolkittest.c index 845ceb1736..ce1ff92bf2 100644 --- a/tests/virpolkittest.c +++ b/tests/virpolkittest.c @@ -123,8 +123,7 @@ VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block, VIR_FREE(cancellationId); virStringListFreeCount(details, detailslen);

- if (virDBusCreateReply(message, - &reply, + if (virDBusCreateReply(&reply, "(bba&{ss})", is_authorized, is_challenge, -- 2.21.0