protocol: Separate device add from boot-option add messages

We want to cater for situations where boot options may be discovered
some time after we get notificiation about devices. For instance,
discovering boot options from DHCP configuration parameters. In this
case, we'll need to notify UIs of boot options appear some time after
the device (and/or other boot options on the same device) has appeared.

This change adds a new protocol message type,
PB_PROTOCOL_ACTION_BOOT_OPTION_ADD. We also rename
PB_PROTOCOL_ACTION_ADD to make it clear that it is just for devices.

The discover server is updated to send boot option add events at device
discover time, but we are now able to decouple this later.

We also update the clients to handle the boot option add events
separately.

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

5 files changed, 141 insertions, 115 deletions

diff --git a/ui/common/discover-client.c b/ui/common/discover-client.c
index c0cfea0..42aa9de 100644
--- a/ui/common/discover-client.c
+++ b/ui/common/discover-client.c
@@ -1,4 +1,5 @@
 
+#include <assert.h>
 #include <errno.h>
 #include <unistd.h>
 #include <stdlib.h>
@@ -37,6 +38,20 @@ void discover_client_destroy(struct discover_client *client)
 	talloc_free(client);
 }
 
+static struct device *find_device(struct discover_client *client,
+		const char *id)
+{
+	int i;
+
+	for (i = 0; i < client->n_devices; i++) {
+		struct device *dev = client->devices[i];
+		if (!strcmp(dev->id, id))
+			return dev;
+	}
+
+	return NULL;
+}
+
 static void device_add(struct discover_client *client, struct device *device)
 {
 	client->n_devices++;
@@ -46,7 +61,25 @@ static void device_add(struct discover_client *client, struct device *device)
 	client->devices[client->n_devices - 1] = device;
 	talloc_steal(client, device);
 
-	client->ops.device_add(device, client->ops.cb_arg);
+	if (client->ops.device_add)
+		client->ops.device_add(device, client->ops.cb_arg);
+}
+
+static void boot_option_add(struct discover_client *client,
+		struct boot_option *opt)
+{
+	struct device *dev;
+
+	dev = find_device(client, opt->device_id);
+
+	/* we require that devices are already present before any boot options
+	 * are added */
+	assert(dev);
+
+	talloc_steal(dev, opt);
+
+	if (client->ops.boot_option_add)
+		client->ops.boot_option_add(dev, opt, client->ops.cb_arg);
 }
 
 static void device_remove(struct discover_client *client, const char *id)
@@ -81,6 +114,7 @@ static int discover_client_process(void *arg)
 {
 	struct discover_client *client = arg;
 	struct pb_protocol_message *message;
+	struct boot_option *opt;
 	struct device *dev;
 	char *dev_id;
 	int rc;
@@ -91,8 +125,9 @@ static int discover_client_process(void *arg)
 		return -1;
 
 	switch (message->action) {
-	case PB_PROTOCOL_ACTION_ADD:
-		dev = talloc(client, struct device);
+	case PB_PROTOCOL_ACTION_DEVICE_ADD:
+		dev = talloc_zero(client, struct device);
+		list_init(&dev->boot_options);
 
 		rc = pb_protocol_deserialise_device(dev, message);
 		if (rc) {
@@ -102,7 +137,18 @@ static int discover_client_process(void *arg)
 
 		device_add(client, dev);
 		break;
-	case PB_PROTOCOL_ACTION_REMOVE:
+	case PB_PROTOCOL_ACTION_BOOT_OPTION_ADD:
+		opt = talloc_zero(client, struct boot_option);
+
+		rc = pb_protocol_deserialise_boot_option(opt, message);
+		if (rc) {
+			pb_log("%s: no boot_option?\n", __func__);
+			return 0;
+		}
+
+		boot_option_add(client, opt);
+		break;
+	case PB_PROTOCOL_ACTION_DEVICE_REMOVE:
 		dev_id = pb_protocol_deserialise_string(client, message);
 		if (!dev_id) {
 			pb_log("%s: no device id?\n", __func__);
diff --git a/ui/common/discover-client.h b/ui/common/discover-client.h
index 6d5d1c4..5317b44 100644
--- a/ui/common/discover-client.h
+++ b/ui/common/discover-client.h
@@ -21,6 +21,8 @@ struct pb_boot_data {
 
 struct discover_client_ops {
 	int (*device_add)(struct device *device, void *arg);
+	int (*boot_option_add)(struct device *dev, struct boot_option *option,
+			void *arg);
 	void (*device_remove)(struct device *device, void *arg);
 	void *cb_arg;
 };
diff --git a/ui/ncurses/nc-cui.c b/ui/ncurses/nc-cui.c
index 2d4dea5..c26819f 100644
--- a/ui/ncurses/nc-cui.c
+++ b/ui/ncurses/nc-cui.c
@@ -142,7 +142,7 @@ static int cui_boot(struct pmenu_item *item)
 
 	def_prog_mode();
 
-	result = discover_client_boot(cui->client, cod->dev, cod->opt, cod->bd);
+	result = discover_client_boot(cui->client, NULL, cod->opt, cod->bd);
 
 	reset_prog_mode();
 	redrawwin(cui->current->main_ncw);
@@ -354,16 +354,18 @@ void cui_on_open(struct pmenu *menu)
  * menu_items into the main menu.  Redraws the main menu if it is active.
  */
 
-static int cui_device_add(struct device *dev, void *arg)
+static int cui_boot_option_add(struct device *dev, struct boot_option *opt,
+		void *arg)
 {
 	struct cui *cui = cui_from_arg(arg);
-	int result;
-	struct boot_option *opt;
-	unsigned int o_count; /* device opts */
+	struct cui_opt_data *cod;
 	unsigned int insert_pt;
+	struct pmenu_item *i;
 	ITEM *selected;
+	char *name;
+	int result;
 
-	pb_log("%s: %p %s\n", __func__, dev, dev->id);
+	pb_log("%s: %p %s\n", __func__, opt, opt->id);
 
 	selected = current_item(cui->main->ncm);
 
@@ -377,57 +379,42 @@ static int cui_device_add(struct device *dev, void *arg)
 	if (result)
 		pb_log("%s: set_menu_items failed: %d\n", __func__, result);
 
-	o_count = 0;
-	list_for_each_entry(&dev->boot_options, opt, list)
-		o_count++;
-
 	/* Insert new items at insert_pt. */
+	insert_pt = pmenu_grow(cui->main, 1);
 
-	insert_pt = pmenu_grow(cui->main, o_count);
-
-	list_for_each_entry(&dev->boot_options, opt, list) {
-		struct pmenu_item *i;
-		struct cui_opt_data *cod;
-		char *name;
-
-		/* Save the item in opt->ui_info for cui_device_remove() */
+	/* Save the item in opt->ui_info for cui_device_remove() */
 
-		opt->ui_info = i = pmenu_item_alloc(cui->main);
+	opt->ui_info = i = pmenu_item_alloc(cui->main);
 
-		i->on_edit = cui_boot_editor_run;
-		i->on_execute = cui_boot;
-		i->data = cod = talloc(i, struct cui_opt_data);
-
-		cod->dev = dev;
-		cod->opt = opt;
-		cod->opt_hash = pb_opt_hash(dev, opt);
-		cod->name = opt->name;
-		cod->bd = talloc(i, struct pb_boot_data);
-
-		cod->bd->image = talloc_strdup(cod->bd, opt->boot_image_file);
-		cod->bd->initrd = talloc_strdup(cod->bd, opt->initrd_file);
-		cod->bd->args = talloc_strdup(cod->bd, opt->boot_args);
-
-		name = cui_make_item_name(i, cod);
-		pmenu_item_setup(cui->main, i, insert_pt, name);
-
-		insert_pt++;
-
-		pb_log("%s: adding opt '%s'\n", __func__, cod->name);
-		pb_log("   image  '%s'\n", cod->bd->image);
-		pb_log("   initrd '%s'\n", cod->bd->initrd);
-		pb_log("   args   '%s'\n", cod->bd->args);
-
-		/* If this is the default_item select it and start timer. */
-
-		if (cod->opt_hash == cui->default_item) {
-			selected = i->nci;
-			ui_timer_kick(&cui->timer);
-		}
+	i->on_edit = cui_boot_editor_run;
+	i->on_execute = cui_boot;
+	i->data = cod = talloc(i, struct cui_opt_data);
+
+	cod->opt = opt;
+	cod->dev = dev;
+	cod->opt_hash = pb_opt_hash(dev, opt);
+	cod->name = opt->name;
+	cod->bd = talloc(i, struct pb_boot_data);
+
+	cod->bd->image = talloc_strdup(cod->bd, opt->boot_image_file);
+	cod->bd->initrd = talloc_strdup(cod->bd, opt->initrd_file);
+	cod->bd->args = talloc_strdup(cod->bd, opt->boot_args);
+
+	name = cui_make_item_name(i, cod);
+	pmenu_item_setup(cui->main, i, insert_pt, name);
+
+	pb_log("%s: adding opt '%s'\n", __func__, cod->name);
+	pb_log("   image  '%s'\n", cod->bd->image);
+	pb_log("   initrd '%s'\n", cod->bd->initrd);
+	pb_log("   args   '%s'\n", cod->bd->args);
+
+	/* If this is the default_item select it and start timer. */
+	if (cod->opt_hash == cui->default_item) {
+		selected = i->nci;
+		ui_timer_kick(&cui->timer);
 	}
 
 	/* Re-attach the items array. */
-
 	result = set_menu_items(cui->main->ncm, cui->main->items);
 
 	if (result)
@@ -506,7 +493,8 @@ static void cui_device_remove(struct device *dev, void *arg)
 }
 
 static struct discover_client_ops cui_client_ops = {
-	.device_add = cui_device_add,
+	.device_add = NULL,
+	.boot_option_add = cui_boot_option_add,
 	.device_remove = cui_device_remove,
 };
 
diff --git a/ui/twin/pbt-client.c b/ui/twin/pbt-client.c
index 875b408..09d8e99 100644
--- a/ui/twin/pbt-client.c
+++ b/ui/twin/pbt-client.c
@@ -63,7 +63,7 @@ static int pbt_client_boot(struct pbt_item *item)
 		pbt_item_name(item));
 
 	result = discover_client_boot(item->pbt_client->discover_client,
-			opt_data->dev, opt_data->opt, opt_data->bd);
+			NULL, opt_data->opt, opt_data->bd);
 
 	if (result) {
 		pb_log("%s: failed: %s\n", __func__, opt_data->bd->image);
@@ -83,101 +83,89 @@ static int pbt_client_on_edit(struct pbt_item *item)
 static int pbt_device_add(struct device *dev, struct pbt_client *client)
 {
 	struct pbt_frame *const frame = &client->frame;
-	struct pbt_item *device_item;
-	struct boot_option *opt;
+	struct pbt_item *item;
 	struct pbt_quad q;
 	const char *icon_file;
-	struct pbt_item *selected_item = NULL;
 
 	pb_log("%s: %p %s: n_options %d\n", __func__, dev, dev->id,
 		dev->n_options);
 
 	pb_protocol_dump_device(dev, "", pb_log_get_stream());
 
-	/* device_item */
-
 	// FIXME: check for existing dev->id
 
 	icon_file = dev->icon_file ? dev->icon_file : pbt_icon_chooser(dev->id);
 
-	device_item = pbt_item_create_reduced(frame->top_menu, dev->id,
+	item = pbt_item_create_reduced(frame->top_menu, dev->id,
 		frame->top_menu->n_items, icon_file);
 
-	if (!device_item)
-		goto fail_device_item_create;
+	if (!item)
+		return -1;
 
-	device_item->pb_device = dev;
+	item->pb_device = dev;
+	dev->ui_info = item;
 	frame->top_menu->n_items++;
 
 	/* sub_menu */
-
 	q.x = frame->top_menu->window->pixmap->width;
 	q.y = 0;
 	q.width = frame->top_menu->scr->tscreen->width - q.x;
 	q.height = frame->top_menu->scr->tscreen->height;
 
-	device_item->sub_menu = pbt_menu_create(device_item, dev->id,
+	item->sub_menu = pbt_menu_create(item, dev->id,
 		frame->top_menu->scr, frame->top_menu, &q,
 		&frame->top_menu->layout);
-	if (!device_item->sub_menu)
-		goto fail_sub_menu_create;
+	if (!item->sub_menu)
+		return -1;
+
+	pbt_menu_show(frame->top_menu, 1);
 
-	list_for_each_entry(&dev->boot_options, opt, list) {
-		struct pbt_item *i;
-		struct pb_opt_data *opt_data;
+	return 0;
+}
+
+static int pbt_boot_option_add(struct device *dev, struct boot_option *opt,
+		struct pbt_client *client)
+{
+	struct pbt_item *opt_item, *device_item;
+	struct pb_opt_data *opt_data;
 
-		i = pbt_item_create(device_item->sub_menu,
+	device_item = dev->ui_info;
+
+	opt_item = pbt_item_create(device_item->sub_menu,
 			opt->id, device_item->sub_menu->n_items++,
 			opt->icon_file, opt->name, opt->description);
 
-		if (!i) {
-			assert(0);
-			break;
-		}
-
-		i->pb_opt = opt;
-		i->pbt_client = client;
-		i->on_execute = pbt_client_boot;
-		i->on_edit = pbt_client_on_edit;
-
-		i->data = opt_data = talloc(i, struct pb_opt_data);
-		opt_data->name = opt->name;
-		opt_data->bd = talloc(i, struct pb_boot_data);
-		opt_data->bd->image = talloc_strdup(opt_data->bd,
-			opt->boot_image_file);
-		opt_data->bd->initrd = talloc_strdup(opt_data->bd,
-			opt->initrd_file);
-		opt_data->bd->args = talloc_strdup(opt_data->bd,
-			opt->boot_args);
-		opt_data->dev = dev;
-		opt_data->opt = opt;
-		opt_data->opt_hash = pb_opt_hash(dev, opt);
-
-		/* Select the first item as default. */
-
-		if (!selected_item)
-			selected_item = i;
-
-		/* If this is the default_item select it and start timer. */
-
-		if (opt_data->opt_hash
-			== device_item->sub_menu->default_item_hash) {
-			selected_item = i;
-			ui_timer_kick(&client->signal_data.timer);
-		}
+	opt_item->pb_opt = opt;
+	opt_item->pbt_client = client;
+	opt_item->on_execute = pbt_client_boot;
+	opt_item->on_edit = pbt_client_on_edit;
+
+	opt_item->data = opt_data = talloc(opt_item, struct pb_opt_data);
+	opt_data->name = opt->name;
+	opt_data->bd = talloc(opt_item, struct pb_boot_data);
+	opt_data->bd->image = talloc_strdup(opt_data->bd,
+		opt->boot_image_file);
+	opt_data->bd->initrd = talloc_strdup(opt_data->bd,
+		opt->initrd_file);
+	opt_data->bd->args = talloc_strdup(opt_data->bd,
+		opt->boot_args);
+	opt_data->opt = opt;
+	opt_data->opt_hash = pb_opt_hash(dev, opt);
+
+	/* If this is the default_item select it and start timer. */
+	if (opt_data->opt_hash == device_item->sub_menu->default_item_hash) {
+		device_item->selected_item = opt_item;
+		pbt_menu_set_selected(device_item->sub_menu, opt_item);
+		ui_timer_kick(&client->signal_data.timer);
 	}
 
-	pbt_menu_set_selected(device_item->sub_menu, selected_item);
+	/* Select the first item as default. */
+	if (!device_item->selected_item)
+		pbt_menu_set_selected(device_item->sub_menu, opt_item);
 
-	pbt_menu_show(frame->top_menu, 1);
 	twin_screen_update(client->frame.scr->tscreen);
 
 	return 0;
-
-fail_sub_menu_create:
-fail_device_item_create:
-	assert(0);
-	return -1;
 }
 
 static void pbt_device_remove(struct device *dev, struct pbt_client *client)
@@ -248,6 +236,7 @@ static void pbt_device_remove(struct device *dev, struct pbt_client *client)
 
 static struct discover_client_ops pbt_client_ops = {
 	.device_add = (void *)pbt_device_add,
+	.boot_option_add = (void *)pbt_boot_option_add,
 	.device_remove = (void *)pbt_device_remove,
 };
 
diff --git a/ui/twin/pbt-menu.h b/ui/twin/pbt-menu.h
index cb696ea..698b64d 100644
--- a/ui/twin/pbt-menu.h
+++ b/ui/twin/pbt-menu.h
@@ -41,6 +41,7 @@ struct pbt_item
 	twin_pixmap_t *pixmap_active;
 
 	struct pbt_menu *sub_menu;
+	struct pbt_item *selected_item;
 
 	int (*on_execute)(struct pbt_item *item);
 	int (*on_edit)(struct pbt_item *item);