2016-10-10 08:08:40

by Paul Durrant

[permalink] [raw]
Subject: [PATCH] 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]>
---
drivers/xen/manage.c | 45 +++++++++++++++++++++++++++++++++++----------
1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index e12bd36..86cf57c 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, true },
+ { "reboot", do_reboot, false },
+#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,31 @@ 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++) {
+ const char *fmt = "feature-%s";
+ int size;
+ char *node;
+
+ if (!handler->flag)
+ continue;
+
+ size = snprintf(NULL, 0, fmt, handler->command);
+
+ node = kmalloc(++size, GFP_KERNEL);
+ if (!node) {
+ pr_err("Failed to allocate feature flag\n");
+ return -ENOMEM;
+ }
+
+ (void) snprintf(node, size, fmt, handler->command);
+ 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-10 09:18:21

by Jürgen Groß

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

On 10/10/16 09:43, 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]>
> ---
> drivers/xen/manage.c | 45 +++++++++++++++++++++++++++++++++++----------
> 1 file changed, 35 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
> index e12bd36..86cf57c 100644
> --- a/drivers/xen/manage.c
> +++ b/drivers/xen/manage.c

> @@ -309,8 +311,31 @@ 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++) {
> + const char *fmt = "feature-%s";
> + int size;
> + char *node;
> +
> + if (!handler->flag)
> + continue;
> +
> + size = snprintf(NULL, 0, fmt, handler->command);
> +
> + node = kmalloc(++size, GFP_KERNEL);
> + if (!node) {
> + pr_err("Failed to allocate feature flag\n");
> + return -ENOMEM;
> + }
> +
> + (void) snprintf(node, size, fmt, handler->command);

One other nit: please us kasprintf() instead of open coding it.


Juergen

2016-10-10 09:23:54

by Jürgen Groß

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

On 10/10/16 09:43, 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]>
> ---
> drivers/xen/manage.c | 45 +++++++++++++++++++++++++++++++++++----------
> 1 file changed, 35 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
> index e12bd36..86cf57c 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, true },
> + { "reboot", do_reboot, false },

I think you meant to set the flag to "false" for halt and "true" for
reboot, no?


Juergen

2016-10-10 09:40:48

by Paul Durrant

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

> -----Original Message-----
> From: Juergen Gross [mailto:[email protected]]
> Sent: 10 October 2016 10:11
> To: Paul Durrant <[email protected]>; [email protected];
> [email protected]
> Cc: Boris Ostrovsky <[email protected]>; David Vrabel
> <[email protected]>
> Subject: Re: [PATCH] xenbus: advertize control feature flags
>
> On 10/10/16 09:43, 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]>
> > ---
> > drivers/xen/manage.c | 45 +++++++++++++++++++++++++++++++++++--
> --------
> > 1 file changed, 35 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
> > index e12bd36..86cf57c 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, true },
> > + { "reboot", do_reboot, false },
>
> I think you meant to set the flag to "false" for halt and "true" for
> reboot, no?
>

Yes, you're right. Don't know why I got that backwards. Will fix in v2.

Paul

>
> Juergen

2016-10-10 10:13:52

by Paul Durrant

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

> -----Original Message-----
> From: Juergen Gross [mailto:[email protected]]
> Sent: 10 October 2016 10:17
> To: Paul Durrant <[email protected]>; [email protected];
> [email protected]
> Cc: Boris Ostrovsky <[email protected]>; David Vrabel
> <[email protected]>
> Subject: Re: [PATCH] xenbus: advertize control feature flags
>
> On 10/10/16 09:43, 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]>
> > ---
> > drivers/xen/manage.c | 45 +++++++++++++++++++++++++++++++++++--
> --------
> > 1 file changed, 35 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
> > index e12bd36..86cf57c 100644
> > --- a/drivers/xen/manage.c
> > +++ b/drivers/xen/manage.c
>
> > @@ -309,8 +311,31 @@ 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++) {
> > + const char *fmt = "feature-%s";
> > + int size;
> > + char *node;
> > +
> > + if (!handler->flag)
> > + continue;
> > +
> > + size = snprintf(NULL, 0, fmt, handler->command);
> > +
> > + node = kmalloc(++size, GFP_KERNEL);
> > + if (!node) {
> > + pr_err("Failed to allocate feature flag\n");
> > + return -ENOMEM;
> > + }
> > +
> > + (void) snprintf(node, size, fmt, handler->command);
>
> One other nit: please us kasprintf() instead of open coding it.
>

Yes, that would be better.

Cheers,

Paul

>
> Juergen