ui/ncurses: Reset console options on boot

Message ID 20181107005154.3446-1-sam@mendozajonas.com
State New
Headers show
Series
  • ui/ncurses: Reset console options on boot
Related show

Commit Message

Samuel Mendoza-Jonas Nov. 7, 2018, 12:51 a.m.
The ncurses UI sets a few console options at startup that are needed for
ncurses to work properly. These aren't reset however and can lead to
quirks like the cursor being invisible after kexecing to the next
kernel.
The UI process doesn't have time to reset these when it is killed by
kexec, so instead add a 'boot_active' field to status updates. This is
set by boot.c's update handler so the UI can assume it is about to boot
if it receives a status update with this field, and resets the console
options. If the boot is cancelled for any reason the status update will
reflect that and the console options are restored.

Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
 discover/boot.c               |  7 ++---
 discover/device-handler.c     |  1 +
 lib/pb-protocol/pb-protocol.c |  9 ++++++-
 lib/types/types.h             |  1 +
 ui/ncurses/nc-cui.c           | 49 +++++++++++++++++++++++++++++------
 ui/ncurses/nc-cui.h           |  1 +
 6 files changed, 56 insertions(+), 12 deletions(-)

Comments

Samuel Mendoza-Jonas Nov. 16, 2018, 3:21 a.m. | #1
On Wed, 2018-11-07 at 11:51 +1100, Samuel Mendoza-Jonas wrote:
> The ncurses UI sets a few console options at startup that are needed for
> ncurses to work properly. These aren't reset however and can lead to
> quirks like the cursor being invisible after kexecing to the next
> kernel.
> The UI process doesn't have time to reset these when it is killed by
> kexec, so instead add a 'boot_active' field to status updates. This is
> set by boot.c's update handler so the UI can assume it is about to boot
> if it receives a status update with this field, and resets the console
> options. If the boot is cancelled for any reason the status update will
> reflect that and the console options are restored.
> 
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>

Merged as 2bc0df4a

> ---
>  discover/boot.c               |  7 ++---
>  discover/device-handler.c     |  1 +
>  lib/pb-protocol/pb-protocol.c |  9 ++++++-
>  lib/types/types.h             |  1 +
>  ui/ncurses/nc-cui.c           | 49 +++++++++++++++++++++++++++++------
>  ui/ncurses/nc-cui.h           |  1 +
>  6 files changed, 56 insertions(+), 12 deletions(-)
> 
> diff --git a/discover/boot.c b/discover/boot.c
> index 3de7d28e..ed67cd50 100644
> --- a/discover/boot.c
> +++ b/discover/boot.c
> @@ -46,6 +46,7 @@ static void __attribute__((format(__printf__, 4, 5))) update_status(
>  
>  	status.type = type;
>  	status.backlog = false;
> +	status.boot_active = type == STATUS_INFO;
>  
>  	pb_debug("boot status: [%d] %s\n", type, status.message);
>  
> @@ -531,7 +532,7 @@ struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
>  		image = opt->boot_image->url;
>  	} else {
>  		pb_log_fn("no image specified\n");
> -		update_status(status_fn, status_arg, STATUS_INFO,
> +		update_status(status_fn, status_arg, STATUS_ERROR,
>  				_("Boot failed: no image specified"));
>  		return NULL;
>  	}
> @@ -585,7 +586,7 @@ struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
>  		} else {
>  			pb_log("%s: no command line signature file"
>  				" specified\n", __func__);
> -			update_status(status_fn, status_arg, STATUS_INFO,
> +			update_status(status_fn, status_arg, STATUS_ERROR,
>  					_("Boot failed: no command line"
>  						" signature file specified"));
>  			talloc_free(boot_task);
> @@ -654,7 +655,7 @@ void boot_cancel(struct boot_task *task)
>  {
>  	task->cancelled = true;
>  
> -	update_status(task->status_fn, task->status_arg, STATUS_INFO,
> +	update_status(task->status_fn, task->status_arg, STATUS_ERROR,
>  			_("Boot cancelled"));
>  
>  	cleanup_cancellations(task, NULL);
> diff --git a/discover/device-handler.c b/discover/device-handler.c
> index 983c5090..271b9880 100644
> --- a/discover/device-handler.c
> +++ b/discover/device-handler.c
> @@ -534,6 +534,7 @@ static void _device_handler_vstatus(struct device_handler *handler,
>  	status.type = type;
>  	status.message = talloc_vasprintf(handler, fmt, ap);
>  	status.backlog = false;
> +	status.boot_active = false;
>  
>  	device_handler_status(handler, &status);
>  
> diff --git a/lib/pb-protocol/pb-protocol.c b/lib/pb-protocol/pb-protocol.c
> index 315efc4c..7c563c8e 100644
> --- a/lib/pb-protocol/pb-protocol.c
> +++ b/lib/pb-protocol/pb-protocol.c
> @@ -223,7 +223,7 @@ int pb_protocol_boot_status_len(const struct status *status)
>  	return  4 +	/* type */
>  		4 + optional_strlen(status->message) +
>  		4 +	/* backlog */
> -		4;
> +		4;	/* boot_active */
>  }
>  
>  int pb_protocol_system_info_len(const struct system_info *sysinfo)
> @@ -457,6 +457,9 @@ int pb_protocol_serialise_boot_status(const struct status *status,
>  	*(bool *)pos = __cpu_to_be32(status->backlog);
>  	pos += sizeof(bool);
>  
> +	*(bool *)pos = __cpu_to_be32(status->boot_active);
> +	pos += sizeof(bool);
> +
>  	assert(pos <= buf + buf_len);
>  	(void)buf_len;
>  
> @@ -952,6 +955,10 @@ int pb_protocol_deserialise_boot_status(struct status *status,
>  	status->backlog = *(bool *)pos;
>  	pos += sizeof(status->backlog);
>  
> +	/* boot_active */
> +	status->boot_active = *(bool *)pos;
> +	pos += sizeof(status->boot_active);
> +
>  	rc = 0;
>  
>  out:
> diff --git a/lib/types/types.h b/lib/types/types.h
> index 5f99b58d..f5392c89 100644
> --- a/lib/types/types.h
> +++ b/lib/types/types.h
> @@ -97,6 +97,7 @@ struct status {
>  	} type;
>  	char	*message;
>  	bool	backlog;
> +	bool	boot_active;
>  };
>  
>  struct statuslog_entry {
> diff --git a/ui/ncurses/nc-cui.c b/ui/ncurses/nc-cui.c
> index d3e00aa0..8ad89553 100644
> --- a/ui/ncurses/nc-cui.c
> +++ b/ui/ncurses/nc-cui.c
> @@ -92,16 +92,30 @@ static bool lockdown_active(void)
>  #endif
>  }
>  
> +static void cui_set_curses_options(bool curses_mode)
> +{
> +	if (curses_mode) {
> +		cbreak();			/* Disable line buffering. */
> +		noecho();			/* Disable getch() echo. */
> +		nonl();				/* Disable new-line translation. */
> +		intrflush(stdscr, FALSE);	/* Disable interrupt flush. */
> +		curs_set(0);			/* Make cursor invisible */
> +		nodelay(stdscr, TRUE);		/* Enable non-blocking getch() */
> +	} else {
> +		nocbreak();			/* Enable line buffering. */
> +		echo();				/* Enable getch() echo. */
> +		nl();				/* Enable new-line translation. */
> +		intrflush(stdscr, TRUE);	/* Enable interrupt flush. */
> +		curs_set(1);			/* Make cursor visible */
> +		nodelay(stdscr, FALSE);		/* Disable non-blocking getch() */
> +	}
> +}
> +
>  static void cui_start(void)
>  {
>  	initscr();			/* Initialize ncurses. */
> -	cbreak();			/* Disable line buffering. */
> -	noecho();			/* Disable getch() echo. */
>  	keypad(stdscr, TRUE);		/* Enable num keypad keys. */
> -	nonl();				/* Disable new-line translation. */
> -	intrflush(stdscr, FALSE);	/* Disable interrupt flush. */
> -	curs_set(0);			/* Make cursor invisible */
> -	nodelay(stdscr, TRUE);		/* Enable non-blocking getch() */
> +	cui_set_curses_options(true);
>  
>  	/* We may be operating with an incorrect $TERM type; in this case
>  	 * the keymappings will be slightly broken. We want at least
> @@ -650,6 +664,12 @@ static int cui_process_key(void *arg)
>  			}
>  		}
>  
> +		if (cui->preboot_mode) {
> +			/* Turn curses options back on if the user interacts */
> +			cui->preboot_mode = false;
> +			cui_set_curses_options(true);
> +		}
> +
>  		if (!cui->has_input && key_cancels_boot(c)) {
>  			cui->has_input = true;
>  			if (cui->client) {
> @@ -980,8 +1000,21 @@ static void cui_update_status(struct status *status, void *arg)
>  	statuslog_append_steal(cui, cui->statuslog, status);
>  
>  	/* Ignore status messages from the backlog */
> -	if (!status->backlog)
> -		nc_scr_status_printf(cui->current, "%s", status->message);
> +	if (status->backlog)
> +		return;
> +
> +	nc_scr_status_printf(cui->current, "%s", status->message);
> +
> +	if (cui->preboot_mode &&
> +		(!status->boot_active || status->type == STATUS_ERROR)) {
> +		cui_set_curses_options(true);
> +		cui->preboot_mode = false;
> +	} else {
> +		cui->preboot_mode = status->boot_active &&
> +						status->type == STATUS_INFO;
> +		if (cui->preboot_mode)
> +			cui_set_curses_options(false);
> +	}
>  }
>  
>  /*
> diff --git a/ui/ncurses/nc-cui.h b/ui/ncurses/nc-cui.h
> index d26883b1..abe4db98 100644
> --- a/ui/ncurses/nc-cui.h
> +++ b/ui/ncurses/nc-cui.h
> @@ -77,6 +77,7 @@ struct cui {
>  	void *platform_info;
>  	unsigned int default_item;
>  	int (*on_boot)(struct cui *cui, struct cui_opt_data *cod);
> +	bool preboot_mode;
>  };
>  
>  struct cui *cui_init(void* platform_info,

Patch

diff --git a/discover/boot.c b/discover/boot.c
index 3de7d28e..ed67cd50 100644
--- a/discover/boot.c
+++ b/discover/boot.c
@@ -46,6 +46,7 @@  static void __attribute__((format(__printf__, 4, 5))) update_status(
 
 	status.type = type;
 	status.backlog = false;
+	status.boot_active = type == STATUS_INFO;
 
 	pb_debug("boot status: [%d] %s\n", type, status.message);
 
@@ -531,7 +532,7 @@  struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
 		image = opt->boot_image->url;
 	} else {
 		pb_log_fn("no image specified\n");
-		update_status(status_fn, status_arg, STATUS_INFO,
+		update_status(status_fn, status_arg, STATUS_ERROR,
 				_("Boot failed: no image specified"));
 		return NULL;
 	}
@@ -585,7 +586,7 @@  struct boot_task *boot(void *ctx, struct discover_boot_option *opt,
 		} else {
 			pb_log("%s: no command line signature file"
 				" specified\n", __func__);
-			update_status(status_fn, status_arg, STATUS_INFO,
+			update_status(status_fn, status_arg, STATUS_ERROR,
 					_("Boot failed: no command line"
 						" signature file specified"));
 			talloc_free(boot_task);
@@ -654,7 +655,7 @@  void boot_cancel(struct boot_task *task)
 {
 	task->cancelled = true;
 
-	update_status(task->status_fn, task->status_arg, STATUS_INFO,
+	update_status(task->status_fn, task->status_arg, STATUS_ERROR,
 			_("Boot cancelled"));
 
 	cleanup_cancellations(task, NULL);
diff --git a/discover/device-handler.c b/discover/device-handler.c
index 983c5090..271b9880 100644
--- a/discover/device-handler.c
+++ b/discover/device-handler.c
@@ -534,6 +534,7 @@  static void _device_handler_vstatus(struct device_handler *handler,
 	status.type = type;
 	status.message = talloc_vasprintf(handler, fmt, ap);
 	status.backlog = false;
+	status.boot_active = false;
 
 	device_handler_status(handler, &status);
 
diff --git a/lib/pb-protocol/pb-protocol.c b/lib/pb-protocol/pb-protocol.c
index 315efc4c..7c563c8e 100644
--- a/lib/pb-protocol/pb-protocol.c
+++ b/lib/pb-protocol/pb-protocol.c
@@ -223,7 +223,7 @@  int pb_protocol_boot_status_len(const struct status *status)
 	return  4 +	/* type */
 		4 + optional_strlen(status->message) +
 		4 +	/* backlog */
-		4;
+		4;	/* boot_active */
 }
 
 int pb_protocol_system_info_len(const struct system_info *sysinfo)
@@ -457,6 +457,9 @@  int pb_protocol_serialise_boot_status(const struct status *status,
 	*(bool *)pos = __cpu_to_be32(status->backlog);
 	pos += sizeof(bool);
 
+	*(bool *)pos = __cpu_to_be32(status->boot_active);
+	pos += sizeof(bool);
+
 	assert(pos <= buf + buf_len);
 	(void)buf_len;
 
@@ -952,6 +955,10 @@  int pb_protocol_deserialise_boot_status(struct status *status,
 	status->backlog = *(bool *)pos;
 	pos += sizeof(status->backlog);
 
+	/* boot_active */
+	status->boot_active = *(bool *)pos;
+	pos += sizeof(status->boot_active);
+
 	rc = 0;
 
 out:
diff --git a/lib/types/types.h b/lib/types/types.h
index 5f99b58d..f5392c89 100644
--- a/lib/types/types.h
+++ b/lib/types/types.h
@@ -97,6 +97,7 @@  struct status {
 	} type;
 	char	*message;
 	bool	backlog;
+	bool	boot_active;
 };
 
 struct statuslog_entry {
diff --git a/ui/ncurses/nc-cui.c b/ui/ncurses/nc-cui.c
index d3e00aa0..8ad89553 100644
--- a/ui/ncurses/nc-cui.c
+++ b/ui/ncurses/nc-cui.c
@@ -92,16 +92,30 @@  static bool lockdown_active(void)
 #endif
 }
 
+static void cui_set_curses_options(bool curses_mode)
+{
+	if (curses_mode) {
+		cbreak();			/* Disable line buffering. */
+		noecho();			/* Disable getch() echo. */
+		nonl();				/* Disable new-line translation. */
+		intrflush(stdscr, FALSE);	/* Disable interrupt flush. */
+		curs_set(0);			/* Make cursor invisible */
+		nodelay(stdscr, TRUE);		/* Enable non-blocking getch() */
+	} else {
+		nocbreak();			/* Enable line buffering. */
+		echo();				/* Enable getch() echo. */
+		nl();				/* Enable new-line translation. */
+		intrflush(stdscr, TRUE);	/* Enable interrupt flush. */
+		curs_set(1);			/* Make cursor visible */
+		nodelay(stdscr, FALSE);		/* Disable non-blocking getch() */
+	}
+}
+
 static void cui_start(void)
 {
 	initscr();			/* Initialize ncurses. */
-	cbreak();			/* Disable line buffering. */
-	noecho();			/* Disable getch() echo. */
 	keypad(stdscr, TRUE);		/* Enable num keypad keys. */
-	nonl();				/* Disable new-line translation. */
-	intrflush(stdscr, FALSE);	/* Disable interrupt flush. */
-	curs_set(0);			/* Make cursor invisible */
-	nodelay(stdscr, TRUE);		/* Enable non-blocking getch() */
+	cui_set_curses_options(true);
 
 	/* We may be operating with an incorrect $TERM type; in this case
 	 * the keymappings will be slightly broken. We want at least
@@ -650,6 +664,12 @@  static int cui_process_key(void *arg)
 			}
 		}
 
+		if (cui->preboot_mode) {
+			/* Turn curses options back on if the user interacts */
+			cui->preboot_mode = false;
+			cui_set_curses_options(true);
+		}
+
 		if (!cui->has_input && key_cancels_boot(c)) {
 			cui->has_input = true;
 			if (cui->client) {
@@ -980,8 +1000,21 @@  static void cui_update_status(struct status *status, void *arg)
 	statuslog_append_steal(cui, cui->statuslog, status);
 
 	/* Ignore status messages from the backlog */
-	if (!status->backlog)
-		nc_scr_status_printf(cui->current, "%s", status->message);
+	if (status->backlog)
+		return;
+
+	nc_scr_status_printf(cui->current, "%s", status->message);
+
+	if (cui->preboot_mode &&
+		(!status->boot_active || status->type == STATUS_ERROR)) {
+		cui_set_curses_options(true);
+		cui->preboot_mode = false;
+	} else {
+		cui->preboot_mode = status->boot_active &&
+						status->type == STATUS_INFO;
+		if (cui->preboot_mode)
+			cui_set_curses_options(false);
+	}
 }
 
 /*
diff --git a/ui/ncurses/nc-cui.h b/ui/ncurses/nc-cui.h
index d26883b1..abe4db98 100644
--- a/ui/ncurses/nc-cui.h
+++ b/ui/ncurses/nc-cui.h
@@ -77,6 +77,7 @@  struct cui {
 	void *platform_info;
 	unsigned int default_item;
 	int (*on_boot)(struct cui *cui, struct cui_opt_data *cod);
+	bool preboot_mode;
 };
 
 struct cui *cui_init(void* platform_info,