ui/ncurses: Spin child to ensure autoboot cancelled on exit
diff mbox

Message ID 20161123034523.4324-1-sam@mendozajonas.com
State Accepted
Headers show

Commit Message

Samuel Mendoza-Jonas Nov. 23, 2016, 3:45 a.m. UTC
If the client is not connected to the server instance when exiting, fork
and have the child process spin until the server is available and can be
told to cancel autoboot. This prevents the scenario of a user exiting
the UI and having the server continue to autoboot while they are using
the command line.

Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
 ui/ncurses/nc-cui.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

Comments

Jeremy Kerr Nov. 23, 2016, 3:52 a.m. UTC | #1
Hi Sam,

In the title, did you mean "Spin child" or "Spawn child"?

> If the client is not connected to the server instance when exiting, fork
> and have the child process spin until the server is available and can be
> told to cancel autoboot. This prevents the scenario of a user exiting
> the UI and having the server continue to autoboot while they are using
> the command line.

Looks good, and solves a long-standing nit.

One minor thing (below), but regardless:

Acked-by: Jeremy Kerr <jk@ozlabs.org>


> @@ -1104,5 +1138,14 @@ int cui_run(struct cui *cui)
>  
>  	cui_atexit();
>  
> +	if (!cui->client) {
> +		/* Fork a child to tell the server to cancel autoboot */
> +		pid = fork();
> +		if (!pid)
> +			cui_server_wait_on_exit(cui);
> +		if (pid < 0)
> +			pb_log("Failed to fork child on exit: %m\n");
> +	}
> +

Can we move the exit() to here, rather than cui_server_wait_on_exit(),
so that it's super obvious that the forked child cannot continue
executing cui_run?

Cheers,


Jeremy
Samuel Mendoza-Jonas Nov. 23, 2016, 3:54 a.m. UTC | #2
On Wed, 2016-11-23 at 11:52 +0800, Jeremy Kerr wrote:
> Hi Sam,
> 
> In the title, did you mean "Spin child" or "Spawn child"?

Spin, but Spawn also works.. I'll stare at that bikeshed for a bit :)

> 
> > If the client is not connected to the server instance when exiting, fork
> > and have the child process spin until the server is available and can be
> > told to cancel autoboot. This prevents the scenario of a user exiting
> > the UI and having the server continue to autoboot while they are using
> > the command line.
> 
> Looks good, and solves a long-standing nit.
> 
> One minor thing (below), but regardless:
> 
> Acked-by: Jeremy Kerr <jk@ozlabs.org>
> 
> 
> > @@ -1104,5 +1138,14 @@ int cui_run(struct cui *cui)
> >  
> >  	cui_atexit();
> >  
> > +	if (!cui->client) {
> > +		/* Fork a child to tell the server to cancel autoboot */
> > +		pid = fork();
> > +		if (!pid)
> > +			cui_server_wait_on_exit(cui);
> > +		if (pid < 0)
> > +			pb_log("Failed to fork child on exit: %m\n");
> > +	}
> > +
> 
> Can we move the exit() to here, rather than cui_server_wait_on_exit(),
> so that it's super obvious that the forked child cannot continue
> executing cui_run?

Ah good point, will do.

> 
> Cheers,
> 
> 
> Jeremy
> _______________________________________________
> Petitboot mailing list
> Petitboot@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/petitboot

Patch
diff mbox

diff --git a/ui/ncurses/nc-cui.c b/ui/ncurses/nc-cui.c
index c2f1c83..e0b6fce 100644
--- a/ui/ncurses/nc-cui.c
+++ b/ui/ncurses/nc-cui.c
@@ -46,6 +46,8 @@ 
 
 extern const struct help_text main_menu_help_text;
 
+static bool cui_detached = false;
+
 static struct pmenu *main_menu_init(struct cui *cui);
 
 static bool lockdown_active(void)
@@ -100,6 +102,9 @@  static void cui_start(void)
 
 static void cui_atexit(void)
 {
+	if (cui_detached)
+		return;
+
 	clear();
 	refresh();
 	endwin();
@@ -924,6 +929,33 @@  static struct discover_client_ops cui_client_ops = {
 	.update_config = cui_update_config,
 };
 
+/* cui_server_wait_on_exit - On exit spin until the server is available.
+ *
+ * If the program exits before connecting to the server autoboot won't be
+ * cancelled even though there has been keyboard activity. This function is
+ * called by a child process which will spin until the server is connected and
+ * told to cancel autoboot.
+ *
+ * Processes exiting from this function will not carry out the cui_atexit()
+ * steps.
+ */
+static void cui_server_wait_on_exit(struct cui *cui)
+{
+	cui_detached = true;
+
+	while (!cui->client) {
+		cui->client = discover_client_init(cui->waitset,
+				&cui_client_ops, cui);
+		if (!cui->client)
+			sleep(1);
+	}
+
+	talloc_steal(cui, cui->client);
+	discover_client_cancel_default(cui->client);
+
+	exit(EXIT_SUCCESS);
+}
+
 /* cui_server_wait - Connect to the discover server.
  * @arg: Pointer to the cui instance.
  *
@@ -1078,6 +1110,8 @@  fail_alloc:
 
 int cui_run(struct cui *cui)
 {
+	pid_t pid;
+
 	assert(main);
 
 	cui->current = &cui->main->scr;
@@ -1104,5 +1138,14 @@  int cui_run(struct cui *cui)
 
 	cui_atexit();
 
+	if (!cui->client) {
+		/* Fork a child to tell the server to cancel autoboot */
+		pid = fork();
+		if (!pid)
+			cui_server_wait_on_exit(cui);
+		if (pid < 0)
+			pb_log("Failed to fork child on exit: %m\n");
+	}
+
 	return cui->abort ? 0 : -1;
 }