On a Thursday in 2024, Jiri Denemark wrote:
To reduce code duplication both function now use a common virSystemdHasService helper.Signed-off-by: Jiri Denemark <[email protected]> --- src/util/virsystemd.c | 76 ++++++++++++++++++++----------------------- 1 file changed, 36 insertions(+), 40 deletions(-) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index cd4de0eef8..21892aca02 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -141,66 +141,62 @@ void virSystemdHasLogindResetCachedValue(void) } -/* -2 = machine1 is not supported on this machine - * -1 = error - * 0 = machine1 is available +/** + * virSystemdHasService: + * + * Check whether a DBus @service is enabled and either the service itself or + * systemd1 service is registered. If @requireSystemd == true, the systemd1 + * service has to be registered even if @service is registered. + * + * Returns + * -2 when service is not supported on this machine + * -1 on error + * 0 when service is available */ -int -virSystemdHasMachined(void) +static int +virSystemdHasService(const char *service, + bool requireSystemd, + int *cached) { int ret; int val; - val = g_atomic_int_get(&virSystemdHasMachinedCachedValue); + val = g_atomic_int_get(cached); if (val != -1) return val; - if ((ret = virGDBusIsServiceEnabled("org.freedesktop.machine1")) < 0) { + if ((ret = virGDBusIsServiceEnabled(service)) < 0) { if (ret == -2) - g_atomic_int_set(&virSystemdHasMachinedCachedValue, -2); + g_atomic_int_set(cached, -2); return ret; } - if ((ret = virGDBusIsServiceRegistered("org.freedesktop.systemd1")) == -1) + if ((ret = virGDBusIsServiceRegistered(service)) == -1) return ret;
From the 'refactor' point of view, this adds the extra check whether
machine1 is also registered. So the bool should rather be
'skipRunningCheck' or something like that.
From a practical point of view, this seems pointless if we discard the result
anyway.
Admittedly, the terminology I added in:
commit 12ee0b98d3ddcb2c71cdb2352512153791ebcb7e
Check if systemd is running before creating machines
is a bit confusing. But if "machine1" is "Enabled" (i.e. in
"ListActivatableNames"), that could have meant that there is systemd
installed but the system was not booted with it.
If it returns -1, the following (unreachable) call would likely return
-1 as well.
If it returns 0, we don't need to check whether systemd1 is registered,
because we already know machined is running.
If it returns -2, we did not really learn anything new.
- g_atomic_int_set(&virSystemdHasMachinedCachedValue, ret);
+
+ if (requireSystemd || ret == -2) {
+ if ((ret = virGDBusIsServiceRegistered("org.freedesktop.systemd1")) ==
-1)
+ return ret;
+ }
+
+ g_atomic_int_set(cached, ret);
return ret;
}
+
int
-virSystemdHasLogind(void)
+virSystemdHasMachined(void)
{
- int ret;
- int val;
-
- val = g_atomic_int_get(&virSystemdHasLogindCachedValue);
- if (val != -1)
- return val;
-
- ret = virGDBusIsServiceEnabled("org.freedesktop.login1");
- if (ret < 0) {
- if (ret == -2)
- g_atomic_int_set(&virSystemdHasLogindCachedValue, -2);
- return ret;
- }
-
- /*
- * Want to use logind if:
- * - logind is already running
- * Or
- * - logind is not running, but this is a systemd host
- * (rely on dbus activation)
- */
- if ((ret = virGDBusIsServiceRegistered("org.freedesktop.login1")) == -1)
- return ret;
Is there a possibility of logind running on a host not booted with systemd? If not, this check is also pointless with the fallback below. (Same question applies to resolved, but is not relevant to this patch) Jano
+ return virSystemdHasService("org.freedesktop.machine1", true,
+ &virSystemdHasMachinedCachedValue);
+}
- if (ret == -2) {
- if ((ret = virGDBusIsServiceRegistered("org.freedesktop.systemd1")) ==
-1)
- return ret;
- }
- g_atomic_int_set(&virSystemdHasLogindCachedValue, ret);
- return ret;
+int
+virSystemdHasLogind(void)
+{
+ return virSystemdHasService("org.freedesktop.login1", false,
+ &virSystemdHasLogindCachedValue);
}
--
2.43.0
_______________________________________________
Devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
signature.asc
Description: PGP signature
_______________________________________________ Devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
