2016-10-10 10:33:29

by Paul Durrant

[permalink] [raw]
Subject: [PATCH v3] 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(-)

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


2016-10-11 10:54:03

by Juergen Gross

[permalink] [raw]
Subject: Re: [PATCH v3] xenbus: advertize control feature flags

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 <[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


Attachments:
0001-xenbus-advertise-control-feature-flags.patch (3.24 kB)

2016-10-11 11:29:05

by David Vrabel

[permalink] [raw]
Subject: Re: [PATCH v3] xenbus: advertize control feature flags

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 <[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.

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

David