Skip to content

Commit

Permalink
daemon: Respect systemd inhibitor locks
Browse files Browse the repository at this point in the history
Today, non-interactive processes, including rpm-ostree, are not
inhibited by systemd inhibitor locks. systemd 248 added the
`--check-inhibitors` option to `systemctl` commands, which can be
used to make non-interactive proccesses check for shutdown/sleep
inhibitors. However, it also checks for `delay` mode inhibitors,
which isn't what we want. Replicate this logic prior to reboots,
but ignore `delay` mode inhibitors.
  • Loading branch information
kelvinfan001 committed Jun 3, 2021
1 parent 9244d7a commit 8806593
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 10 deletions.
6 changes: 6 additions & 0 deletions src/daemon/rpmostreed-daemon.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,12 @@ rpmostreed_daemon_is_rebooting (RpmostreedDaemon *self)
return self->rebooting;
}

GDBusConnection *
rpmostreed_daemon_connection(void)
{
g_assert (_daemon_instance->connection);
return _daemon_instance->connection;
}

void
rpmostreed_daemon_run_until_idle_exit (RpmostreedDaemon *self)
Expand Down
1 change: 1 addition & 0 deletions src/daemon/rpmostreed-daemon.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ G_BEGIN_DECLS

GType rpmostreed_daemon_get_type (void) G_GNUC_CONST;
RpmostreedDaemon * rpmostreed_daemon_get (void);
GDBusConnection * rpmostreed_daemon_connection (void);
gboolean rpmostreed_get_client_uid (RpmostreedDaemon *self,
const char *client,
uid_t *out_uid);
Expand Down
38 changes: 33 additions & 5 deletions src/daemon/rpmostreed-transaction-types.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,11 @@ rollback_transaction_execute (RpmostreedTransaction *transaction,
}

if (self->reboot)
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
{
if (!check_sd_inhibitor_locks (cancellable, error))
return FALSE;
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
}

return TRUE;
}
Expand Down Expand Up @@ -1526,7 +1530,11 @@ deploy_transaction_execute (RpmostreedTransaction *transaction,
}

if (deploy_has_bool_option (self, "reboot"))
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
{
if (!check_sd_inhibitor_locks (cancellable, error))
return FALSE;
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
}
}
else
{
Expand Down Expand Up @@ -1928,7 +1936,11 @@ initramfs_etc_transaction_execute (RpmostreedTransaction *transaction,
return FALSE;

if (vardict_lookup_bool (self->options, "reboot", FALSE))
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
{
if (!check_sd_inhibitor_locks (cancellable, error))
return FALSE;
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
}

return TRUE;
}
Expand Down Expand Up @@ -2066,7 +2078,11 @@ initramfs_state_transaction_execute (RpmostreedTransaction *transaction,
return FALSE;

if (vardict_lookup_bool (self->options, "reboot", FALSE))
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
{
if (!check_sd_inhibitor_locks (cancellable, error))
return FALSE;
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
}

return TRUE;
}
Expand Down Expand Up @@ -2582,6 +2598,10 @@ finalize_deployment_transaction_execute (RpmostreedTransaction *transaction,
OstreeSysroot *sysroot = rpmostreed_transaction_get_sysroot (transaction);
OstreeRepo *repo = ostree_sysroot_repo (sysroot);

// Check for inhibitor locks before unlocking staged deployment.
if (!check_sd_inhibitor_locks (cancellable, error))
return FALSE;

auto command_line = (const char*)
vardict_lookup_ptr (self->options, "initiating-command-line", "&s");

Expand Down Expand Up @@ -2610,6 +2630,10 @@ finalize_deployment_transaction_execute (RpmostreedTransaction *transaction,
return glnx_throw (error, "Expected staged base checksum %s, but found %s",
expected_checksum, checksum);

// Check for inhibitor locks before unlocking staged deployment.
if (!check_sd_inhibitor_locks (cancellable, error))
return FALSE;

if (unlink (_OSTREE_SYSROOT_RUNSTATE_STAGED_LOCKED) < 0)
{
if (errno != ENOENT)
Expand Down Expand Up @@ -2802,7 +2826,11 @@ kernel_arg_transaction_execute (RpmostreedTransaction *transaction,
return FALSE;

if (vardict_lookup_bool (self->options, "reboot", FALSE))
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
{
if (!check_sd_inhibitor_locks (cancellable, error))
return FALSE;
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
}

return TRUE;
}
Expand Down
50 changes: 50 additions & 0 deletions src/daemon/rpmostreed-utils.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "rpmostree-util.h"
#include "rpmostreed-utils.h"
#include "rpmostreed-errors.h"
#include "rpmostreed-daemon.h"
#include "libglnx.h"
#include <systemd/sd-journal.h>
#include <stdint.h>
Expand Down Expand Up @@ -609,3 +610,52 @@ rpmostreed_parse_revision (const char *revision,
out:
return ret;
}

/* Throw an error if there exists systemd inhibitor locks in `block` mode only.
* Note: systemd 248 provides a `--check-inhibitors` option, but it also checks
* for inhibitors in `delay` mode, which isn't what we want. */
gboolean
check_sd_inhibitor_locks (GCancellable *cancellable,
GError **error)
{
GDBusConnection *connection = rpmostreed_daemon_connection ();
g_autoptr(GVariant) inhibitors_array_tuple =
g_dbus_connection_call_sync (connection, "org.freedesktop.login1", "/org/freedesktop/login1",
"org.freedesktop.login1.Manager", "ListInhibitors",
NULL, (const GVariantType*)"(a(ssssuu))",
G_DBUS_CALL_FLAGS_NONE, -1, cancellable, error);
if (!inhibitors_array_tuple)
return glnx_prefix_error (error, "Checking systemd inhibitor locks");
else if (g_variant_n_children (inhibitors_array_tuple) < 1)
return glnx_throw (error, "LoadInhibitors returned empty tuple");
g_autoptr(GVariant) inhibitors_array =
g_variant_get_child_value (inhibitors_array_tuple, 0);

char *what;
char *who;
char *why;
char *mode;
int num_block_inhibitors = 0;
g_autoptr(GString) error_msg = g_string_new(NULL);
GVariantIter viter;
g_variant_iter_init (&viter, inhibitors_array);
while (g_variant_iter_loop (&viter, "(ssssuu)", &what, &who, &why, &mode, NULL, NULL))
{
// Only consider shutdown inhibitors in `block` mode.
if (strstr (what, "shutdown") && strcmp (mode, "block") == 0)
if (++num_block_inhibitors == 1)
g_string_append_printf (error_msg,
"Reboot blocked by a systemd inhibitor lock in `block` mode\n"
"Held by: %s\nReason: %s",
who, why);
}

if (num_block_inhibitors == 0)
return TRUE;
else if (num_block_inhibitors > 1)
g_string_append_printf (error_msg,
"\nand %d other blocking inhibitor lock(s)\n"
"Use `systemd-inhibit --list` to see details",
num_block_inhibitors - 1);
return glnx_throw (error, "%s", error_msg->str);
}
3 changes: 3 additions & 0 deletions src/daemon/rpmostreed-utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,7 @@ gboolean rpmostreed_parse_revision (const char *revision,
char **out_version,
GError **error);

gboolean check_sd_inhibitor_locks (GCancellable *cancellable,
GError **error);

G_END_DECLS
4 changes: 2 additions & 2 deletions tests/common/libvm.sh
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ EOF

vm_setup

# XXX: hack around https://github.com/systemd/systemd/issues/14328
vm_cmd systemctl mask --now systemd-logind
# # XXX: hack around https://github.com/systemd/systemd/issues/14328
# vm_cmd systemctl mask --now systemd-logind

# Some tests expect the ref to be on `vmcheck`. We should drop that
# requirement, but for now let's just mangle the origin
Expand Down
14 changes: 11 additions & 3 deletions tests/vmcheck/test-misc-2.sh
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ assert_streq "$(vm_get_booted_csum)" "${booted_csum}"
vm_assert_journal_has_content $cursor 'Not finalizing; found /run/ostree/staged-deployment-locked'
echo "ok locked rebase staging"

# This also now tests custom client IDs in the journal.
# This also tests custom client IDs in the journal and interaction with systemd inhibitor locks.
cursor=$(vm_get_journal_cursor)
vm_cmd env RPMOSTREE_CLIENT_ID=testing-agent-id \
rpm-ostree deploy revision="${commit}" \
Expand All @@ -58,7 +58,15 @@ fi
vm_cmd journalctl --after-cursor "'$cursor'" -u rpm-ostreed -o json | jq -r '.AGENT//""' > agent.txt
assert_file_has_content agent.txt testing-agent-id
vm_cmd journalctl --after-cursor "'$cursor'" -u rpm-ostreed -o json | jq -r '.AGENT_SD_UNIT//""' > agent_sd_unit.txt
assert_file_has_content agent_sd_unit.txt sshd.service
assert_file_has_content agent_sd_unit.txt session-1.scope
vm_cmd "systemd-inhibit --what=shutdown --mode=block sh -c 'while ! test -f /run/wakeup; do sleep 0.1; done'" &
if vm_rpmostree finalize-deployment "${commit}" 2>err.txt; then
assert_not_reached "finalized with inhibitor lock in block mode present"
fi
assert_file_has_content err.txt 'Reboot blocked'
vm_cmd test -f /run/ostree/staged-deployment-locked # Make sure that staged deployment is still locked.
vm_cmd touch /run/wakeup
sleep 0.1 # Wait a bit for the process holding the lock to exit.
vm_reboot_cmd rpm-ostree finalize-deployment "${commit}"
assert_streq "$(vm_get_booted_csum)" "${commit}"
vm_assert_journal_has_content $cursor "Finalized deployment; rebooting into ${commit}"
Expand Down Expand Up @@ -364,7 +372,7 @@ vm_cmd rpm-ostree deploy \'\' \
--register-driver=OtherTestDriver --bypass-driver
# Make sure OtherTestDriver's systemd unit will be the same as the commandline's
vm_cmd rpm-ostree status -v > verbose_status.txt
assert_file_has_content verbose_status.txt 'AutomaticUpdatesDriver: OtherTestDriver (sshd.service)'
assert_file_has_content verbose_status.txt 'AutomaticUpdatesDriver: OtherTestDriver (session-1.scope)'
vm_rpmostree upgrade 2>err.txt
assert_not_file_has_content err.txt 'Updates and deployments are driven by OtherTestDriver'
vm_rpmostree cleanup -p
Expand Down

0 comments on commit 8806593

Please sign in to comment.