Both virSystemdTerminateMachine and virSystemdCreateMachine propagate the error to tell between a non-systemd system and a hard error. In virSystemdGetMachineNameByPID both are treated the same, but an error is ignored by the callers. Split out the checks into a separate function. --- src/util/virsystemd.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 7ec3eee..d7a4e78 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -182,6 +182,20 @@ virSystemdMakeMachineName(const char *drivername, return machinename; } +/* -2 = machine1 is not supported on this machine + * -1 = error + * 0 = machine1 is available + */ +static int +virSystemdHasCreateMachine(void) +{ + int ret; + if ((ret = virDBusIsServiceEnabled("org.freedesktop.machine1")) < 0) + return ret; + + return virDBusIsServiceRegistered("org.freedesktop.systemd1"); +} + char * virSystemdGetMachineNameByPID(pid_t pid) @@ -190,10 +204,7 @@ virSystemdGetMachineNameByPID(pid_t pid) DBusMessage *reply = NULL; char *name = NULL, *object = NULL; - if (virDBusIsServiceEnabled("org.freedesktop.machine1") < 0) - goto cleanup; - - if (virDBusIsServiceRegistered("org.freedesktop.systemd1") < 0) + if (virSystemdHasCreateMachine() < 0) goto cleanup; if (!(conn = virDBusGetSystemBus())) @@ -268,11 +279,7 @@ int virSystemdCreateMachine(const char *name, char *slicename = NULL; static int hasCreateWithNetwork = 1; - ret = virDBusIsServiceEnabled("org.freedesktop.machine1"); - if (ret < 0) - return ret; - - if ((ret = virDBusIsServiceRegistered("org.freedesktop.systemd1")) < 0) + if ((ret = virSystemdHasCreateMachine()) < 0) return ret; if (!(conn = virDBusGetSystemBus())) @@ -434,11 +441,7 @@ int virSystemdTerminateMachine(const char *name) memset(&error, 0, sizeof(error)); - ret = virDBusIsServiceEnabled("org.freedesktop.machine1"); - if (ret < 0) - goto cleanup; - - if ((ret = virDBusIsServiceRegistered("org.freedesktop.systemd1")) < 0) + if ((ret = virSystemdHasCreateMachine()) < 0) goto cleanup; ret = -1; -- 2.10.2

On Thu, Feb 23, 2017 at 12:53 PM +0100, Ján Tomko <jtomko(a)redhat.com&gt; wrote: Would it make sense to you to generalize this function? Because at least the same calls are needed for 'org.freedesktop.login1' (logind) in virsystemd.c:virSystemdPMSupportTarget(). -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

After the system has been booted, it should not change. Cache the return value of virSystemdHasCreateMachine. Allow starting and terminating machines with just one DBus call, instead of three, reducing the chance of the call timing out. Also introduce a small function for resetting the cache to be used in tests. --- src/libvirt_private.syms | 1 + src/util/virsystemd.c | 28 ++++++++++++++++++++++++++-- src/util/virsystemdpriv.h | 33 +++++++++++++++++++++++++++++++++ tests/virsystemdtest.c | 4 ++++ 4 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 src/util/virsystemdpriv.h diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 07a3533..738c3c8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2557,6 +2557,7 @@ virSystemdCanHibernate; virSystemdCanHybridSleep; virSystemdCanSuspend; virSystemdCreateMachine; +virSystemdCreateMachineResetCachedValue; virSystemdGetMachineNameByPID; virSystemdMakeMachineName; virSystemdMakeScopeName; diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index d7a4e78..5982bc4 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -26,6 +26,9 @@ # include <sys/un.h> #endif +#define __VIR_SYSTEMD_PRIV_H_ALLOW__ 1 +#include "virsystemdpriv.h" + #include "virsystemd.h" #include "viratomic.h" #include "virbuffer.h" @@ -182,6 +185,15 @@ virSystemdMakeMachineName(const char *drivername, return machinename; } +static int virSystemdCreateMachineCachedValue = -1; + +/* Reset the cache from tests for testing the underlying dbus calls + * as well */ +void virSystemdCreateMachineResetCachedValue(void) +{ + virSystemdCreateMachineCachedValue = -1; +} + /* -2 = machine1 is not supported on this machine * -1 = error * 0 = machine1 is available @@ -190,10 +202,22 @@ static int virSystemdHasCreateMachine(void) { int ret; - if ((ret = virDBusIsServiceEnabled("org.freedesktop.machine1")) < 0) + int val; + + val = virAtomicIntGet(&virSystemdCreateMachineCachedValue); + if (val != -1) + return val; + + if ((ret = virDBusIsServiceEnabled("org.freedesktop.machine1")) < 0) { + if (ret == -2) + virAtomicIntSet(&virSystemdCreateMachineCachedValue, -2); return ret; + } - return virDBusIsServiceRegistered("org.freedesktop.systemd1"); + if ((ret = virDBusIsServiceRegistered("org.freedesktop.systemd1")) == -1) + return ret; + virAtomicIntSet(&virSystemdCreateMachineCachedValue, ret); + return ret; } diff --git a/src/util/virsystemdpriv.h b/src/util/virsystemdpriv.h new file mode 100644 index 0000000..413bc8b --- /dev/null +++ b/src/util/virsystemdpriv.h @@ -0,0 +1,33 @@ +/* + * virsystemdpriv.h: Functions for testing virSystemd APIs + * + * Copyright (C) 2016 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + */ + +#ifndef __VIR_SYSTEMD_PRIV_H_ALLOW__ +# error "virsystemdpriv.h may only be included by virsystemd.c or test suites" +#endif + +#ifndef __VIR_SYSTEMD_PRIV_H__ +# define __VIR_SYSTEMD_PRIV_H__ + +# include "virsystemd.h" + +void virSystemdCreateMachineResetCachedValue(void); + +#endif /* __VIR_SYSTEMD_PRIV_H__ */ diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c index 177cccc..eca6e84 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -27,6 +27,9 @@ # include <stdlib.h> # include <dbus/dbus.h> +# define __VIR_SYSTEMD_PRIV_H_ALLOW__ 1 +# include "virsystemdpriv.h" + # include "virsystemd.h" # include "virdbus.h" # include "virlog.h" @@ -522,6 +525,7 @@ mymain(void) do { \ if (virTestRun(_name, func, NULL) < 0) \ ret = -1; \ + virSystemdCreateMachineResetCachedValue(); \ } while (0) DO_TEST("Test create container ", testCreateContainer); -- 2.10.2