[libvirt] [PATCH v4] Introduce --without-pm-utils to get rid of pm-is-supported dependency

Cedric Bosdonnat cbosdonnat at suse.com
Thu Apr 10 15:58:53 UTC 2014


Could anyone have a look at this one?

--
Cedric

On Thu, 2014-04-03 at 14:20 +0200, Cédric Bosdonnat wrote:
> This uses the dbus api of systemd to check the power management
> capabilities of the node.
> ---
>  Diff with v3:
>    * Added unit tests vir virSystemdCan* helpers
>    * Make the default for with-pm-utils depending on dbus and systemd detection
>    * Changed the implementation of the helpers to return true for
>      'yes' and 'challenge' responses, we shouldn't get a 'no' given libvirtd runs
>      as privileged user... but who knows.
> 
>  configure.ac              |  24 +++++++++++
>  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/virsystemdmock.c    |  22 ++++++++++
>  tests/virsystemdtest.c    | 100 +++++++++++++++++++++++++++++++++++++++++++++-
>  8 files changed, 281 insertions(+), 17 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 73efffa..34e5ec2 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,25 @@ fi
>  
>  AM_CONDITIONAL([WITH_PHYP],[test "$with_phyp" = "yes"])
>  
> +dnl
> +dnl Should we build with pm-utils support?
> +dnl
> +set -x
> +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"])
> +set +x
> +
>  dnl virsh libraries
>  VIRSH_LIBS="$VIRSH_LIBS $READLINE_LIBS"
>  AC_SUBST([VIRSH_LIBS])
> @@ -2845,6 +2868,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 eab9b23..5c20955 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}
>  %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 38fbf63..ce51bdf 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1879,6 +1879,9 @@ virSysinfoSetup;
>  
> 
>  # util/virsystemd.h
> +virSystemdCanHibernate;
> +virSystemdCanHybridSleep;
> +virSystemdCanSuspend;
>  virSystemdCreateMachine;
>  virSystemdMakeMachineName;
>  virSystemdMakeScopeName;
> diff --git a/src/util/virnodesuspend.c b/src/util/virnodesuspend.c
> index 8088931..b1eddff 100644
> --- a/src/util/virnodesuspend.c
> +++ b/src/util/virnodesuspend.c
> @@ -22,6 +22,7 @@
>  #include <config.h>
>  #include "virnodesuspend.h"
>  
> +# include "virsystemd.h"
>  #include "vircommand.h"
>  #include "virthread.h"
>  #include "datatypes.h"
> @@ -245,23 +246,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;
> @@ -300,6 +287,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;
> +
> +    *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/virsystemdmock.c b/tests/virsystemdmock.c
> index 23167db..cc385ec 100644
> --- a/tests/virsystemdmock.c
> +++ b/tests/virsystemdmock.c
> @@ -76,10 +76,21 @@ DBusMessage *dbus_connection_send_with_reply_and_block(DBusConnection *connectio
>          } 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, sub;
>          reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN);
>          dbus_message_iter_init_append(reply, &iter);
> @@ -95,11 +106,17 @@ DBusMessage *dbus_connection_send_with_reply_and_block(DBusConnection *connectio
>                                              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, sub;
>          reply = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN);
>          dbus_message_iter_init_append(reply, &iter);
> @@ -115,6 +132,11 @@ DBusMessage *dbus_connection_send_with_reply_and_block(DBusConnection *connectio
>                                              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);
> diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
> index 4fc5137..a9583e7 100644
> --- a/tests/virsystemdtest.c
> +++ b/tests/virsystemdtest.c
> @@ -205,7 +205,6 @@ static int testCreateBadSystemd(const void *opaque ATTRIBUTE_UNUSED)
>      return 0;
>  }
>  
> -
>  struct testScopeData {
>      const char *name;
>      const char *partition;
> @@ -237,6 +236,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:
> +    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)
>  {
> @@ -275,6 +354,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;
>  }
>  





More information about the libvir-list mailing list