[libvirt] [PATCH] build: Embed dbus_message_unref in virdbus

[Martin Kletzander]

On Mon, Apr 14, 2014 at 01:02:02PM +0100, Daniel P. Berrange wrote:
>On Mon, Apr 14, 2014 at 01:41:18PM +0200, Martin Kletzander wrote:
>> Adding dbus_message_unref() into virDBusMessageDecodeArgs() makes sure
>> that the message gets unref'd, thus making one more pure dbus call not
>> necessary.  Even though we are calling a lot of dbus_* functions
>> outside virdbus (which should be fixed in the future IMHO), this patch
>> fixes only this one instance because it merely aims to fix a
>> build-breaker caused by improperly included dbus.h.  The message
>> printed when failing (using --without-dbus) is:
>
>
>> diff --git a/src/util/virdbus.c b/src/util/virdbus.c
>> index 0cd3858..aef1d34 100644
>> --- a/src/util/virdbus.c
>> +++ b/src/util/virdbus.c
>> @@ -1112,6 +1112,7 @@ int virDBusMessageDecodeArgs(DBusMessage* msg,
>>      }
>>
>>      ret = virDBusMessageIterDecode(&iter, types, args);
>> +    dbus_message_unref(msg);
>>
>>   cleanup:
>>      return ret;
>
>NACK, this is basically reverting the change I did previously and
>will break the firewall patches I have pending.
>

OK, this makes sense if we do one of the following:

 a) Implement a virDBusMessageUnref() which does the same thing if we
    have dbus, which would be no-op otherwise, thus making pure dbus_*
    calls completely wrapped such uses or

 b) encapsulate systemd checks for pm-utils in ifdef
    WITH_SYSTEMD_DAEMON like this:

diff --git i/src/util/virnodesuspend.c w/src/util/virnodesuspend.c
index 59b84ef..20b85b8 100644
--- i/src/util/virnodesuspend.c
+++ w/src/util/virnodesuspend.c
@@ -1,6 +1,7 @@
 /*
  * virnodesuspend.c: Support for suspending a node (host machine)
  *
+ * Copyright (C) 2014 Red Hat, Inc.
  * Copyright (C) 2011 Srivatsa S. Bhat <srivatsa.bhat at linux.vnet.ibm.com>
  *
  * This library is free software; you can redistribute it and/or
@@ -269,6 +270,7 @@ virNodeSuspendSupportsTargetPMUtils(unsigned int target, bool *supported)
 }
 #endif

+#ifdef WITH_SYSTEMD_DAEMON
 static int
 virNodeSuspendSupportsTargetSystemd(unsigned int target, bool *supported)
 {
@@ -292,6 +294,7 @@ virNodeSuspendSupportsTargetSystemd(unsigned int target, bool *supported)

     return ret;
 }
+#endif /* WITH_SYSTEMD_DAEMON */

 /**
  * virNodeSuspendSupportsTarget:
@@ -310,14 +313,23 @@ virNodeSuspendSupportsTargetSystemd(unsigned int target, bool *supported)
 static int
 virNodeSuspendSupportsTarget(unsigned int target, bool *supported)
 {
-    int ret;
+    int ret = 0;
+
+#ifdef WITH_SYSTEMD_DAEMON
+    if (virNodeSuspendSupportsTargetSystemd(target, supported) == 0)
+        goto cleanup;
+#endif

-    ret = virNodeSuspendSupportsTargetSystemd(target, supported);
 #ifdef WITH_PM_UTILS
-    if (ret < 0)
-        ret = virNodeSuspendSupportsTargetPMUtils(target, supported);
+    if (virNodeSuspendSupportsTargetPMUtils(target, supported) == 0)
+        goto cleanup;
 #endif

+    ret = -1;
+    virReportSystemError(VIR_ERR_INTERNAL_ERROR, "%s",
+                         _("Unable to get supported suspend types"));
+
+ cleanup:
     return ret;
 }

diff --git i/src/util/virsystemd.c w/src/util/virsystemd.c
index e9ca564..d953f8a 100644
--- i/src/util/virsystemd.c
+++ w/src/util/virsystemd.c
@@ -1,7 +1,7 @@
 /*
  * virsystemd.c: helpers for using systemd APIs
  *
- * Copyright (C) 2013 Red Hat, Inc.
+ * Copyright (C) 2013, 2014 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -326,6 +326,7 @@ virSystemdNotifyStartup(void)
 #endif
 }

+#ifdef WITH_SYSTEMD_DAEMON
 static int
 virSystemdPMSupportTarget(const char *methodName, bool *result)
 {
@@ -370,6 +371,17 @@ virSystemdPMSupportTarget(const char *methodName, bool *result)
     return ret;
 }

+#else /* ! WITH_SYSTEMD_DAEMON */
+
+static int
+virSystemdPMSupportTarget(const char *methodName ATTRIBUTE_UNUSED,
+                          bool *result ATTRIBUTE_UNUSED)
+{
+    return -1;
+}
+
+#endif /* ! WITH_SYSTEMD_DAEMON */
+
 int virSystemdCanSuspend(bool *result)
 {
     return virSystemdPMSupportTarget("CanSuspend", result);
--

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140414/a75a19ba/attachment-0001.sig>