xenbus: advertize control feature flags

PATCH Superseded Paul Durrant 3 participants 4 messages Oct 10, 2016 – Oct 11, 2016 Xen hypervisor interface

Share & tools

Paul Durrant

author

2016-10-10 10:33 UTC #0 lore raw

PATCH v3 1/1 xenbus: advertize control feature flags

The Xen docs specify several flags which a guest can set to advertize which values of the xenstore control/shutdown key it will recognize. This patch adds code to write all the relevant feature-flag keys.

Signed-off-by Paul Durrant <[email protected]>

Cc Boris Ostrovsky <[email protected]>

Cc David Vrabel <[email protected]>

Cc Juergen Gross <[email protected]>

---

v2:
- Fix flag logic inversion
- Use kasprintf()

v3:
- Re-instate check for flag mistakenly removed in v2

---
 drivers/xen/manage.c | 41 +++++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

drivers/xen/manage.c +31 ~~-10~~

index e12bd36..e16ba9f 100644
@@ -170,6 +170,7 @@ out:
 struct shutdown_handler {
 	const char *command;
 	void (*cb)(void);
+	bool flag;
 };
 
 static int poweroff_nb(struct notifier_block *cb, unsigned long code, void *unused)

@@ -206,21 +207,22 @@ static void do_reboot(void)
 	ctrl_alt_del();
 }
 
+static struct shutdown_handler shutdown_handlers[] = {
+	{ "poweroff", do_poweroff, true },
+	{ "halt", do_poweroff, false },
+	{ "reboot", do_reboot, true },
+#ifdef CONFIG_HIBERNATE_CALLBACKS
+	{ "suspend", do_suspend, true },
+#endif
+	{NULL, NULL, false },
+};
+
 static void shutdown_handler(struct xenbus_watch *watch,
 			     const char **vec, unsigned int len)
 {
 	char *str;
 	struct xenbus_transaction xbt;
 	int err;
-	static struct shutdown_handler handlers[] = {
-		{ "poweroff",	do_poweroff },
-		{ "halt",	do_poweroff },
-		{ "reboot",	do_reboot   },
-#ifdef CONFIG_HIBERNATE_CALLBACKS
-		{ "suspend",	do_suspend  },
-#endif
-		{NULL, NULL},
-	};
 	static struct shutdown_handler *handler;
 
 	if (shutting_down != SHUTDOWN_INVALID)

@@ -238,7 +240,7 @@ static void shutdown_handler(struct xenbus_watch *watch,
 		return;
 	}
 
-	for (handler = &handlers[0]; handler->command; handler++) {
+	for (handler = &shutdown_handlers[0]; handler->command; handler++) {
 		if (strcmp(str, handler->command) == 0)
 			break;
 	}

@@ -309,8 +311,27 @@ static struct notifier_block xen_reboot_nb = {
 
 static int setup_shutdown_watcher(void)
 {
+	static struct shutdown_handler *handler;
 	int err;
 
+	for (handler = &shutdown_handlers[0]; handler->command; handler++) {
+		char *node;
+
+		if (!handler->flag)
+			continue;
+
+		node = kasprintf(GFP_KERNEL, "feature-%s",
+				 handler->command);
+		if (!node) {
+			pr_err("Failed to allocate feature flag\n");
+			return -ENOMEM;
+		}
+
+		xenbus_printf(XBT_NIL, "control", node, "%u", 1);
+
+		kfree(node);
+	}
+
 	err = register_xenbus_watch(&shutdown_watch);
 	if (err) {
 		pr_err("Failed to set shutdown watcher\n");

Signature · 1 line

2.1.4

Juergen Gross

maintainer

2016-10-11 10:53 UTC #1 lore raw

Quoted 8 lines · Paul Durrant

The Xen docs specify several flags which a guest can set to advertize which values of the xenstore control/shutdown key it will recognize. This patch adds code to write all the relevant feature-flag keys.
Signed-off-by Paul Durrant <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc David Vrabel <[email protected]>
Cc Juergen Gross <[email protected]>

Hmm, I'd prefer node[] allocated on the stack over dynamic allocation.

What about something like the following? It will at least issue build warnings in case the size is too small, will use less memory and less coding.

Juergen

Quoted 1 line

From 2a09fbf3a53ac1491e75746742682105f39f9e3a Mon Sep 17 00:00:00 2001

From: Juergen Gross <[email protected]>
Date: Tue, 11 Oct 2016 12:20:11 +0200
Subject: [PATCH] xenbus: advertise control feature flags

The Xen docs specify several flags which a guest can set to advertise which values of the xenstore control/shutdown key it will recognize. This patch adds code to write all the relevant feature-flag keys.

Based-on-patch-by Paul Durrant <[email protected]>

Signed-off-by Juergen Gross <[email protected]>

---
 drivers/xen/manage.c | 43 +++++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 16 deletions(-)

drivers/xen/manage.c +27 ~~-16~~

index e12bd36..46d435a 100644
@@ -168,7 +168,9 @@ static void do_suspend(void)
 #endif	/* CONFIG_HIBERNATE_CALLBACKS */
 
 struct shutdown_handler {
-	const char *command;
+#define SHUTDOWN_CMD_SIZE 11
+	const char command[SHUTDOWN_CMD_SIZE];
+	bool flag;
 	void (*cb)(void);
 };

@@ -206,22 +208,22 @@ static void do_reboot(void)
 	ctrl_alt_del();
 }
 
+static struct shutdown_handler shutdown_handlers[] = {
+	{ "poweroff",	true,	do_poweroff },
+	{ "halt",	false,	do_poweroff },
+	{ "reboot",	true,	do_reboot   },
+#ifdef CONFIG_HIBERNATE_CALLBACKS
+	{ "suspend",	true,	do_suspend  },
+#endif
+};
+
 static void shutdown_handler(struct xenbus_watch *watch,
 			     const char **vec, unsigned int len)
 {
 	char *str;
 	struct xenbus_transaction xbt;
 	int err;
-	static struct shutdown_handler handlers[] = {
-		{ "poweroff",	do_poweroff },
-		{ "halt",	do_poweroff },
-		{ "reboot",	do_reboot   },
-#ifdef CONFIG_HIBERNATE_CALLBACKS
-		{ "suspend",	do_suspend  },
-#endif
-		{NULL, NULL},
-	};
-	static struct shutdown_handler *handler;
+	int idx;
 
 	if (shutting_down != SHUTDOWN_INVALID)
 		return;

@@ -238,13 +240,13 @@ static void shutdown_handler(struct xenbus_watch *watch,
 		return;
 	}
 
-	for (handler = &handlers[0]; handler->command; handler++) {
-		if (strcmp(str, handler->command) == 0)
+	for (idx = 0; idx < ARRAY_SIZE(shutdown_handlers); idx++) {
+		if (strcmp(str, shutdown_handlers[idx].command) == 0)
 			break;
 	}
 
 	/* Only acknowledge commands which we are prepared to handle. */
-	if (handler->cb)
+	if (idx < ARRAY_SIZE(shutdown_handlers))
 		xenbus_write(xbt, "control", "shutdown", "");
 
 	err = xenbus_transaction_end(xbt, 0);

@@ -253,8 +255,8 @@ static void shutdown_handler(struct xenbus_watch *watch,
 		goto again;
 	}
 
-	if (handler->cb) {
-		handler->cb();
+	if (idx < ARRAY_SIZE(shutdown_handlers)) {
+		shutdown_handlers[idx].cb();
 	} else {
 		pr_info("Ignoring shutdown request: %s\n", str);
 		shutting_down = SHUTDOWN_INVALID;

@@ -310,6 +312,8 @@ static struct notifier_block xen_reboot_nb = {
 static int setup_shutdown_watcher(void)
 {
 	int err;
+	int idx;
+	char node[SHUTDOWN_CMD_SIZE + sizeof("feature-")];
 
 	err = register_xenbus_watch(&shutdown_watch);
 	if (err) {

@@ -326,6 +330,13 @@ static int setup_shutdown_watcher(void)
 	}
 #endif
 
+	for (idx = 0; idx < ARRAY_SIZE(shutdown_handlers); idx++) {
+		if (!shutdown_handlers[idx].flag)
+			continue;
+		sprintf(node, "feature-%s", shutdown_handlers[idx].command);
+		xenbus_printf(XBT_NIL, "control", node, "%u", 1);
+	}
+
 	return 0;
 }

Signature · 1 line

2.6.6

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Juergen Gross

maintainer

2016-10-11 10:54 UTC #2 lore raw

In reply to Paul Durrant

Quoted 8 lines · Paul Durrant

The Xen docs specify several flags which a guest can set to advertize which values of the xenstore control/shutdown key it will recognize. This patch adds code to write all the relevant feature-flag keys.
Signed-off-by Paul Durrant <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc David Vrabel <[email protected]>
Cc Juergen Gross <[email protected]>

Hmm, I'd prefer node[] allocated on the stack over dynamic allocation.

What about something like the following? It will at least issue build warnings in case the size is too small, will use less memory and less coding.

Juergen

Quoted 1 line

From 2a09fbf3a53ac1491e75746742682105f39f9e3a Mon Sep 17 00:00:00 2001

From: Juergen Gross <[email protected]>
Date: Tue, 11 Oct 2016 12:20:11 +0200
Subject: [PATCH] xenbus: advertise control feature flags

Based-on-patch-by Paul Durrant <[email protected]>

Signed-off-by Juergen Gross <[email protected]>

---
 drivers/xen/manage.c | 43 +++++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 16 deletions(-)

drivers/xen/manage.c +27 ~~-16~~

index e12bd36..46d435a 100644
@@ -168,7 +168,9 @@ static void do_suspend(void)
 #endif	/* CONFIG_HIBERNATE_CALLBACKS */
 
 struct shutdown_handler {
-	const char *command;
+#define SHUTDOWN_CMD_SIZE 11
+	const char command[SHUTDOWN_CMD_SIZE];
+	bool flag;
 	void (*cb)(void);
 };

@@ -206,22 +208,22 @@ static void do_reboot(void)
 	ctrl_alt_del();
 }
 
+static struct shutdown_handler shutdown_handlers[] = {
+	{ "poweroff",	true,	do_poweroff },
+	{ "halt",	false,	do_poweroff },
+	{ "reboot",	true,	do_reboot   },
+#ifdef CONFIG_HIBERNATE_CALLBACKS
+	{ "suspend",	true,	do_suspend  },
+#endif
+};
+
 static void shutdown_handler(struct xenbus_watch *watch,
 			     const char **vec, unsigned int len)
 {
 	char *str;
 	struct xenbus_transaction xbt;
 	int err;
-	static struct shutdown_handler handlers[] = {
-		{ "poweroff",	do_poweroff },
-		{ "halt",	do_poweroff },
-		{ "reboot",	do_reboot   },
-#ifdef CONFIG_HIBERNATE_CALLBACKS
-		{ "suspend",	do_suspend  },
-#endif
-		{NULL, NULL},
-	};
-	static struct shutdown_handler *handler;
+	int idx;
 
 	if (shutting_down != SHUTDOWN_INVALID)
 		return;

@@ -238,13 +240,13 @@ static void shutdown_handler(struct xenbus_watch *watch,
 		return;
 	}
 
-	for (handler = &handlers[0]; handler->command; handler++) {
-		if (strcmp(str, handler->command) == 0)
+	for (idx = 0; idx < ARRAY_SIZE(shutdown_handlers); idx++) {
+		if (strcmp(str, shutdown_handlers[idx].command) == 0)
 			break;
 	}
 
 	/* Only acknowledge commands which we are prepared to handle. */
-	if (handler->cb)
+	if (idx < ARRAY_SIZE(shutdown_handlers))
 		xenbus_write(xbt, "control", "shutdown", "");
 
 	err = xenbus_transaction_end(xbt, 0);

@@ -253,8 +255,8 @@ static void shutdown_handler(struct xenbus_watch *watch,
 		goto again;
 	}
 
-	if (handler->cb) {
-		handler->cb();
+	if (idx < ARRAY_SIZE(shutdown_handlers)) {
+		shutdown_handlers[idx].cb();
 	} else {
 		pr_info("Ignoring shutdown request: %s\n", str);
 		shutting_down = SHUTDOWN_INVALID;

@@ -310,6 +312,8 @@ static struct notifier_block xen_reboot_nb = {
 static int setup_shutdown_watcher(void)
 {
 	int err;
+	int idx;
+	char node[SHUTDOWN_CMD_SIZE + sizeof("feature-")];
 
 	err = register_xenbus_watch(&shutdown_watch);
 	if (err) {

@@ -326,6 +330,13 @@ static int setup_shutdown_watcher(void)
 	}
 #endif
 
+	for (idx = 0; idx < ARRAY_SIZE(shutdown_handlers); idx++) {
+		if (!shutdown_handlers[idx].flag)
+			continue;
+		sprintf(node, "feature-%s", shutdown_handlers[idx].command);
+		xenbus_printf(XBT_NIL, "control", node, "%u", 1);
+	}
+
 	return 0;
 }

Signature · 1 line

2.6.6

David Vrabel

2016-10-11 11:26 UTC #3 lore raw

Quoted 10 lines · Juergen Gross

Quoted 8 lines · Paul Durrant
The Xen docs specify several flags which a guest can set to advertize which values of the xenstore control/shutdown key it will recognize. This patch adds code to write all the relevant feature-flag keys.
Signed-off-by Paul Durrant <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc David Vrabel <[email protected]>
Cc Juergen Gross <[email protected]>

Quoted 5 lines

Hmm, I'd prefer node[] allocated on the stack over dynamic allocation.
What about something like the following? It will at least issue build warnings in case the size is too small, will use less memory and less coding.

You still want snprintf() just to be sure. You can add a Reviewed-by David Vrabel <[email protected]> if you make this change.

David

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel