[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] xm reboot/shutdown/sysrq to HVM domain



Hi Steven,

Thank you for your comments.

Steven Smith wrote:
>> This patch enhances 'xm reboot'/'xm shutdown' commands to
>> reboot/shutdown guest Linux on HVM domain as gracefully as para-Linux.
>> In addtion, sysrq key signal can be sent to HVM domain by 'xm sysrq'
>> command.
> Thanks, that's really useful.  I have a couple of comments about the
> patch, though:
>
> -- It looks like you had some problems with ctrl_alt_del(), and instead
>    used kill_proc(cad_pid, SIGINT, 1).  What was the reason for this?

The symbol ctrl_alt_del() can't be found when it is used in loadable
module.  On build, the warning message is shown that ctrl_alt_del() is
undefined, and on loading, the error message is shown that it is unknown
symbol.  I'm not sure why this happens, but I tried kill_proc(), which
is called in ctrl_alt_del(), it works correctly.

> -- You've introduced a lot of #ifdefs into reboot.c.  It might be
>    easier to just split the file in two; did you look at this at all?

reboot.c has common process to deal with reboot/shutdown/sysrq for
para-linux (built in kernel) and full-linux (loadable module), so I
think that it would be better to be one file in consideration of code
maintenance.

> -- You set reboot_module from within a xenbus transaction.  I don't
>    think that's necessary, since xenbus_writes are supposed to be
>    atomic anyway.

The reason why I use xenbus_write is that I could not find other
interface to write xenstore through xenbus module for HVM.  I'm not sure
which interface you suggest to use, but for example, xb_write() is not
exported, so it can not be called from reboot module.  If I should use
other interface, please let me know.

> -- Because of the way mkbuildtree works, you're going to create
>    symlinks from unmodified-drivers to all of the files in
>    linux-2.6-xen-sparse/drivers/core, rather than just to reboot.c.
>    It's a trivial aesthetic issue, but it'd be nice not to create lots
>    of useless symlinks.

The modified patch is attached.

Regards,

Tetsu Yamamoto

Signed-off-by: Tetsu Yamamoto <yamamoto.tetsu@xxxxxxxxxxxxxx>


>
> Apart from that, it looks pretty reasonable.
>
> Steven.


diff -r 38f9bd7a4ce6 linux-2.6-xen-sparse/drivers/xen/core/reboot.c
--- a/linux-2.6-xen-sparse/drivers/xen/core/reboot.c    Tue Oct 03 11:39:22 
2006 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/core/reboot.c    Tue Oct 10 15:06:27 
2006 +0900
@@ -19,7 +19,13 @@
 #include <xen/xencons.h>
 #include <xen/cpu_hotplug.h>
 
+#ifdef CONFIG_XEN
 extern void ctrl_alt_del(void);
+#endif /* CONFIG_XEN */
+
+#ifndef CONFIG_XEN
+MODULE_LICENSE("Dual BSD/GPL");
+#endif /* !CONFIG_XEN */
 
 #define SHUTDOWN_INVALID  -1
 #define SHUTDOWN_POWEROFF  0
@@ -31,6 +37,7 @@ extern void ctrl_alt_del(void);
  */
 #define SHUTDOWN_HALT      4
 
+#ifdef CONFIG_XEN
 #if defined(__i386__) || defined(__x86_64__)
 
 /*
@@ -71,6 +78,7 @@ EXPORT_SYMBOL(machine_power_off);
 EXPORT_SYMBOL(machine_power_off);
 
 #endif /* defined(__i386__) || defined(__x86_64__) */
+#endif /* CONFIG_XEN */
 
 /******************************************************************************
  * Stop/pickle callback handling.
@@ -81,6 +89,7 @@ static void __shutdown_handler(void *unu
 static void __shutdown_handler(void *unused);
 static DECLARE_WORK(shutdown_work, __shutdown_handler, NULL);
 
+#ifdef CONFIG_XEN
 #if defined(__i386__) || defined(__x86_64__)
 
 /* Ensure we run on the idle task page tables so that we will
@@ -210,6 +219,7 @@ static int __do_suspend(void *ignore)
 
        return err;
 }
+#endif /* CONFIG_XEN */
 
 static int shutdown_process(void *__unused)
 {
@@ -222,12 +232,17 @@ static int shutdown_process(void *__unus
 
        if ((shutting_down == SHUTDOWN_POWEROFF) ||
            (shutting_down == SHUTDOWN_HALT)) {
+#ifdef CONFIG_XEN
                if (execve("/sbin/poweroff", poweroff_argv, envp) < 0) {
                        sys_reboot(LINUX_REBOOT_MAGIC1,
                                   LINUX_REBOOT_MAGIC2,
                                   LINUX_REBOOT_CMD_POWER_OFF,
                                   NULL);
                }
+#else /* !CONFIG_XEN */
+               call_usermodehelper_keys("/sbin/poweroff", poweroff_argv, envp, 
NULL, 0);
+
+#endif /* !CONFIG_XEN */
        }
 
        shutting_down = SHUTDOWN_INVALID; /* could try again */
@@ -235,6 +250,7 @@ static int shutdown_process(void *__unus
        return 0;
 }
 
+#ifdef CONFIG_XEN
 static int kthread_create_on_cpu(int (*f)(void *arg),
                                 void *arg,
                                 const char *name,
@@ -248,17 +264,24 @@ static int kthread_create_on_cpu(int (*f
        wake_up_process(p);
        return 0;
 }
+#endif /* CONFIG_XEN */
 
 static void __shutdown_handler(void *unused)
 {
        int err;
 
+#ifdef CONFIG_XEN
        if (shutting_down != SHUTDOWN_SUSPEND)
                err = kernel_thread(shutdown_process, NULL,
                                    CLONE_FS | CLONE_FILES);
        else
                err = kthread_create_on_cpu(__do_suspend, NULL, "suspend", 0);
 
+#else /* !CONFIG_XEN */
+       err = kernel_thread(shutdown_process, NULL,
+                           CLONE_FS | CLONE_FILES);
+#endif /* !CONFIG_XEN */
+
        if (err < 0) {
                printk(KERN_WARNING "Error creating shutdown process (%d): "
                       "retrying...\n", -err);
@@ -272,6 +295,9 @@ static void shutdown_handler(struct xenb
        char *str;
        struct xenbus_transaction xbt;
        int err;
+#ifndef CONFIG_XEN
+       int cad_pid = 1; 
+#endif /* !CONFIG_XEN */
 
        if (shutting_down != SHUTDOWN_INVALID)
                return;
@@ -298,7 +324,11 @@ static void shutdown_handler(struct xenb
        if (strcmp(str, "poweroff") == 0)
                shutting_down = SHUTDOWN_POWEROFF;
        else if (strcmp(str, "reboot") == 0)
+#ifdef CONFIG_XEN  
                ctrl_alt_del();
+#else /* !CONFIG_XEN */
+               kill_proc(cad_pid, SIGINT, 1);
+#endif /* !CONFIG_XEN */
        else if (strcmp(str, "suspend") == 0)
                shutting_down = SHUTDOWN_SUSPEND;
        else if (strcmp(str, "halt") == 0)
@@ -374,10 +404,27 @@ static int setup_shutdown_watcher(struct
 
 static int __init setup_shutdown_event(void)
 {
+#ifndef CONFIG_XEN
+       int err;
+       struct xenbus_transaction xbt;
+#endif /* !CONFIG_XEN */
+
        static struct notifier_block xenstore_notifier = {
                .notifier_call = setup_shutdown_watcher
        };
        register_xenstore_notifier(&xenstore_notifier);
+#ifndef CONFIG_XEN
+ again:
+       err = xenbus_transaction_start(&xbt);
+       if (err)
+               return -1;
+       xenbus_write(xbt, "control", "reboot_module", "installed");
+
+       err = xenbus_transaction_end(xbt, 0);
+       if (err == -EAGAIN) {
+               goto again;
+       }
+#endif /* !CONFIG_XEN */
        return 0;
 }
 
diff -r 38f9bd7a4ce6 tools/python/xen/xend/image.py
--- a/tools/python/xen/xend/image.py    Tue Oct 03 11:39:22 2006 +0100
+++ b/tools/python/xen/xend/image.py    Tue Oct 10 15:06:27 2006 +0900
@@ -281,6 +281,7 @@ class HVMImageHandler(ImageHandler):
         log.debug("apic           = %d", self.apic)
 
         self.register_shutdown_watch()
+        self.register_reboot_module_watch()
 
         return xc.hvm_build(dom            = self.vm.getDomid(),
                             image          = self.kernel,
@@ -383,6 +384,7 @@ class HVMImageHandler(ImageHandler):
 
     def destroy(self):
         self.unregister_shutdown_watch();
+        self.unregister_reboot_module_watch();
         import signal
         if not self.pid:
             return
@@ -425,6 +427,39 @@ class HVMImageHandler(ImageHandler):
                 vm.refreshShutdown(vm.info)
 
         return 1 # Keep watching
+
+    def register_reboot_module_watch(self):
+        """ add xen store watch on control/reboot_module """
+        self.rebootModuleWatch = xswatch(self.vm.dompath + 
"/control/reboot_module", \
+                                    self.hvm_reboot_module)
+        log.debug("hvm reboot module watch registered")
+
+    def unregister_reboot_module_watch(self):
+        """Remove the watch on the control/reboot_module, if any. Nothrow
+        guarantee."""
+
+        try:
+            if self.rebootModuleWatch:
+                self.rebootModuleWatch.unwatch()
+        except:
+            log.exception("Unwatching hvm reboot module watch failed.")
+        self.rebootModuleWatch = None
+        log.debug("hvm reboot module watch unregistered")
+
+    def hvm_reboot_module(self, _):
+        """ watch call back on node control/reboot_module,
+            if node changed, this function will be called
+        """
+        xd = xen.xend.XendDomain.instance()
+        vm = xd.domain_lookup( self.vm.getDomid() )
+
+        reboot_module_status = vm.readDom('control/reboot_module')
+        log.debug("hvm_reboot_module fired, module status=%s", 
reboot_module_status)
+        if reboot_module_status == 'installed':
+            self.unregister_shutdown_watch()
+
+        return 1 # Keep watching
+
 
 class IA64_HVM_ImageHandler(HVMImageHandler):
 
diff -r 38f9bd7a4ce6 unmodified_drivers/linux-2.6/Makefile
--- a/unmodified_drivers/linux-2.6/Makefile     Tue Oct 03 11:39:22 2006 +0100
+++ b/unmodified_drivers/linux-2.6/Makefile     Tue Oct 10 15:06:27 2006 +0900
@@ -4,3 +4,4 @@ obj-m += xenbus/
 obj-m += xenbus/
 obj-m += blkfront/
 obj-m += netfront/
+obj-m += util/
diff -r 38f9bd7a4ce6 unmodified_drivers/linux-2.6/mkbuildtree
--- a/unmodified_drivers/linux-2.6/mkbuildtree  Tue Oct 03 11:39:22 2006 +0100
+++ b/unmodified_drivers/linux-2.6/mkbuildtree  Tue Oct 10 15:06:27 2006 +0900
@@ -14,6 +14,7 @@ ln -sf ${XL}/drivers/xen/core/gnttab.c p
 ln -sf ${XL}/drivers/xen/core/gnttab.c platform-pci
 ln -sf ${XL}/drivers/xen/core/features.c platform-pci
 ln -sf ${XL}/drivers/xen/core/xen_proc.c xenbus
+ln -sf ${XL}/drivers/xen/core/reboot.c util
 
 mkdir -p include
 mkdir -p include/xen
diff -r 38f9bd7a4ce6 unmodified_drivers/linux-2.6/util/Kbuild
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/unmodified_drivers/linux-2.6/util/Kbuild  Tue Oct 10 15:11:29 2006 +0900
@@ -0,0 +1,3 @@
+include $(M)/overrides.mk
+
+obj-m := reboot.o
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.