diff mbox series

[3/5] discover/handler: Implement temporary autoboot messages

Message ID 20180703063447.8338-4-jk@ozlabs.org
State Accepted
Headers show
Series Implement a set of hotkeys for temporary autoboot override | expand

Commit Message

Jeremy Kerr July 3, 2018, 6:34 a.m. UTC
Handle incoming requests for temporary autoboot settings.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
---
 discover/device-handler.c  | 70 +++++++++++++++++++++++++++++++++++++++-------
 discover/device-handler.h  |  2 ++
 discover/discover-server.c | 16 +++++++++++
 3 files changed, 78 insertions(+), 10 deletions(-)

Comments

samjonas July 9, 2018, 4:35 a.m. UTC | #1
On Tue, 2018-07-03 at 16:34 +1000, Jeremy Kerr wrote:
> Handle incoming requests for temporary autoboot settings.
> 
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
> ---
>  discover/device-handler.c  | 70 +++++++++++++++++++++++++++++++++++++++-------
>  discover/device-handler.h  |  2 ++
>  discover/discover-server.c | 16 +++++++++++
>  3 files changed, 78 insertions(+), 10 deletions(-)
> 
> diff --git a/discover/device-handler.c b/discover/device-handler.c
> index 7d8b53c..3d75e57 100644
> --- a/discover/device-handler.c
> +++ b/discover/device-handler.c
> @@ -43,8 +43,9 @@
>  #include "ipmi.h"
>  
>  enum default_priority {
> -	DEFAULT_PRIORITY_REMOTE		= 1,
> -	DEFAULT_PRIORITY_LOCAL_FIRST	= 2,
> +	DEFAULT_PRIORITY_TEMP_USER	= 1,
> +	DEFAULT_PRIORITY_REMOTE		= 2,
> +	DEFAULT_PRIORITY_LOCAL_FIRST	= 3,
>  	DEFAULT_PRIORITY_LOCAL_LAST	= 0xfe,
>  	DEFAULT_PRIORITY_DISABLED	= 0xff,
>  };
> @@ -75,6 +76,7 @@ struct device_handler {
>  	struct waiter		*timeout_waiter;
>  	bool			autoboot_enabled;
>  	unsigned int		sec_to_boot;
> +	struct autoboot_option	*temp_autoboot;
>  
>  	struct discover_boot_option *default_boot_option;
>  	int			default_boot_option_priority;
> @@ -815,19 +817,30 @@ static int autoboot_option_priority(const struct config *config,
>   * for these options.
>   */
>  static enum default_priority default_option_priority(
> +		struct device_handler *handler,
>  		struct discover_boot_option *opt)
>  {
>  	const struct config *config;
>  
> +	/* Temporary user-provided autoboot options have highest priority */
> +	if (handler->temp_autoboot) {
> +		if (autoboot_option_matches(handler->temp_autoboot,
> +					opt->device))
> +			return DEFAULT_PRIORITY_TEMP_USER;
> +
> +		pb_debug("handler: disabled default priority due to "
> +				"temporary user override\n");
> +		return DEFAULT_PRIORITY_DISABLED;
> +	}
> +
>  	config = config_get();
>  
> -	/* We give highest priority to IPMI-configured boot options. If
> -	 * we have an IPMI bootdev configuration set, then we don't allow
> -	 * any other defaults */
> -	if (config->ipmi_bootdev) {
> -		bool ipmi_match = ipmi_device_type_matches(config->ipmi_bootdev,
> -				opt->device->device->type);
> -		if (ipmi_match)
> +	/* Next highest priority to IPMI-configured boot options. If we have an
> +	 * IPMI bootdev configuration set, then we don't allow any other
> +	 * defaults */
> +	if (config->ipmi_bootdev) { bool ipmi_match =
> +		ipmi_device_type_matches(config->ipmi_bootdev,
> +				opt->device->device->type); if (ipmi_match)
>  			return DEFAULT_PRIORITY_REMOTE;
>  
>  		pb_debug("handler: disabled default priority due to "
> @@ -863,7 +876,7 @@ static void set_default(struct device_handler *handler,
>  
>  	pb_debug("handler: new default option: %s\n", opt->option->id);
>  
> -	new_prio = default_option_priority(opt);
> +	new_prio = default_option_priority(handler, opt);
>  
>  	/* Anything outside our range prevents a default boot */
>  	if (new_prio >= DEFAULT_PRIORITY_DISABLED)
> @@ -903,6 +916,43 @@ static void set_default(struct device_handler *handler,
>  	default_timeout(handler);
>  }
>  
> +void device_handler_apply_temp_autoboot(struct device_handler *handler,
> +		struct autoboot_option *opt)
> +{
> +	unsigned int i;
> +
> +	handler->temp_autoboot = talloc_steal(handler, opt);
> +
> +	if (!handler->autoboot_enabled)
> +		return;
> +
> +	if (!handler->default_boot_option)
> +		return;

Hi Jeremy,

If I'm reading this correctly, applying a temporary autoboot override is
a one-time event in that it checks the currently available boot options
and sets a default if one matches the device type. But this won't be
applied to newly found boot options right? So a new boot option that is
higher in the boot order could come along and supersede what is set here?

Cheers,
Sam

> +
> +	if (autoboot_option_matches(opt, handler->default_boot_option->device))
> +		return;
> +
> +	/* cancel the default, and rescan available options */
> +	device_handler_cancel_default(handler);
> +
> +	handler->autoboot_enabled = true;
> +
> +	for (i = 0; i < handler->n_devices; i++) {
> +		struct discover_device *dev = handler->devices[i];
> +		struct discover_boot_option *boot_opt;
> +
> +		if (!autoboot_option_matches(opt, dev))
> +			continue;
> +
> +		list_for_each_entry(&dev->boot_options, boot_opt, list) {
> +			if (boot_opt->option->is_default) {
> +				set_default(handler, boot_opt);
> +				break;
> +			}
> +		}
> +	}
> +}
> +
>  static bool resource_is_resolved(struct resource *res)
>  {
>  	return !res || res->resolved;
> diff --git a/discover/device-handler.h b/discover/device-handler.h
> index 771cd06..b215663 100644
> --- a/discover/device-handler.h
> +++ b/discover/device-handler.h
> @@ -163,6 +163,8 @@ void device_handler_process_url(struct device_handler *handler,
>  void device_handler_install_plugin(struct device_handler *handler,
>  		const char *plugin_file);
>  void device_handler_reinit(struct device_handler *handler);
> +void device_handler_apply_temp_autoboot(struct device_handler *handler,
> +		struct autoboot_option *opt);
>  
>  int device_request_write(struct discover_device *dev, bool *release);
>  void device_release_write(struct discover_device *dev, bool release);
> diff --git a/discover/discover-server.c b/discover/discover-server.c
> index 814053d..3377fa6 100644
> --- a/discover/discover-server.c
> +++ b/discover/discover-server.c
> @@ -247,6 +247,7 @@ static int write_config_message(struct discover_server *server,
>  
>  static int discover_server_process_message(void *arg)
>  {
> +	struct autoboot_option *autoboot_opt;
>  	struct pb_protocol_message *message;
>  	struct boot_command *boot_command;
>  	struct client *client = arg;
> @@ -311,6 +312,21 @@ static int discover_server_process_message(void *arg)
>  		device_handler_install_plugin(client->server->device_handler,
>  				url);
>  		break;
> +
> +	case PB_PROTOCOL_ACTION_TEMP_AUTOBOOT:
> +		autoboot_opt = talloc_zero(client, struct autoboot_option);
> +		rc = pb_protocol_deserialise_temp_autoboot(autoboot_opt,
> +				message);
> +		if (rc) {
> +			pb_log("can't parse temporary autoboot message\n");
> +			return 0;
> +		}
> +
> +		device_handler_apply_temp_autoboot(
> +				client->server->device_handler,
> +				autoboot_opt);
> +		break;
> +
>  	default:
>  		pb_log("%s: invalid action %d\n", __func__, message->action);
>  		return 0;
Jeremy Kerr July 10, 2018, 2:18 a.m. UTC | #2
Hi Sam,

> If I'm reading this correctly, applying a temporary autoboot override is
> a one-time event in that it checks the currently available boot options
> and sets a default if one matches the device type. But this won't be
> applied to newly found boot options right?

We apply to new options too. By setting handler->temp_autoboot (and the
additions to default_option_priority), we enforce the temporary autoboot
option on newly discovered options.

[That's really the core part of the patch; going through the existing
options to re-find a default is needed too, but there's a bit more code
involved with that]

If you like, I can add some comments to better explain this.

Cheers,


Jeremy
samjonas July 10, 2018, 3:55 a.m. UTC | #3
On Tue, 2018-07-10 at 10:18 +0800, Jeremy Kerr wrote:
> Hi Sam,
> 
> > If I'm reading this correctly, applying a temporary autoboot override is
> > a one-time event in that it checks the currently available boot options
> > and sets a default if one matches the device type. But this won't be
> > applied to newly found boot options right?
> 
> We apply to new options too. By setting handler->temp_autoboot (and the
> additions to default_option_priority), we enforce the temporary autoboot
> option on newly discovered options.
> 
> [That's really the core part of the patch; going through the existing
> options to re-find a default is needed too, but there's a bit more code
> involved with that]
> 
> If you like, I can add some comments to better explain this.

Aha, my eyes skipped over the 

> +     handler->temp_autoboot = talloc_steal(handler, opt);

and associated bits.

Don't think it needs a new comment, I just fixed up some wonky indenting
around if (config->ipmi_bootdev) and merged as 99a1f9. Thanks!

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

Patch

diff --git a/discover/device-handler.c b/discover/device-handler.c
index 7d8b53c..3d75e57 100644
--- a/discover/device-handler.c
+++ b/discover/device-handler.c
@@ -43,8 +43,9 @@ 
 #include "ipmi.h"
 
 enum default_priority {
-	DEFAULT_PRIORITY_REMOTE		= 1,
-	DEFAULT_PRIORITY_LOCAL_FIRST	= 2,
+	DEFAULT_PRIORITY_TEMP_USER	= 1,
+	DEFAULT_PRIORITY_REMOTE		= 2,
+	DEFAULT_PRIORITY_LOCAL_FIRST	= 3,
 	DEFAULT_PRIORITY_LOCAL_LAST	= 0xfe,
 	DEFAULT_PRIORITY_DISABLED	= 0xff,
 };
@@ -75,6 +76,7 @@  struct device_handler {
 	struct waiter		*timeout_waiter;
 	bool			autoboot_enabled;
 	unsigned int		sec_to_boot;
+	struct autoboot_option	*temp_autoboot;
 
 	struct discover_boot_option *default_boot_option;
 	int			default_boot_option_priority;
@@ -815,19 +817,30 @@  static int autoboot_option_priority(const struct config *config,
  * for these options.
  */
 static enum default_priority default_option_priority(
+		struct device_handler *handler,
 		struct discover_boot_option *opt)
 {
 	const struct config *config;
 
+	/* Temporary user-provided autoboot options have highest priority */
+	if (handler->temp_autoboot) {
+		if (autoboot_option_matches(handler->temp_autoboot,
+					opt->device))
+			return DEFAULT_PRIORITY_TEMP_USER;
+
+		pb_debug("handler: disabled default priority due to "
+				"temporary user override\n");
+		return DEFAULT_PRIORITY_DISABLED;
+	}
+
 	config = config_get();
 
-	/* We give highest priority to IPMI-configured boot options. If
-	 * we have an IPMI bootdev configuration set, then we don't allow
-	 * any other defaults */
-	if (config->ipmi_bootdev) {
-		bool ipmi_match = ipmi_device_type_matches(config->ipmi_bootdev,
-				opt->device->device->type);
-		if (ipmi_match)
+	/* Next highest priority to IPMI-configured boot options. If we have an
+	 * IPMI bootdev configuration set, then we don't allow any other
+	 * defaults */
+	if (config->ipmi_bootdev) { bool ipmi_match =
+		ipmi_device_type_matches(config->ipmi_bootdev,
+				opt->device->device->type); if (ipmi_match)
 			return DEFAULT_PRIORITY_REMOTE;
 
 		pb_debug("handler: disabled default priority due to "
@@ -863,7 +876,7 @@  static void set_default(struct device_handler *handler,
 
 	pb_debug("handler: new default option: %s\n", opt->option->id);
 
-	new_prio = default_option_priority(opt);
+	new_prio = default_option_priority(handler, opt);
 
 	/* Anything outside our range prevents a default boot */
 	if (new_prio >= DEFAULT_PRIORITY_DISABLED)
@@ -903,6 +916,43 @@  static void set_default(struct device_handler *handler,
 	default_timeout(handler);
 }
 
+void device_handler_apply_temp_autoboot(struct device_handler *handler,
+		struct autoboot_option *opt)
+{
+	unsigned int i;
+
+	handler->temp_autoboot = talloc_steal(handler, opt);
+
+	if (!handler->autoboot_enabled)
+		return;
+
+	if (!handler->default_boot_option)
+		return;
+
+	if (autoboot_option_matches(opt, handler->default_boot_option->device))
+		return;
+
+	/* cancel the default, and rescan available options */
+	device_handler_cancel_default(handler);
+
+	handler->autoboot_enabled = true;
+
+	for (i = 0; i < handler->n_devices; i++) {
+		struct discover_device *dev = handler->devices[i];
+		struct discover_boot_option *boot_opt;
+
+		if (!autoboot_option_matches(opt, dev))
+			continue;
+
+		list_for_each_entry(&dev->boot_options, boot_opt, list) {
+			if (boot_opt->option->is_default) {
+				set_default(handler, boot_opt);
+				break;
+			}
+		}
+	}
+}
+
 static bool resource_is_resolved(struct resource *res)
 {
 	return !res || res->resolved;
diff --git a/discover/device-handler.h b/discover/device-handler.h
index 771cd06..b215663 100644
--- a/discover/device-handler.h
+++ b/discover/device-handler.h
@@ -163,6 +163,8 @@  void device_handler_process_url(struct device_handler *handler,
 void device_handler_install_plugin(struct device_handler *handler,
 		const char *plugin_file);
 void device_handler_reinit(struct device_handler *handler);
+void device_handler_apply_temp_autoboot(struct device_handler *handler,
+		struct autoboot_option *opt);
 
 int device_request_write(struct discover_device *dev, bool *release);
 void device_release_write(struct discover_device *dev, bool release);
diff --git a/discover/discover-server.c b/discover/discover-server.c
index 814053d..3377fa6 100644
--- a/discover/discover-server.c
+++ b/discover/discover-server.c
@@ -247,6 +247,7 @@  static int write_config_message(struct discover_server *server,
 
 static int discover_server_process_message(void *arg)
 {
+	struct autoboot_option *autoboot_opt;
 	struct pb_protocol_message *message;
 	struct boot_command *boot_command;
 	struct client *client = arg;
@@ -311,6 +312,21 @@  static int discover_server_process_message(void *arg)
 		device_handler_install_plugin(client->server->device_handler,
 				url);
 		break;
+
+	case PB_PROTOCOL_ACTION_TEMP_AUTOBOOT:
+		autoboot_opt = talloc_zero(client, struct autoboot_option);
+		rc = pb_protocol_deserialise_temp_autoboot(autoboot_opt,
+				message);
+		if (rc) {
+			pb_log("can't parse temporary autoboot message\n");
+			return 0;
+		}
+
+		device_handler_apply_temp_autoboot(
+				client->server->device_handler,
+				autoboot_opt);
+		break;
+
 	default:
 		pb_log("%s: invalid action %d\n", __func__, message->action);
 		return 0;