[libvirt] [PATCH 0/3] virNetDaemonCallInhibit: check for logind availability

* get rid of a pointless error logged when system D-Bus is not available * do not send messages on systems without logind * remove four extra dbus calls on libvirtd startup when checking for logind availability Ján Tomko (3): util: introduce virSystemdHasLogind util: cache the result of whether logind is available rpc: make virNetDaemonCallInhibit a no-op with no logind src/libvirt_private.syms | 2 ++ src/rpc/virnetdaemon.c | 3 +++ src/util/virsystemd.c | 37 ++++++++++++++++++++++++++++++++----- src/util/virsystemd.h | 2 ++ src/util/virsystemdpriv.h | 1 + tests/virsystemdtest.c | 3 +++ 6 files changed, 43 insertions(+), 5 deletions(-) -- 2.19.2

Split it out from virSystemdPMSupportTarget. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virsystemd.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 26b751311f..b401eda6a2 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -174,6 +174,21 @@ virSystemdHasMachined(void) return ret; } +static int +virSystemdHasLogind(void) +{ + int ret; + + ret = virDBusIsServiceEnabled("org.freedesktop.login1"); + if (ret < 0) + return ret; + + if ((ret = virDBusIsServiceRegistered("org.freedesktop.login1")) < 0) + return ret; + + return ret; +} + char * virSystemdGetMachineNameByPID(pid_t pid) @@ -547,11 +562,7 @@ virSystemdPMSupportTarget(const char *methodName, bool *result) DBusMessage *message = NULL; char *response; - ret = virDBusIsServiceEnabled("org.freedesktop.login1"); - if (ret < 0) - return ret; - - if ((ret = virDBusIsServiceRegistered("org.freedesktop.login1")) < 0) + if ((ret = virSystemdHasLogind()) < 0) return ret; if (!(conn = virDBusGetSystemBus())) -- 2.19.2

Similar to how we cache the availability of machined. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virsystemd.c | 20 ++++++++++++++++++-- src/util/virsystemdpriv.h | 1 + tests/virsystemdtest.c | 3 +++ 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7a3feb8efa..5940342087 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3135,6 +3135,7 @@ virSystemdCanSuspend; virSystemdCreateMachine; virSystemdGetActivation; virSystemdGetMachineNameByPID; +virSystemdHasLogindResetCachedValue; virSystemdHasMachinedResetCachedValue; virSystemdMakeScopeName; virSystemdMakeSliceName; diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index b401eda6a2..66d0b7bd53 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -140,6 +140,7 @@ char *virSystemdMakeSliceName(const char *partition) } static int virSystemdHasMachinedCachedValue = -1; +static int virSystemdHasLogindCachedValue = -1; /* Reset the cache from tests for testing the underlying dbus calls * as well */ @@ -148,6 +149,12 @@ void virSystemdHasMachinedResetCachedValue(void) virSystemdHasMachinedCachedValue = -1; } +void virSystemdHasLogindResetCachedValue(void) +{ + virSystemdHasLogindCachedValue = -1; +} + + /* -2 = machine1 is not supported on this machine * -1 = error * 0 = machine1 is available @@ -178,14 +185,23 @@ static int virSystemdHasLogind(void) { int ret; + int val; + + val = virAtomicIntGet(&virSystemdHasLogindCachedValue); + if (val != -1) + return val; ret = virDBusIsServiceEnabled("org.freedesktop.login1"); - if (ret < 0) + if (ret < 0) { + if (ret == -2) + virAtomicIntSet(&virSystemdHasLogindCachedValue, -2); return ret; + } - if ((ret = virDBusIsServiceRegistered("org.freedesktop.login1")) < 0) + if ((ret = virDBusIsServiceRegistered("org.freedesktop.login1")) == -1) return ret; + virAtomicIntSet(&virSystemdHasLogindCachedValue, ret); return ret; } diff --git a/src/util/virsystemdpriv.h b/src/util/virsystemdpriv.h index a49c91e649..736c53d3fa 100644 --- a/src/util/virsystemdpriv.h +++ b/src/util/virsystemdpriv.h @@ -28,3 +28,4 @@ #include "virsystemd.h" void virSystemdHasMachinedResetCachedValue(void); +void virSystemdHasLogindResetCachedValue(void); diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c index 340b038095..2dafce2764 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -751,12 +751,15 @@ mymain(void) }; \ if (virTestRun("Test " name " ", testPMSupportHelper, &data) < 0) \ ret = -1; \ + virSystemdHasLogindResetCachedValue(); \ if (virTestRun("Test " name " no systemd ", \ testPMSupportHelperNoSystemd, &data) < 0) \ ret = -1; \ + virSystemdHasLogindResetCachedValue(); \ if (virTestRun("Test systemd " name " not running ", \ testPMSupportSystemdNotRunning, &data) < 0) \ ret = -1; \ + virSystemdHasLogindResetCachedValue(); \ } while (0) TESTS_PM_SUPPORT_HELPER("canSuspend", &virSystemdCanSuspend); -- 2.19.2

As a side effect, this also silences the possible: internal error: Unable to get DBus system bus connection: Failed to connect to socket /run/dbus/system_bus_socket: No such file or directory error, since we check upfront whether dbus is available. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libvirt_private.syms | 1 + src/rpc/virnetdaemon.c | 3 +++ src/util/virsystemd.c | 2 +- src/util/virsystemd.h | 2 ++ 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5940342087..ff4ddd1c5c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3135,6 +3135,7 @@ virSystemdCanSuspend; virSystemdCreateMachine; virSystemdGetActivation; virSystemdGetMachineNameByPID; +virSystemdHasLogind; virSystemdHasLogindResetCachedValue; virSystemdHasMachinedResetCachedValue; virSystemdMakeScopeName; diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index d098cf4ae9..7b68ddbdf2 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -508,6 +508,9 @@ virNetDaemonCallInhibit(virNetDaemonPtr dmn, VIR_DEBUG("dmn=%p what=%s who=%s why=%s mode=%s", dmn, NULLSTR(what), NULLSTR(who), NULLSTR(why), NULLSTR(mode)); + if (virSystemdHasLogind() < 0) + return; + if (!(systemBus = virDBusGetSystemBus())) return; diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 66d0b7bd53..e75c5b835a 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -181,7 +181,7 @@ virSystemdHasMachined(void) return ret; } -static int +int virSystemdHasLogind(void) { int ret; diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h index 96626f8fff..5f1a4413fe 100644 --- a/src/util/virsystemd.h +++ b/src/util/virsystemd.h @@ -57,6 +57,8 @@ int virSystemdTerminateMachine(const char *name); void virSystemdNotifyStartup(void); +int virSystemdHasLogind(void); + int virSystemdCanSuspend(bool *result); int virSystemdCanHibernate(bool *result); -- 2.19.2

On 8/13/19 5:08 PM, Ján Tomko wrote:
* get rid of a pointless error logged when system D-Bus is not available * do not send messages on systems without logind * remove four extra dbus calls on libvirtd startup when checking for logind availability
Ján Tomko (3): util: introduce virSystemdHasLogind util: cache the result of whether logind is available rpc: make virNetDaemonCallInhibit a no-op with no logind
src/libvirt_private.syms | 2 ++ src/rpc/virnetdaemon.c | 3 +++ src/util/virsystemd.c | 37 ++++++++++++++++++++++++++++++++----- src/util/virsystemd.h | 2 ++ src/util/virsystemdpriv.h | 1 + tests/virsystemdtest.c | 3 +++ 6 files changed, 43 insertions(+), 5 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Ján Tomko
-
Michal Privoznik