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

[Eric Richter]

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?

>> +	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);
>>  	}
>