[PATCH 3/4] ui/ncurses: Add system config checkbox to enable kexec_file load

[Samuel Mendoza-Jonas]

On Fri, 2017-03-24 at 13:46 -0500, Eric Richter wrote:
> 
> On 03/23/2017 09:22 PM, Samuel Mendoza-Jonas wrote:
> > On Thu, 2017-03-23 at 11:46 -0500, Eric Richter wrote:
> > > This patch adds an option to the system configuration menu that if
> > > checked, enables the use of kexec_file_load.
> > > 
> > > Signed-off-by: Eric Richter <erichte at linux.vnet.ibm.com>
> > > ---
> > >  ui/ncurses/nc-config.c | 24 +++++++++++++++++++++++-
> > >  1 file changed, 23 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/ui/ncurses/nc-config.c b/ui/ncurses/nc-config.c
> > > index 8349629..1e9524b 100644
> > > --- a/ui/ncurses/nc-config.c
> > > +++ b/ui/ncurses/nc-config.c
> > > @@ -33,7 +33,7 @@
> > >  #include "nc-config.h"
> > >  #include "nc-widgets.h"
> > > 
> > > -#define N_FIELDS	48
> > > +#define N_FIELDS	50
> > > 
> > >  extern struct help_text config_help_text;
> > > 
> > > @@ -68,6 +68,7 @@ struct config_screen {
> > >  	bool			autoboot_enabled;
> > >  	bool			ipmi_override;
> > >  	bool			net_override;
> > > +	bool			kexec_method;
> > > 
> > >  	struct {
> > >  		struct nc_widget_label		*autoboot_l;
> > > @@ -82,6 +83,9 @@ struct config_screen {
> > >  		struct nc_widget_label		*timeout_l;
> > >  		struct nc_widget_label		*timeout_help_l;
> > > 
> > > +		struct nc_widget_label		*kexec_method_l;
> > > +		struct nc_widget_checkbox	*kexec_method_cb;
> > > +
> > >  		struct nc_widget_label		*ipmi_type_l;
> > >  		struct nc_widget_label		*ipmi_clear_l;
> > >  		struct nc_widget_checkbox	*ipmi_clear_cb;
> > > @@ -256,6 +260,8 @@ static int screen_process_form(struct config_screen *screen)
> > >  			config->autoboot_timeout_sec = x;
> > >  	}
> > > 
> > > +	config->kexec_method = widget_checkbox_get_value(screen->widgets.kexec_method_cb);
> > > +
> > >  	if (screen->ipmi_override)
> > >  		if (widget_checkbox_get_value(screen->widgets.ipmi_clear_cb))
> > >  			config->ipmi_bootdev = IPMI_BOOTDEV_INVALID;
> > > @@ -490,6 +496,16 @@ static void config_screen_layout_widgets(struct config_screen *screen)
> > >  	} else
> > >  		y += 1;
> > > 
> > > +
> > > +	wl = widget_label_base(screen->widgets.kexec_method_l);
> > > +	widget_set_visible(wl, true);
> > > +	widget_move(wl, y, screen->label_x);
> > > +
> > > +	wf = widget_checkbox_base(screen->widgets.kexec_method_cb);
> > > +	widget_set_visible(wf, true);
> > > +	widget_move(wf, y, screen->field_x);
> > > +	y += 1;
> > > +
> > >  	if (screen->ipmi_override) {
> > >  		wl = widget_label_base(screen->widgets.ipmi_type_l);
> > >  		widget_set_visible(wl, true);
> > > @@ -927,6 +943,11 @@ static void config_screen_setup_widgets(struct config_screen *screen,
> > >  	widget_textbox_set_fixed_size(screen->widgets.timeout_f);
> > >  	widget_textbox_set_validator_integer(screen->widgets.timeout_f, 0, 999);
> > > 
> > > +	screen->widgets.kexec_method_l = widget_new_label(set, 0, 0,
> > > +					_("Use file_load:"));
> > 
> > This probably needs a more descriptive label, I don't think it's clear
> > what "file load" means if you don't already have kexec on you mind.
> > Maybe "Use secure kexec", or something similar?
> > 
> 
> The string "Use file_load:" while unspecific, is exactly the length of 
> space before the widgets are on the right. "Use secure kexec:" gets cut 
> off by the widget, if that stays aligned with the others above/below.
> 
> Would it be preferable to push the checkbox widget out a bit to 
> accommodate a clearer label?

Ah yes, I wasn't thinking about the width limit. Perhaps just
"Secure kexec:" would be sufficient to convey what the option enables
without making the label too wide.

> 
> > > +	screen->widgets.kexec_method_cb = widget_new_checkbox(set, 0, 0,
> > > +					screen->kexec_method);
> > 
> > I think you can just use config->kexec_method directly instead of adding
> > the screen->kexec_method field.
> > 
> > > +
> > >  	if (config->ipmi_bootdev) {
> > >  		label = talloc_asprintf(screen,
> > >  				_("%s IPMI boot option: %s"),
> > > @@ -1170,6 +1191,7 @@ static void config_screen_draw(struct config_screen *screen,
> > >  		config_screen_setup_empty(screen);
> > >  	} else {
> > >  		screen->net_conf_type = find_net_conf_type(config);
> > > +		screen->kexec_method = config->kexec_method;
> > >  		config_screen_setup_widgets(screen, config, sysinfo);
> > >  		config_screen_layout_widgets(screen);
> > >  	}
> 
>