Skip to content

Commit 9244d7a

Browse files
committed
daemon: Make actually initiating reboot asynchronous
Alternative to #2845 which moved the `reboot` invocation into the client (which I think still makes sense in some cases). Previously we were starting the reboot before we're returned a success reply to the client, so it could see the daemon killed by `SIGTERM` and hence emit a spurious error. (Really in this case any 3 of the calling process, the client binary or the daemon could be killed in any order, so it's messy but this just closes one race) For cleanliness we reject starting any new transactions after the daemon has started a reboot. Closes: #1348
1 parent d799cfe commit 9244d7a

6 files changed

+52
-16
lines changed

src/daemon/rpmostreed-daemon.cxx

+42
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ struct _RpmostreedDaemon {
6161
GHashTable *bus_clients; /* <utf8 busname, struct RpmOstreeClient> */
6262

6363
gboolean running;
64+
gboolean rebooting;
6465
GDBusProxy *bus_proxy;
6566
GSource *idle_exit_source;
6667
guint rerender_status_id;
@@ -774,6 +775,47 @@ rpmostreed_daemon_exit_now (RpmostreedDaemon *self)
774775
self->running = FALSE;
775776
}
776777

778+
static gboolean
779+
idle_initiate_reboot (void *_unused)
780+
{
781+
sd_journal_print (LOG_INFO, "Initiating reboot requested from transaction");
782+
783+
/* Note that we synchronously spawn this command, but the command just queues the request and returns.
784+
*/
785+
const char *child_argv[] = { "systemctl", "reboot", NULL };
786+
g_autoptr(GError) local_error = NULL;
787+
if (!g_spawn_sync (NULL, (char**)child_argv, NULL, (GSpawnFlags)(G_SPAWN_CHILD_INHERITS_STDIN | G_SPAWN_SEARCH_PATH),
788+
NULL, NULL, NULL, NULL, NULL, &local_error))
789+
{
790+
sd_journal_print (LOG_WARNING, "Failed to initate reboot: %s", local_error->message);
791+
// And now...not a lot of great choices. We could loop and retry, but exiting will surface the error
792+
// in an obvious way.
793+
exit (1);
794+
}
795+
796+
return FALSE;
797+
}
798+
799+
void
800+
rpmostreed_daemon_reboot (RpmostreedDaemon *self)
801+
{
802+
g_assert (!self->rebooting);
803+
self->rebooting = TRUE;
804+
/* Queue actually starting the reboot until we return to the client, so
805+
* that they get a success message for the transaction. Otherwise
806+
* if the daemon gets killed via SIGTERM they just see the bus connection
807+
* broken and may spuriously error out.
808+
*/
809+
g_idle_add_full (G_PRIORITY_LOW, idle_initiate_reboot, NULL, NULL);
810+
}
811+
812+
gboolean
813+
rpmostreed_daemon_is_rebooting (RpmostreedDaemon *self)
814+
{
815+
return self->rebooting;
816+
}
817+
818+
777819
void
778820
rpmostreed_daemon_run_until_idle_exit (RpmostreedDaemon *self)
779821
{

src/daemon/rpmostreed-daemon.h

+2
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ char * rpmostreed_daemon_client_get_agent_id (RpmostreedDaemon *self
5353
char * rpmostreed_daemon_client_get_sd_unit (RpmostreedDaemon *self,
5454
const char *client);
5555
void rpmostreed_daemon_exit_now (RpmostreedDaemon *self);
56+
void rpmostreed_daemon_reboot (RpmostreedDaemon *self);
57+
gboolean rpmostreed_daemon_is_rebooting (RpmostreedDaemon *self);
5658
void rpmostreed_daemon_run_until_idle_exit (RpmostreedDaemon *self);
5759
void rpmostreed_daemon_publish (RpmostreedDaemon *self,
5860
const gchar *path,

src/daemon/rpmostreed-sysroot.cxx

+2
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,8 @@ rpmostreed_sysroot_prep_for_txn (RpmostreedSysroot *self,
818818
RpmostreedTransaction **out_compat_txn,
819819
GError **error)
820820
{
821+
if (rpmostreed_daemon_is_rebooting (rpmostreed_daemon_get()))
822+
return glnx_throw (error, "Reboot initiated, cannot start new transaction");
821823
if (self->transaction)
822824
{
823825
if (rpmostreed_transaction_is_compatible (self->transaction, invocation))

src/daemon/rpmostreed-transaction-types.cxx

+6-6
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,7 @@ rollback_transaction_execute (RpmostreedTransaction *transaction,
546546
}
547547

548548
if (self->reboot)
549-
rpmostreed_reboot (cancellable, error);
549+
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
550550

551551
return TRUE;
552552
}
@@ -1526,7 +1526,7 @@ deploy_transaction_execute (RpmostreedTransaction *transaction,
15261526
}
15271527

15281528
if (deploy_has_bool_option (self, "reboot"))
1529-
rpmostreed_reboot (cancellable, error);
1529+
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
15301530
}
15311531
else
15321532
{
@@ -1928,7 +1928,7 @@ initramfs_etc_transaction_execute (RpmostreedTransaction *transaction,
19281928
return FALSE;
19291929

19301930
if (vardict_lookup_bool (self->options, "reboot", FALSE))
1931-
rpmostreed_reboot (cancellable, error);
1931+
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
19321932

19331933
return TRUE;
19341934
}
@@ -2066,7 +2066,7 @@ initramfs_state_transaction_execute (RpmostreedTransaction *transaction,
20662066
return FALSE;
20672067

20682068
if (vardict_lookup_bool (self->options, "reboot", FALSE))
2069-
rpmostreed_reboot (cancellable, error);
2069+
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
20702070

20712071
return TRUE;
20722072
}
@@ -2625,7 +2625,7 @@ finalize_deployment_transaction_execute (RpmostreedTransaction *transaction,
26252625
(void) rpmostree_syscore_bump_mtime (sysroot, NULL);
26262626

26272627
sd_journal_print (LOG_INFO, "Finalized deployment; rebooting into %s", checksum);
2628-
rpmostreed_reboot (cancellable, error);
2628+
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
26292629
return TRUE;
26302630
}
26312631

@@ -2802,7 +2802,7 @@ kernel_arg_transaction_execute (RpmostreedTransaction *transaction,
28022802
return FALSE;
28032803

28042804
if (vardict_lookup_bool (self->options, "reboot", FALSE))
2805-
rpmostreed_reboot (cancellable, error);
2805+
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
28062806

28072807
return TRUE;
28082808
}

src/daemon/rpmostreed-utils.cxx

-8
Original file line numberDiff line numberDiff line change
@@ -211,14 +211,6 @@ rpmostreed_refspec_parse_partial (const gchar *new_provided_refspec,
211211
return TRUE;
212212
}
213213

214-
void
215-
rpmostreed_reboot (GCancellable *cancellable, GError **error)
216-
{
217-
const char *child_argv[] = { "systemctl", "reboot", NULL };
218-
(void) g_spawn_sync (NULL, (char**)child_argv, NULL, (GSpawnFlags)(G_SPAWN_CHILD_INHERITS_STDIN | G_SPAWN_SEARCH_PATH),
219-
NULL, NULL, NULL, NULL, NULL, NULL);
220-
}
221-
222214
/**
223215
* rpmostreed_repo_pull_ancestry:
224216
* @repo: Repo

src/daemon/rpmostreed-utils.h

-2
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@ gboolean rpmostreed_refspec_parse_partial (const gchar *new_provided_refspec,
3636
const gchar *base_refspec,
3737
gchar **out_refspec,
3838
GError **error);
39-
void
40-
rpmostreed_reboot (GCancellable *cancellable, GError **error);
4139

4240
/* XXX These pull-ancestry and lookup-version functions should eventually
4341
* be integrated into libostree, but it's still a bit premature to do

0 commit comments

Comments
 (0)