Hi
On Fri, Sep 6, 2019 at 7:37 PM Michal Privoznik <mprivozn@xxxxxxxxxx> 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@xxxxxxxxxx>
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@xxxxxxxxxx>
> ---
> 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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
