[PATCH v3] xenbus: advertize control feature flags

[PATCH v3] xenbus: advertize control feature flags

[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 <paul.durrant@citrix.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Juergen Gross <jgross@suse.com>
---

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(-)

diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index e12bd36..e16ba9f 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -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");
-- 
2.1.4

Re: [PATCH v3] xenbus: advertize control feature flags

[Juergen Gross]

[-- Attachment #1: Type: text/plain, Size: 674 bytes --]

On 10/10/16 12:13, Paul Durrant wrote:
> 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 <paul.durrant@citrix.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Juergen Gross <jgross@suse.com>

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

[-- Attachment #2: 0001-xenbus-advertise-control-feature-flags.patch --]
[-- Type: text/x-patch, Size: 3321 bytes --]

>From 2a09fbf3a53ac1491e75746742682105f39f9e3a Mon Sep 17 00:00:00 2001
From: Juergen Gross <jgross@suse.com>
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 <paul.durrant@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 drivers/xen/manage.c | 43 +++++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index e12bd36..46d435a 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -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;
 }
 
-- 
2.6.6

Re: [PATCH v3] xenbus: advertize control feature flags

[David Vrabel]

On 11/10/16 11:53, Juergen Gross wrote:
> On 10/10/16 12:13, Paul Durrant wrote:
>> 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 <paul.durrant@citrix.com>
>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Cc: David Vrabel <david.vrabel@citrix.com>
>> Cc: Juergen Gross <jgross@suse.com>
> 
> 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 <david.vrabel@citrix.com> if you make this change.

David