[PATCH] Convert shutdown to use xenstore

[PATCH] Convert shutdown to use xenstore

[Dan Smith]

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

The attached patch:

1. Converts the shutdown driver and xend to use the store instead of
   control messages, 

2. Includes Anthony's xenstore notification code, and

3. Changes xend so that sysrq's are no longer sent as "special case"
   shutdown messages.  Store keys are cheap, so making the sysrq
   delivery less obscure is good.

I think I have made all of the appropriate modifications to Xend, but
it is complex, so I may have missed something.  Comments are welcome.

Signed-off-by: Dan Smith <danms@us.ibm.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: shutdown.patch --]
[-- Type: text/x-patch, Size: 13812 bytes --]

diff -r 60c4cd9ebaa1 linux-2.6-xen-sparse/arch/xen/kernel/reboot.c
--- a/linux-2.6-xen-sparse/arch/xen/kernel/reboot.c	Wed Aug  3 16:11:32 2005
+++ b/linux-2.6-xen-sparse/arch/xen/kernel/reboot.c	Wed Aug  3 15:43:36 2005
@@ -11,7 +11,6 @@
 #include <linux/sysrq.h>
 #include <asm/irq.h>
 #include <asm/mmu_context.h>
-#include <asm-xen/ctrl_if.h>
 #include <asm-xen/evtchn.h>
 #include <asm-xen/hypervisor.h>
 #include <asm-xen/xen-public/dom0_ops.h>
@@ -19,6 +18,11 @@
 #include <asm-xen/queues.h>
 #include <asm-xen/xenbus.h>
 
+#define SHUTDOWN_INVALID  -1
+#define SHUTDOWN_POWEROFF  0
+#define SHUTDOWN_REBOOT    1
+#define SHUTDOWN_SUSPEND   2
+
 void machine_restart(char * __unused)
 {
 	/* We really want to get pending console data out before we die. */
@@ -53,7 +57,7 @@
  */
 
 /* Ignore multiple shutdown requests. */
-static int shutting_down = -1;
+static int shutting_down = SHUTDOWN_INVALID;
 
 static void __do_suspend(void)
 {
@@ -126,8 +130,6 @@
 
     xenbus_suspend();
 
-    ctrl_if_suspend();
-
     irq_suspend();
 
     gnttab_suspend();
@@ -140,7 +142,7 @@
 
     HYPERVISOR_suspend(virt_to_machine(suspend_record) >> PAGE_SHIFT);
 
-    shutting_down = -1; 
+    shutting_down = SHUTDOWN_INVALID; 
 
     memcpy(&xen_start_info, &suspend_record->resume_info,
            sizeof(xen_start_info));
@@ -163,8 +165,6 @@
 
     irq_resume();
 
-    ctrl_if_resume();
-
     xenbus_resume();
 
 #ifdef CONFIG_SMP
@@ -204,7 +204,7 @@
 
     switch ( shutting_down )
     {
-    case CMSG_SHUTDOWN_POWEROFF:
+    case SHUTDOWN_POWEROFF:
         if ( execve("/sbin/poweroff", poweroff_argv, envp) < 0 )
         {
             sys_reboot(LINUX_REBOOT_MAGIC1,
@@ -214,7 +214,7 @@
         }
         break;
 
-    case CMSG_SHUTDOWN_REBOOT:
+    case SHUTDOWN_REBOOT:
         if ( execve("/sbin/reboot", restart_argv, envp) < 0 )
         {
             sys_reboot(LINUX_REBOOT_MAGIC1,
@@ -225,7 +225,7 @@
         break;
     }
 
-    shutting_down = -1; /* could try again */
+    shutting_down = SHUTDOWN_INVALID; /* could try again */
 
     return 0;
 }
@@ -234,7 +234,7 @@
 {
     int err;
 
-    if ( shutting_down != CMSG_SHUTDOWN_SUSPEND )
+    if ( shutting_down != SHUTDOWN_SUSPEND )
     {
         err = kernel_thread(shutdown_process, NULL, CLONE_FS | CLONE_FILES);
         if ( err < 0 )
@@ -246,42 +246,103 @@
     }
 }
 
-static void shutdown_handler(ctrl_msg_t *msg, unsigned long id)
+static void shutdown_handler(struct xenbus_watch *watch, const char *node)
 {
     static DECLARE_WORK(shutdown_work, __shutdown_handler, NULL);
 
-    if ( msg->subtype == CMSG_SHUTDOWN_SYSRQ )
-    {
-	int sysrq = ((shutdown_sysrq_t *)&msg->msg[0])->key;
-	
+    int type = -1;
+
+    if (!xenbus_scanf("control", "shutdown", "%i", &type)) {
+        printk("Unable to read code in control/shutdown\n");
+        return;
+    };
+
+    xenbus_printf("control", "shutdown", "%i", SHUTDOWN_INVALID);
+
+    if ((type == SHUTDOWN_POWEROFF) ||
+        (type == SHUTDOWN_REBOOT)   ||
+        (type == SHUTDOWN_SUSPEND)) {
+        shutting_down = type;
+        schedule_work(&shutdown_work);
+    }
+
+}
+
 #ifdef CONFIG_MAGIC_SYSRQ
+static void sysrq_handler(struct xenbus_watch *watch, const char *node)
+{
+    char sysrq_key = '\0';
+    
+    if (!xenbus_scanf("control", "sysrq", "%c", &sysrq_key)) {
+        printk("Unable to read sysrq code in control/sysrq\n");
+        return;
+    }
+
+    xenbus_printf("control", "sysrq", "%c", '\0');
+
+    if (sysrq_key != '\0') {
+
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
-	handle_sysrq(sysrq, NULL, NULL);
-#else
-	handle_sysrq(sysrq, NULL, NULL, NULL);
-#endif
-#endif
-    }
-    else if ( (shutting_down == -1) &&
-         ((msg->subtype == CMSG_SHUTDOWN_POWEROFF) ||
-          (msg->subtype == CMSG_SHUTDOWN_REBOOT) ||
-          (msg->subtype == CMSG_SHUTDOWN_SUSPEND)) )
-    {
-        shutting_down = msg->subtype;
-        schedule_work(&shutdown_work);
-    }
-    else
-    {
-        printk("Ignore spurious shutdown request\n");
-    }
-
-    ctrl_if_send_response(msg);
+        handle_sysrq(sysrq_key, NULL, NULL);
+#else
+        handle_sysrq(sysrq_key, NULL, NULL, NULL);
+#endif
+    }
+}
+#endif
+
+static struct xenbus_watch shutdown_watch = {
+    .node = "control/shutdown",
+    .callback = shutdown_handler
+};
+
+#ifdef CONFIG_MAGIC_SYSRQ
+static struct xenbus_watch sysrq_watch = {
+    .node ="control/sysrq",
+    .callback = sysrq_handler
+};
+#endif
+
+static struct notifier_block xenstore_notifier;
+
+static int setup_shutdown_watcher(struct notifier_block *notifier,
+                                  unsigned long event,
+                                  void *data)
+{
+    int err1=0, err2=0;
+
+    down(&xenbus_lock);
+    err1 = register_xenbus_watch(&shutdown_watch);
+#ifdef CONFIG_MAGIC_SYSRQ
+    err2 = register_xenbus_watch(&sysrq_watch);
+#endif
+    up(&xenbus_lock);
+
+    if (err1) {
+        printk("Failed to set shutdown watcher\n");
+    }
+    
+#ifdef CONFIG_MAGIC_SYSRQ
+    if (err2) {
+        printk("Failed to set sysrq watcher\n");
+    }
+#endif
+
+    return NOTIFY_STOP;
 }
 
 static int __init setup_shutdown_event(void)
 {
-    ctrl_if_register_receiver(CMSG_SHUTDOWN, shutdown_handler, 0);
+    
+    xenstore_notifier.notifier_call = setup_shutdown_watcher;
+
+    if (xen_start_info.store_evtchn) {
+        setup_shutdown_watcher(&xenstore_notifier, 0, NULL);
+    } else {
+        register_xenstore_notifier(&xenstore_notifier);
+    }
+    
     return 0;
 }
 
-__initcall(setup_shutdown_event);
+subsys_initcall(setup_shutdown_event);
diff -r 60c4cd9ebaa1 linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c
--- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c	Wed Aug  3 16:11:32 2005
+++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c	Wed Aug  3 15:43:36 2005
@@ -36,9 +36,15 @@
 #include <linux/ctype.h>
 #include <linux/fcntl.h>
 #include <stdarg.h>
+#include <linux/notifier.h>
 #include "xenbus_comms.h"
 
 #define streq(a, b) (strcmp((a), (b)) == 0)
+
+/* Protects notifier chain */
+DECLARE_MUTEX(xenstore_control);
+
+static struct notifier_block *xenstore_chain;
 
 /* If something in array of ids matches this device, return it. */
 static const struct xenbus_device_id *
@@ -309,6 +315,26 @@
 	up(&xenbus_lock);
 }
 
+int register_xenstore_notifier(struct notifier_block *nb)
+{
+	int ret;
+
+	if ((ret = down_interruptible(&xenstore_control)) != 0) 
+		return ret;
+	ret = notifier_chain_register(&xenstore_chain, nb);
+	up(&xenstore_control);
+	return ret;
+}
+EXPORT_SYMBOL(register_xenstore_notifier);
+
+void unregister_xenstore_notifier(struct notifier_block *nb)
+{
+	down(&xenstore_control);
+	notifier_chain_unregister(&xenstore_chain, nb);
+	up(&xenstore_control);
+}
+EXPORT_SYMBOL(unregister_xenstore_notifier);
+
 /* called from a thread in privcmd/privcmd.c */
 int do_xenbus_probe(void *unused)
 {
@@ -323,6 +349,15 @@
 		return err;
 	}
 
+	err = notifier_call_chain(&xenstore_chain, 0, 0);
+	if (err == NOTIFY_BAD) {
+		printk("%s: calling xenstore notify chain failed\n",
+		       __FUNCTION__);
+		return -EINVAL;
+	}
+
+	err = 0;
+
 	/* Initialize non-xenbus drivers */
 	balloon_init_watcher();
 
diff -r 60c4cd9ebaa1 linux-2.6-xen-sparse/include/asm-xen/xenbus.h
--- a/linux-2.6-xen-sparse/include/asm-xen/xenbus.h	Wed Aug  3 16:11:32 2005
+++ b/linux-2.6-xen-sparse/include/asm-xen/xenbus.h	Wed Aug  3 15:43:36 2005
@@ -29,6 +29,7 @@
  * IN THE SOFTWARE.
  */
 #include <linux/device.h>
+#include <linux/notifier.h>
 #include <asm/semaphore.h>
 
 /* A xenbus device. */
@@ -112,6 +113,10 @@
 	void (*callback)(struct xenbus_watch *, const char *node);
 };
 
+/* notifer routines for when the xenstore comes up */
+int register_xenstore_notifier(struct notifier_block *nb);
+void unregister_xenstore_notifier(struct notifier_block *nb);
+
 int register_xenbus_watch(struct xenbus_watch *watch);
 void unregister_xenbus_watch(struct xenbus_watch *watch);
 
diff -r 60c4cd9ebaa1 tools/python/xen/xend/XendClient.py
--- a/tools/python/xen/xend/XendClient.py	Wed Aug  3 16:11:32 2005
+++ b/tools/python/xen/xend/XendClient.py	Wed Aug  3 15:43:36 2005
@@ -213,11 +213,15 @@
         return self.xendPost(self.domainurl(id),
                              {'op'      : 'pause' })
 
-    def xend_domain_shutdown(self, id, reason, key=0):
+    def xend_domain_shutdown(self, id, reason):
         return self.xendPost(self.domainurl(id),
                              {'op'      : 'shutdown',
-                              'reason'  : reason,
-                              'key'     : key })
+                              'reason'  : reason})
+
+    def xend_domain_sysrq(self, id, key):
+        return self.xendPost(self.domainurl(id),
+                             {'op'      : 'sysrq',
+                              'key'     : key})
 
     def xend_domain_destroy(self, id, reason):
         return self.xendPost(self.domainurl(id),
diff -r 60c4cd9ebaa1 tools/python/xen/xend/XendDomain.py
--- a/tools/python/xen/xend/XendDomain.py	Wed Aug  3 16:11:32 2005
+++ b/tools/python/xen/xend/XendDomain.py	Wed Aug  3 15:43:36 2005
@@ -386,7 +386,7 @@
         except Exception, ex:
             raise XendError(str(ex))
     
-    def domain_shutdown(self, id, reason='poweroff', key=0):
+    def domain_shutdown(self, id, reason='poweroff'):
         """Shutdown domain (nicely).
          - poweroff: restart according to exit code and restart mode
          - reboot:   restart on exit
@@ -402,9 +402,16 @@
         eserver.inject('xend.domain.shutdown', [dominfo.name, dominfo.id, reason])
         if reason == 'halt':
             reason = 'poweroff'
-        val = dominfo.shutdown(reason, key=key)
-        if not reason in ['suspend', 'sysrq']:
+        val = dominfo.shutdown(reason)
+        if not reason in ['suspend']:
             self.domain_shutdowns()
+        return val
+
+    def domain_sysrq(self, id, key):
+        """Send a SysRq to a domain
+        """
+        dominfo = self.domain_lookup(id)
+        val = dominfo.send_sysrq(key)
         return val
 
     def domain_shutdowns(self):
diff -r 60c4cd9ebaa1 tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py	Wed Aug  3 16:11:32 2005
+++ b/tools/python/xen/xend/XendDomainInfo.py	Wed Aug  3 15:43:36 2005
@@ -52,13 +52,12 @@
     DOMAIN_CRASH   : "crash",
     }
 
-"""Map shutdown reasons to the message type to use.
+"""Map shutdown reasons to codes
 """
-shutdown_messages = {
-    'poweroff' : 'shutdown_poweroff_t',
-    'reboot'   : 'shutdown_reboot_t',
-    'suspend'  : 'shutdown_suspend_t',
-    'sysrq'    : 'shutdown_sysrq_t',
+shutdown_codes = {
+    'poweroff' : DOMAIN_POWEROFF,
+    'reboot'   : DOMAIN_REBOOT,
+    'suspend'  : DOMAIN_SUSPEND,
     }
 
 RESTART_ALWAYS   = 'always'
@@ -152,8 +151,6 @@
         vm = cls(db)
         vm.construct(config)
         vm.saveToDB(sync=True)
-        # Flush info to xenstore immediately
-        vm.exportToDB()
 
         return vm
 
@@ -976,19 +973,20 @@
 
             self.channel.writeRequest(msg)
 
-    def shutdown(self, reason, key=0):
-        msgtype = shutdown_messages.get(reason)
-        if not msgtype:
+    def shutdown(self, reason):
+        reasonid = shutdown_codes.get(reason)
+        if reasonid == None:
             raise XendError('invalid reason:' + reason)
-        extra = {}
-        if reason == 'sysrq':
-            extra['key'] = key
-        if self.channel:
-            msg = messages.packMsg(msgtype, extra)
-            self.channel.writeRequest(msg)
-        if not reason in ['suspend', 'sysrq']:
-            self.shutdown_pending = {'start':time.time(), 'reason':reason,
-                                     'key':key}
+        db = self.db.addChild("/control");
+        db['shutdown'] = '%i' % reasonid;
+        db.saveDB(save=True);
+        if not reason in ['suspend']:
+            self.shutdown_pending = {'start':time.time(), 'reason':reason}
+
+    def send_sysrq(self, key=0):
+        db = self.db.addChild("/control");
+        db['sysrq'] = '%c' % key;
+        db.saveDB(save=True);        
 
     def shutdown_time_left(self, timeout):
         if not self.shutdown_pending:
diff -r 60c4cd9ebaa1 tools/python/xen/xend/server/SrvDomain.py
--- a/tools/python/xen/xend/server/SrvDomain.py	Wed Aug  3 16:11:32 2005
+++ b/tools/python/xen/xend/server/SrvDomain.py	Wed Aug  3 15:43:36 2005
@@ -41,9 +41,17 @@
     def op_shutdown(self, op, req):
         fn = FormFn(self.xd.domain_shutdown,
                     [['dom',    'int'],
-                     ['reason', 'str'],
+                     ['reason', 'str']])
+        val = fn(req.args, {'dom': self.dom.id})
+        req.setResponseCode(http.ACCEPTED)
+        req.setHeader("Location", "%s/.." % req.prePathURL())
+        return val
+
+    def op_sysrq(self, op, req):
+        fn = FormFn(self.xd.domain_sysrq,
+                    [['dom',    'int'],
                      ['key',    'int']])
-        val = fn(req.args, {'dom': self.dom.id})
+        val = fn(req.args, {'dom' : self.dom.id})
         req.setResponseCode(http.ACCEPTED)
         req.setHeader("Location", "%s/.." % req.prePathURL())
         return val
diff -r 60c4cd9ebaa1 tools/python/xen/xm/sysrq.py
--- a/tools/python/xen/xm/sysrq.py	Wed Aug  3 16:11:32 2005
+++ b/tools/python/xen/xm/sysrq.py	Wed Aug  3 15:43:36 2005
@@ -21,9 +21,6 @@
          fn=set_true, default=0,
          use="Print this help.")
 
-def sysrq(dom, req):
-    server.xend_domain_shutdown(dom, 'sysrq', req)
-
 def main(argv):
     opts = gopts
     args = opts.parse(argv)
@@ -36,4 +33,4 @@
     if len(args) < 2: opts.err('Missing sysrq character')
     dom = args[0]
     req = ord(args[1][0])
-    sysrq(dom, req)
+    server.xend_domain_sysrq(dom, req)

[-- Attachment #3: Type: text/plain, Size: 88 bytes --]


-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms@us.ibm.com

[-- Attachment #4: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] Convert shutdown to use xenstore

[Mark Williamson]

Hi Dan,

Any reason not to use textual data in the store rather than numbers?  IMO it'd 
make it a little more readable - numbers only make sense if you have the 
header file - but I don't know if there's a convention on this.

Cheers,
Mark

On Thursday 04 August 2005 16:35, Dan Smith wrote:
> The attached patch:
>
> 1. Converts the shutdown driver and xend to use the store instead of
>    control messages,
>
> 2. Includes Anthony's xenstore notification code, and
>
> 3. Changes xend so that sysrq's are no longer sent as "special case"
>    shutdown messages.  Store keys are cheap, so making the sysrq
>    delivery less obscure is good.
>
> I think I have made all of the appropriate modifications to Xend, but
> it is complex, so I may have missed something.  Comments are welcome.
>
> Signed-off-by: Dan Smith <danms@us.ibm.com>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Re: [PATCH] Convert shutdown to use xenstore

[Rusty Russell]

On Fri, 2005-08-05 at 02:58 +0100, Mark Williamson wrote:
> Hi Dan,
> 
> Any reason not to use textual data in the store rather than numbers?  IMO it'd 
> make it a little more readable - numbers only make sense if you have the 
> header file - but I don't know if there's a convention on this.

If something fundamentally *is* a number, or an ordinal, that's one
thing.  But otherwise, a simple value is better.

We've seen this in sysfs and Open Firmware; the only defence against
crap accumulating in the store is constant vigilance.  This is only
likely if it's actually useful to browse the thing, ie. as consistent
and human-readable as possible.  This is also an end in itself:
something that is self-evident and easy to understand is easier to
maintain ("hey, I was using the lower bit of that value to determine if
it's read-only, and you broke it!" vs "you removed the field called
"read-only").

An ideal system is one where I can implement a new backend, frontend or
tool simply by looking at the paths and values in the store.  It also
has clear implications for diagnosing problems.

Cheers,
Rusty.
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

Re: [PATCH] Convert shutdown to use xenstore

[Rusty Russell]

On Thu, 2005-08-04 at 08:35 -0700, Dan Smith wrote:
> 2. Includes Anthony's xenstore notification code, and

There are subtleties here.  We do need a notifier, because in domain 0,
the store gets started long after boot (when xenstored starts).
However, we don't need a notifier for domU code, because the store
exists from the moment we boot.

ie. for domU, this notifier chain is a noop, and I think the name should
reflect that, otherwise we'll have innocent drivers thinking registering
is the right thing.  register_dom0_xenstore_start()?

Or we could hide the whole thing, and simply call the notifier
immediately in the dom0/store-already-up case:

/* Only required if your code runs in dom0 as well as domU */
static int register_xenstore_notifier(struct notifier_block *notifier)
{
	int ret = 0;

	down(&xenbus_lock);
	if (xen_start_info.evtchn)
		ret = notifier.notifier_call(notifier, 0, NULL);
	else
		notifier_chain_register(&xenstore_chain, nb);
	up(&xenbus_lock);
	return ret;
}

Implementation detail: just use the xenbus_lock rather than another
lock, and make sure you hold it while traversing, not just registering!

Rusty.
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

Re: [PATCH] Convert shutdown to use xenstore

[Anthony Liguori]

Rusty Russell wrote:

>/* Only required if your code runs in dom0 as well as domU */
>static int register_xenstore_notifier(struct notifier_block *notifier)
>{
>	int ret = 0;
>
>	down(&xenbus_lock);
>	if (xen_start_info.evtchn)
>		ret = notifier.notifier_call(notifier, 0, NULL);
>	else
>		notifier_chain_register(&xenstore_chain, nb);
>	up(&xenbus_lock);
>	return ret;
>}
>  
>
I like this.  It avoids having special case code for dom0 in all the 
drivers.

>Implementation detail: just use the xenbus_lock rather than another
>lock, and make sure you hold it while traversing, not just registering!
>  
>
I was following kernel/cpu.c:cpu_down().  Definitely makes sense though.

Regards,

Anthony Liguori

>Rusty.
>  
>

Re: [PATCH] Convert shutdown to use xenstore

[Keir Fraser]

On 5 Aug 2005, at 03:09, Rusty Russell wrote:

> Or we could hide the whole thing, and simply call the notifier
> immediately in the dom0/store-already-up case:

Yes!

  -- Keir

Re: [PATCH] Convert shutdown to use xenstore

[aq]

On 8/5/05, Dan Smith <danms@us.ibm.com> wrote:
> The attached patch:
> 
> 1. Converts the shutdown driver and xend to use the store instead of
>    control messages,
> 
> 2. Includes Anthony's xenstore notification code, and
> 
> 3. Changes xend so that sysrq's are no longer sent as "special case"
>    shutdown messages.  Store keys are cheap, so making the sysrq
>    delivery less obscure is good.
> 

+static int setup_shutdown_watcher(struct notifier_block *notifier,
+                                  unsigned long event,
+                                  void *data)
+{
+    int err1=0, err2=0;
+
+    down(&xenbus_lock);
+    err1 = register_xenbus_watch(&shutdown_watch);
+#ifdef CONFIG_MAGIC_SYSRQ
+    err2 = register_xenbus_watch(&sysrq_watch);
+#endif
+    up(&xenbus_lock);
+
+    if (err1) {
+        printk("Failed to set shutdown watcher\n");
+    }

How about putting declaration of err2 inside a "#ifdef CONFIG_MAGIC_SYSRQ" ?

Besides i think some printk should use KERN_ERR to report error properly.

regards,
aq

Re: [PATCH] Convert shutdown to use xenstore

[Dan Smith]

aq> How about putting declaration of err2 inside a "#ifdef
aq> CONFIG_MAGIC_SYSRQ" ?

Oh, I guess that'll issue a warning if CONFIG_MAGIC_SYSRQ isn't
enabled, won't it?

aq> Besides i think some printk should use KERN_ERR to report error
aq> properly.

OK.  I'll wrap these and the other suggestions in a new patch and
re-submit tomorrow.

-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms@us.ibm.com