Skip to content

Commit e77a88f

Browse files
committed
daemon: Make actually initiating reboot asynchronous
Alternative to coreos#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: coreos#1348
1 parent 57250d1 commit e77a88f

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
@@ -542,7 +542,7 @@ rollback_transaction_execute (RpmostreedTransaction *transaction,
542542
}
543543

544544
if (self->reboot)
545-
rpmostreed_reboot (cancellable, error);
545+
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
546546

547547
return TRUE;
548548
}
@@ -1511,7 +1511,7 @@ deploy_transaction_execute (RpmostreedTransaction *transaction,
15111511
}
15121512

15131513
if (deploy_has_bool_option (self, "reboot"))
1514-
rpmostreed_reboot (cancellable, error);
1514+
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
15151515
}
15161516
else
15171517
{
@@ -1913,7 +1913,7 @@ initramfs_etc_transaction_execute (RpmostreedTransaction *transaction,
19131913
return FALSE;
19141914

19151915
if (vardict_lookup_bool (self->options, "reboot", FALSE))
1916-
rpmostreed_reboot (cancellable, error);
1916+
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
19171917

19181918
return TRUE;
19191919
}
@@ -2051,7 +2051,7 @@ initramfs_state_transaction_execute (RpmostreedTransaction *transaction,
20512051
return FALSE;
20522052

20532053
if (vardict_lookup_bool (self->options, "reboot", FALSE))
2054-
rpmostreed_reboot (cancellable, error);
2054+
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
20552055

20562056
return TRUE;
20572057
}
@@ -2610,7 +2610,7 @@ finalize_deployment_transaction_execute (RpmostreedTransaction *transaction,
26102610
(void) rpmostree_syscore_bump_mtime (sysroot, NULL);
26112611

26122612
sd_journal_print (LOG_INFO, "Finalized deployment; rebooting into %s", checksum);
2613-
rpmostreed_reboot (cancellable, error);
2613+
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
26142614
return TRUE;
26152615
}
26162616

@@ -2787,7 +2787,7 @@ kernel_arg_transaction_execute (RpmostreedTransaction *transaction,
27872787
return FALSE;
27882788

27892789
if (vardict_lookup_bool (self->options, "reboot", FALSE))
2790-
rpmostreed_reboot (cancellable, error);
2790+
rpmostreed_daemon_reboot (rpmostreed_daemon_get ());
27912791

27922792
return TRUE;
27932793
}

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)