Re: [PATCH] Revert "dbus: correctly build reply message"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux