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

[Daniel P. Berrangé]

On Mon, Sep 09, 2019 at 02:11:36PM +0100, Daniel P. Berrangé wrote:
> On Mon, Sep 09, 2019 at 03:47:21PM +0400, Marc-André Lureau wrote:
> > Hi
> > 
> > On Fri, Sep 6, 2019 at 7:37 PM Michal Privoznik <mprivozn at 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 at 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.
> 
> It is not really a bug - rather its due to the short cut taken by
> the test suite.
> 
> virpolkittest is mocking the dbus_connection_send_with_reply_and_block
> method.
> 
> This method accepts a DBusMessage input (the MethodCall) and has to
> create  DBusMessage as output (the MethodReply)
> 
> The DBusMessage input has no serial number set, since this is the job
> of the real dbus_connection_send_with_reply_and_block() method impl.
> 
> The dbus_message_new_method_return() impl expects the original message
> to have a valid serial number though.
> 
> Thus when we create the DBusMessage output, we hit an assertion failure
> when calling dbus_message_new_method_return().
> 
> This is in fact why the code intentionally uses
> 
>   dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN)
> 
> in the first place.
> 
> What's strange though is why it ever succeeds on any distro. The
> assertion we're seeing should *always* hit us on every distro.

Oh, I missed that we now have a mock dbus_message_set_reply_serial
method that does nothing.

I wonder if on certain builds the compiler decided to inline that
method when called from dbus_message_new_method_return, so our mock
didn't take effect.  Oddly I can't reproduce it even on Ubuntu 18
using 'make ci-test at ubuntu-18'

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|