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;
}
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list