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
|