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@xxxxxxxxxxxxxxxxxx>
*
* 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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list