Cédric Bosdonnat wrote:
This uses the dbus api of systemd to check the power management
capabilities of the node.
---
Difference with v4:
* Removed left-over debug bits
* Rebased
* Cleaned up the spurious white space change
configure.ac | 22 +++++++++
libvirt.spec.in | 9 ++++
src/libvirt_private.syms | 3 ++
src/util/virnodesuspend.c | 75 ++++++++++++++++++++++------
src/util/virsystemd.c | 59 ++++++++++++++++++++++
src/util/virsystemd.h | 6 +++
tests/virsystemdtest.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++
7 files changed, 279 insertions(+), 16 deletions(-)
diff --git a/configure.ac b/configure.ac
index 52c50df..ea85851 100644
--- a/configure.ac
+++ b/configure.ac
@@ -563,6 +563,10 @@ AC_ARG_WITH([chrdev-lock-files],
[location for UUCP style lock files for character devices
(use auto for default paths on some platforms) @<:@default=auto@:>@])])
m4_divert_text([DEFAULTS], [with_chrdev_lock_files=auto])
+AC_ARG_WITH([pm-utils],
+ [AS_HELP_STRING([--with-pm-utils],
+ [use pm-utils for power management @<:@default=yes@:>@])])
+m4_divert_text([DEFAULTS], [with_pm_utils=check])
dnl
dnl in case someone want to build static binaries
@@ -1621,6 +1625,23 @@ fi
AM_CONDITIONAL([WITH_PHYP],[test "$with_phyp" = "yes"])
+dnl
+dnl Should we build with pm-utils support?
+dnl
+if test "$with_pm_utils" = "check"; then
+ with_pm_utils=yes
+ if test "$with_dbus" = "yes"; then
+ if test "$init_systemd" = "yes"; then
+ with_pm_utils=no
+ fi
+ fi
+fi
+
+if test "$with_pm_utils" = "yes"; then
+ AC_DEFINE_UNQUOTED([WITH_PM_UTILS], 1, [whether to use pm-utils])
+fi
+AM_CONDITIONAL([WITH_PM_UTILS], [test "$with_pm_utils" = "yes"])
+
dnl virsh libraries
VIRSH_LIBS="$VIRSH_LIBS $READLINE_LIBS"
AC_SUBST([VIRSH_LIBS])
@@ -2853,6 +2874,7 @@ AC_MSG_NOTICE([ rbd: $LIBRBD_LIBS])
else
AC_MSG_NOTICE([ rbd: no])
fi
+AC_MSG_NOTICE([pm-utils: $with_pm_utils])
AC_MSG_NOTICE([])
AC_MSG_NOTICE([Test suite])
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8e83fb3..b61ef8d 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -132,6 +132,7 @@
%define with_libssh2 0%{!?_without_libssh2:0}
%define with_wireshark 0%{!?_without_wireshark:0}
%define with_systemd_daemon 0%{!?_without_systemd_daemon:0}
+%define with_pm_utils 1
# Non-server/HV driver defaults which are always enabled
%define with_sasl 0%{!?_without_sasl:1}
@@ -182,6 +183,7 @@
%if 0%{?fedora} >= 17 || 0%{?rhel} >= 7
%define with_systemd 1
%define with_systemd_daemon 1
+ %define with_pm_utils 0
%endif
# Fedora 18 / RHEL-7 are first where firewalld support is enabled
@@ -1138,8 +1140,10 @@ Requires: nc
Requires: gettext
# Needed by virt-pki-validate script.
Requires: gnutls-utils
+%if %{with_pm_utils}
# Needed for probing the power management features of the host.
Requires: pm-utils
+%{endif}
%endif
%if %{with_sasl}
Requires: cyrus-sasl
# Not technically required, but makes 'out-of-box' config
@@ -1395,6 +1399,10 @@ driver
%define _without_systemd_daemon --without-systemd-daemon
%endif
+%if ! %{with_pm_utils}
+ %define _without_pm_utils --without-pm-utils
+%endif
+
%define when %(date +"%%F-%%T")
%define where %(hostname)
%define who %{?packager}%{!?packager:Unknown}
@@ -1471,6 +1479,7 @@ rm -f po/stamp-po
%{?_with_firewalld} \
%{?_without_wireshark} \
%{?_without_systemd_daemon} \
+ %{?_without_pm_utils} \
%{with_packager} \
%{with_packager_version} \
--with-qemu-user=%{qemu_user} \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index cd43335..6e807c4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1888,6 +1888,9 @@ virSysinfoSetup;
# util/virsystemd.h
+virSystemdCanHibernate;
+virSystemdCanHybridSleep;
+virSystemdCanSuspend;
virSystemdCreateMachine;
virSystemdMakeMachineName;
virSystemdMakeScopeName;
diff --git a/src/util/virnodesuspend.c b/src/util/virnodesuspend.c
index 08c47aa..70ede2a 100644
--- a/src/util/virnodesuspend.c
+++ b/src/util/virnodesuspend.c
@@ -22,6 +22,7 @@
#include <config.h>
#include "virnodesuspend.h"
+# include "virsystemd.h"
Fails make syntax-check with cppi installed.
#include "vircommand.h"
#include "virthread.h"
#include "datatypes.h"
@@ -228,23 +229,9 @@ int nodeSuspendForDuration(unsigned int target,
return ret;
}
-
-/**
- * virNodeSuspendSupportsTarget:
- * @target: The power management target to check whether it is supported
- * by the host. Values could be:
- * VIR_NODE_SUSPEND_TARGET_MEM
- * VIR_NODE_SUSPEND_TARGET_DISK
- * VIR_NODE_SUSPEND_TARGET_HYBRID
- * @supported: set to true if supported, false otherwise
- *
- * Run the script 'pm-is-supported' (from the pm-utils package)
- * to find out if @target is supported by the host.
- *
- * Returns 0 if the query was successful, -1 on failure.
- */
+#ifdef WITH_PM_UTILS
static int
-virNodeSuspendSupportsTarget(unsigned int target, bool *supported)
+virNodeSuspendSupportsTargetPMUtils(unsigned int target, bool *supported)
{
virCommandPtr cmd;
int status;
@@ -280,6 +267,62 @@ virNodeSuspendSupportsTarget(unsigned int target, bool *supported)
virCommandFree(cmd);
return ret;
}
+#endif
+
+static int
+virNodeSuspendSupportsTargetSystemd(unsigned int target, bool *supported)
+{
+ int ret = -1;
+
+ if (virNodeSuspendInitialize() < 0)
+ return -1;
This can be removed after 3f671e6c.
+
+ *supported = false;
+
+ switch (target) {
+ case VIR_NODE_SUSPEND_TARGET_MEM:
+ ret = virSystemdCanSuspend(supported);
+ break;
+ case VIR_NODE_SUSPEND_TARGET_DISK:
+ ret = virSystemdCanHibernate(supported);
+ break;
+ case VIR_NODE_SUSPEND_TARGET_HYBRID:
+ ret = virSystemdCanHybridSleep(supported);
+ break;
+ default:
+ return ret;
+ }
+
+ return ret;
+}
+
+/**
+ * virNodeSuspendSupportsTarget:
+ * @target: The power management target to check whether it is supported
+ * by the host. Values could be:
+ * VIR_NODE_SUSPEND_TARGET_MEM
+ * VIR_NODE_SUSPEND_TARGET_DISK
+ * VIR_NODE_SUSPEND_TARGET_HYBRID
+ * @supported: set to true if supported, false otherwise
+ *
+ * Run the script 'pm-is-supported' (from the pm-utils package)
+ * to find out if @target is supported by the host.
+ *
+ * Returns 0 if the query was successful, -1 on failure.
+ */
+static int
+virNodeSuspendSupportsTarget(unsigned int target, bool *supported)
+{
+ int ret;
+
+ ret = virNodeSuspendSupportsTargetSystemd(target, supported);
+#ifdef WITH_PM_UTILS
+ if (ret < 0)
+ ret = virNodeSuspendSupportsTargetPMUtils(target, supported);
+#endif
+
+ return ret;
+}
/**
* virNodeSuspendGetTargetMask:
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 93b3f9c..e9ca564 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -325,3 +325,62 @@ virSystemdNotifyStartup(void)
sd_notify(0, "READY=1");
#endif
}
+
+static int
+virSystemdPMSupportTarget(const char *methodName, bool *result)
+{
+ int ret;
+ DBusConnection *conn;
+ DBusMessage *message = NULL;
+ char *response;
+
+ ret = virDBusIsServiceEnabled("org.freedesktop.login1");
+ if (ret < 0)
+ return ret;
+
+ if ((ret = virDBusIsServiceRegistered("org.freedesktop.login1")) < 0)
+ return ret;
+
+ if (!(conn = virDBusGetSystemBus()))
+ return -1;
+
+ ret = -1;
+
+ if (virDBusCallMethod(conn,
+ &message,
+ NULL,
+ "org.freedesktop.login1",
+ "/org/freedesktop/login1",
+ "org.freedesktop.login1.Manager",
+ methodName,
+ NULL) < 0)
+ return ret;
+
+ if ((ret = virDBusMessageRead(message, "s", &response)) < 0)
+ goto cleanup;
+
+ *result = STREQ("yes", response) || STREQ("challenge",
response);
+
+ ret = 0;
+
+ cleanup:
+ dbus_message_unref(message);
+ VIR_FREE(response);
+
+ return ret;
+}
+
+int virSystemdCanSuspend(bool *result)
+{
+ return virSystemdPMSupportTarget("CanSuspend", result);
+}
+
+int virSystemdCanHibernate(bool *result)
+{
+ return virSystemdPMSupportTarget("CanHibernate", result);
+}
+
+int virSystemdCanHybridSleep(bool *result)
+{
+ return virSystemdPMSupportTarget("CanHybridSleep", result);
+}
diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h
index 7fed456..491c9b7 100644
--- a/src/util/virsystemd.h
+++ b/src/util/virsystemd.h
@@ -48,4 +48,10 @@ int virSystemdTerminateMachine(const char *name,
void virSystemdNotifyStartup(void);
+int virSystemdCanSuspend(bool *result);
+
+int virSystemdCanHibernate(bool *result);
+
+int virSystemdCanHybridSleep(bool *result);
+
#endif /* __VIR_SYSTEMD_H__ */
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index 2b014cc..86d717f 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -55,10 +55,21 @@ VIR_MOCK_IMPL_RET_ARGS(dbus_connection_send_with_reply_and_block,
} else {
reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN);
}
+ } else if (STREQ(service, "org.freedesktop.login1")) {
+ char *supported = getenv("RESULT_SUPPORT");
+ DBusMessageIter iter;
+ reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN);
+ dbus_message_iter_init_append(reply, &iter);
+
+ if (!dbus_message_iter_append_basic(&iter,
+ DBUS_TYPE_STRING,
+ &supported))
+ goto error;
} else if (STREQ(service, "org.freedesktop.DBus") &&
STREQ(member, "ListActivatableNames")) {
const char *svc1 = "org.foo.bar.wizz";
const char *svc2 = "org.freedesktop.machine1";
+ const char *svc3 = "org.freedesktop.login1";
DBusMessageIter iter;
DBusMessageIter sub;
reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN);
@@ -75,11 +86,17 @@ VIR_MOCK_IMPL_RET_ARGS(dbus_connection_send_with_reply_and_block,
DBUS_TYPE_STRING,
&svc2))
goto error;
+ if (!getenv("FAIL_NO_SERVICE") &&
+ !dbus_message_iter_append_basic(&sub,
+ DBUS_TYPE_STRING,
+ &svc3))
+ goto error;
dbus_message_iter_close_container(&iter, &sub);
} else if (STREQ(service, "org.freedesktop.DBus") &&
STREQ(member, "ListNames")) {
const char *svc1 = "org.foo.bar.wizz";
const char *svc2 = "org.freedesktop.systemd1";
+ const char *svc3 = "org.freedesktop.login1";
DBusMessageIter iter;
DBusMessageIter sub;
reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN);
@@ -96,6 +113,11 @@ VIR_MOCK_IMPL_RET_ARGS(dbus_connection_send_with_reply_and_block,
DBUS_TYPE_STRING,
&svc2))
goto error;
+ if ((!getenv("FAIL_NO_SERVICE") &&
!getenv("FAIL_NOT_REGISTERED")) &&
+ !dbus_message_iter_append_basic(&sub,
+ DBUS_TYPE_STRING,
+ &svc3))
+ goto error;
dbus_message_iter_close_container(&iter, &sub);
} else {
reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN);
@@ -313,6 +335,86 @@ testScopeName(const void *opaque)
return ret;
}
+typedef int (*virSystemdCanHelper)(bool * result);
+struct testPMSupportData {
+ virSystemdCanHelper tested;
+};
+
+static int testPMSupportHelper(const void *opaque)
+{
+ int rv;
+ bool result;
+ size_t i;
+ const char *results[4] = {"yes", "no", "na",
"challenge"};
+ int expected[4] = {1, 0, 0, 1};
+ const struct testPMSupportData *data = opaque;
+
+ for (i = 0; i < 4; i++) {
+ setenv("RESULT_SUPPORT", results[i], 1);
+ if ((rv = data->tested(&result)) < 0) {
+ fprintf(stderr, "%s", "Unexpected canSuspend error\n");
+ return -1;
+ }
+
+ if (result != expected[i]) {
+ fprintf(stderr, "Unexpected result for answer '%s'\n",
results[i]);
+ goto error;
+ }
+ unsetenv("RESULT_SUPPORT");
+ }
+
+ return 0;
+error:
Whitespace off on the label.
+ unsetenv("RESULT_SUPPORT");
+ return -1;
+}
+
+static int testPMSupportHelperNoSystemd(const void *opaque)
+{
+ int rv;
+ bool result;
+ const struct testPMSupportData *data = opaque;
+
+ setenv("FAIL_NO_SERVICE", "1", 1);
+
+ if ((rv = data->tested(&result)) == 0) {
+ unsetenv("FAIL_NO_SERVICE");
+ fprintf(stderr, "%s", "Unexpected canSuspend success\n");
+ return -1;
+ }
+ unsetenv("FAIL_NO_SERVICE");
+
+ if (rv != -2) {
+ fprintf(stderr, "%s", "Unexpected canSuspend error\n");
+ return -1;
+ }
+
+ return 0;
+}
+
+static int testPMSupportSystemdNotRunning(const void *opaque)
+{
+ int rv;
+ bool result;
+ const struct testPMSupportData *data = opaque;
+
+ setenv("FAIL_NOT_REGISTERED", "1", 1);
+
+ if ((rv = data->tested(&result)) == 0) {
+ unsetenv("FAIL_NOT_REGISTERED");
+ fprintf(stderr, "%s", "Unexpected canSuspend success\n");
+ return -1;
+ }
+ unsetenv("FAIL_NOT_REGISTERED");
+
+ if (rv != -2) {
+ fprintf(stderr, "%s", "Unexpected canSuspend error\n");
+ return -1;
+ }
+
+ return 0;
+}
+
static int
mymain(void)
{
@@ -351,6 +453,25 @@ mymain(void)
TEST_SCOPE("demo", "/machine/eng-dept/testing!stuff",
"machine-eng\\x2ddept-testing\\x21stuff-lxc\\x2ddemo.scope");
+# define TESTS_PM_SUPPORT_HELPER(name, function) \
+ do { \
+ struct testPMSupportData data = { \
+ function \
+ }; \
+ if (virtTestRun("Test " name " ", testPMSupportHelper,
&data) < 0) \
+ ret = -1; \
+ if (virtTestRun("Test " name " no systemd ",
\
+ testPMSupportHelperNoSystemd, &data) < 0) \
+ ret = -1; \
+ if (virtTestRun("Test systemd " name " not running ",
\
+ testPMSupportSystemdNotRunning, &data) < 0) \
+ ret = -1; \
+ } while (0)
+
+ TESTS_PM_SUPPORT_HELPER("canSuspend", &virSystemdCanSuspend);
+ TESTS_PM_SUPPORT_HELPER("canHibernate", &virSystemdCanHibernate);
+ TESTS_PM_SUPPORT_HELPER("canHybridSleep", &virSystemdCanHybridSleep);
+
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
But looks good otherwise. I've tested this on a systemd machine and an
older sysvinit system that uses pm-is-supported. I'll squash in the
below diff and push shortly.
Regards,
Jim
diff --git a/libvirt.spec.in b/libvirt.spec.in
index b61ef8d..4e70a41 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1143,7 +1143,7 @@ Requires: gnutls-utils
%if %{with_pm_utils}
# Needed for probing the power management features of the host.
Requires: pm-utils
-%{endif}
+%endif
%if %{with_sasl}
Requires: cyrus-sasl
# Not technically required, but makes 'out-of-box' config
diff --git a/src/util/virnodesuspend.c b/src/util/virnodesuspend.c
index 70ede2a..59b84ef 100644
--- a/src/util/virnodesuspend.c
+++ b/src/util/virnodesuspend.c
@@ -22,7 +22,7 @@
#include <config.h>
#include "virnodesuspend.h"
-# include "virsystemd.h"
+#include "virsystemd.h"
#include "vircommand.h"
#include "virthread.h"
#include "datatypes.h"
@@ -274,9 +274,6 @@ virNodeSuspendSupportsTargetSystemd(unsigned int
target, bool *supported)
{
int ret = -1;
- if (virNodeSuspendInitialize() < 0)
- return -1;
-
*supported = false;
switch (target) {
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index 86d717f..0fcd4e8 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -364,7 +364,7 @@ static int testPMSupportHelper(const void *opaque)
}
return 0;
-error:
+ error:
unsetenv("RESULT_SUPPORT");
return -1;
}