From fcde973e26c585ade21bb16dd13df5205a4f0998 Mon Sep 17 00:00:00 2001 From: "kaf24@firebug.cl.cam.ac.uk" Date: Tue, 15 Nov 2005 18:43:28 +0100 Subject: [PATCH 1/8] Ensure we initialise the cpu_present_map before topology_init() is called. In latest Linux kernels it iterates over cpu_present_map rather than cpu_possible_map. Also, save/restore should really iterate over possible cpus, not present ones (not that it really matters, but it's a small cleanup). Signed-off-by: Keir Fraser --- linux-2.6-xen-sparse/arch/xen/kernel/reboot.c | 2 +- linux-2.6-xen-sparse/arch/xen/kernel/smpboot.c | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/linux-2.6-xen-sparse/arch/xen/kernel/reboot.c b/linux-2.6-xen-sparse/arch/xen/kernel/reboot.c index 89c2005450..81f0f10c21 100644 --- a/linux-2.6-xen-sparse/arch/xen/kernel/reboot.c +++ b/linux-2.6-xen-sparse/arch/xen/kernel/reboot.c @@ -188,7 +188,7 @@ static int __do_suspend(void *ignore) xenbus_resume(); #ifdef CONFIG_SMP - for_each_present_cpu(i) + for_each_cpu(i) vcpu_prepare(i); out_reenable_cpus: diff --git a/linux-2.6-xen-sparse/arch/xen/kernel/smpboot.c b/linux-2.6-xen-sparse/arch/xen/kernel/smpboot.c index 31ea6473f1..66bb891ec6 100644 --- a/linux-2.6-xen-sparse/arch/xen/kernel/smpboot.c +++ b/linux-2.6-xen-sparse/arch/xen/kernel/smpboot.c @@ -277,6 +277,18 @@ void __devinit smp_prepare_boot_cpu(void) #ifdef CONFIG_HOTPLUG_CPU +/* + * Initialize cpu_present_map late to skip SMP boot code in init/main.c. + * But do it early enough to catch critical for_each_present_cpu() loops + * in i386-specific code. + */ +static int __init initialize_cpu_present_map(void) +{ + cpu_present_map = cpu_possible_map; + return 0; +} +core_initcall(initialize_cpu_present_map); + static void vcpu_hotplug(unsigned int cpu) { int err; @@ -293,7 +305,6 @@ static void vcpu_hotplug(unsigned int cpu) } if (strcmp(state, "online") == 0) { - cpu_set(cpu, cpu_present_map); (void)cpu_up(cpu); } else if (strcmp(state, "offline") == 0) { (void)cpu_down(cpu); From 9773f5386e655c7c6fb42fffc1b948d8c1595342 Mon Sep 17 00:00:00 2001 From: "emellor@leeni.uk.xensource.com" Date: Tue, 15 Nov 2005 18:47:39 +0100 Subject: [PATCH 2/8] Remove the handling of TypeError inside gather. It is not thrown by int() when given a non-convertible value, as claimed by the comment, as the exception ValueError is thrown in that situation. TypeError is only thrown on int(None), which cannot actually take place in this code, because of the explicit check for None. Signed-off-by: Ewan Mellor --- tools/python/xen/xend/xenstore/xstransact.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/tools/python/xen/xend/xenstore/xstransact.py b/tools/python/xen/xend/xenstore/xstransact.py index ac06c2b1b8..dcdad54a22 100644 --- a/tools/python/xen/xend/xenstore/xstransact.py +++ b/tools/python/xen/xend/xenstore/xstransact.py @@ -177,18 +177,15 @@ class xstransact: (key, fn, defval) = tup val = self._read(key) - # If fn is str, then this will successfully convert None to - # 'None'. If it is int, then it will throw TypeError on None, or - # on any other non-integer value. We have to, therefore, both - # check explicitly for None, and catch TypeError. Either failure - # will result in defval being used instead. + # If fn is str, then this will successfully convert None to 'None' + # (which we don't want). If it is int or float, then it will + # throw ValueError on any non-convertible value. We check + # explicitly for None, using defval instead, but allow ValueError + # to propagate. if val is None: val = defval else: - try: - val = fn(val) - except TypeError: - val = defval + val = fn(val) ret.append(val) if len(ret) == 1: return ret[0] From 5d0f388f77e52674f41d87c1e50ba7578a292ce6 Mon Sep 17 00:00:00 2001 From: "emellor@leeni.uk.xensource.com" Date: Tue, 15 Nov 2005 19:08:11 +0100 Subject: [PATCH 3/8] Have a watch callback return 0 or 1 depending upon whether it would like to continue to receive watches. This means that it is not necessary to probe around in the xswatch internals to unregister a watch. Tidy up the hotplug watch handling, moving the nested function out to a separate place (I don't think that this was a problem in the end, but it was making me nervous as I was debugging the recent race condition, and I find it clearer out of the class in any case. Pass an integer code representing hotplug status, once it has been parsed from the store, as there are now a few different places we can detect failure, and it's cleaner to pass a code from those places. Small tidy up to XendDomain, removing the unused releaseDomain field. --- tools/python/xen/xend/XendDomain.py | 11 ++- tools/python/xen/xend/server/DevController.py | 87 ++++++++++++------- tools/python/xen/xend/xenstore/xswatch.py | 27 +++--- 3 files changed, 76 insertions(+), 49 deletions(-) diff --git a/tools/python/xen/xend/XendDomain.py b/tools/python/xen/xend/XendDomain.py index a429852788..a087b2525a 100644 --- a/tools/python/xen/xend/XendDomain.py +++ b/tools/python/xen/xend/XendDomain.py @@ -36,6 +36,7 @@ from xen.xend import XendCheckpoint from xen.xend.XendError import XendError from xen.xend.XendLogging import log from xen.xend.server import relocate +from xen.xend.xenstore.xswatch import xswatch xc = xen.lowlevel.xc.new() @@ -58,9 +59,11 @@ class XendDomain: # to import XendDomain from XendDomainInfo causes unbounded recursion. # So we stuff the XendDomain instance (self) into xroot's components. xroot.add_component("xen.xend.XendDomain", self) + self.domains = {} self.domains_lock = threading.RLock() - self.watchReleaseDomain() + + xswatch("@releaseDomain", self.onReleaseDomain) self.domains_lock.acquire() try: @@ -112,11 +115,7 @@ class XendDomain: self.refresh() finally: self.domains_lock.release() - - - def watchReleaseDomain(self): - from xen.xend.xenstore.xswatch import xswatch - self.releaseDomain = xswatch("@releaseDomain", self.onReleaseDomain) + return 1 def xen_domains(self): diff --git a/tools/python/xen/xend/server/DevController.py b/tools/python/xen/xend/server/DevController.py index b48ba032e3..36226d7c9a 100644 --- a/tools/python/xen/xend/server/DevController.py +++ b/tools/python/xen/xend/server/DevController.py @@ -29,6 +29,12 @@ DEVICE_CREATE_TIMEOUT = 5 HOTPLUG_STATUS_NODE = "hotplug-status" HOTPLUG_STATUS_ERROR = "error" +Connected = 1 +Died = 2 +Error = 3 +Missing = 4 +Timeout = 5 + xenbusState = { 'Unknown' : 0, 'Initialising' : 1, @@ -87,18 +93,28 @@ class DevController: def waitForDevice(self, devid): log.debug("Waiting for %s.", devid) - status, fn_ret = self.waitForBackend(devid) - if status: - self.destroyDevice(devid) - raise VmError( ("Device %s (%s) could not be connected. " - "Hotplug scripts not working") - % (devid, self.deviceClass)) + status = self.waitForBackend(devid) - elif fn_ret == HOTPLUG_STATUS_ERROR: + if status == Timeout: self.destroyDevice(devid) - raise VmError( ("Device %s (%s) could not be connected. " - "Backend device not found!") - % (devid, self.deviceClass)) + raise VmError("Device %s (%s) could not be connected. " + "Hotplug scripts not working" % + (devid, self.deviceClass)) + + elif status == Error: + self.destroyDevice(devid) + raise VmError("Device %s (%s) could not be connected. " + "Backend device not found" % + (devid, self.deviceClass)) + + elif status == Missing: + raise VmError("Device %s (%s) could not be connected. " + "Device not found" % (devid, self.deviceClass)) + + elif status == Died: + self.destroyDevice(devid) + raise VmError("Device %s (%s) could not be connected. " + "Device has died" % (devid, self.deviceClass)) def reconfigureDevice(self, devid, config): @@ -302,35 +318,22 @@ class DevController: raise - def waitForBackend(self,devid): - ev = Event() + def waitForBackend(self, devid): - def hotplugStatus(): - log.debug("hotplugStatus %d", devid) - - try: - status = self.readBackend(devid, HOTPLUG_STATUS_NODE) - except VmError: - status = "died" - if status is not None: - watch.xs.unwatch(backpath, watch) - hotplugStatus.value = status - ev.set() - - hotplugStatus.value = None frontpath = self.frontendPath(devid) backpath = xstransact.Read(frontpath, "backend") if backpath: - watch = xswatch(backpath, hotplugStatus) + statusPath = backpath + '/' + HOTPLUG_STATUS_NODE + ev = Event() + result = { 'status': Timeout } + + xswatch(statusPath, hotplugStatusCallback, statusPath, ev, result) ev.wait(DEVICE_CREATE_TIMEOUT) - if ev.isSet(): - return (0, hotplugStatus.value) - else: - return (-1, hotplugStatus.value) + return result['status'] else: - return (-1, "missing") + return Missing def backendPath(self, backdom, devid): @@ -352,3 +355,25 @@ class DevController: def frontendMiscPath(self): return "%s/device-misc/%s" % (self.vm.getDomainPath(), self.deviceClass) + + +def hotplugStatusCallback(statusPath, ev, result): + log.debug("hotplugStatusCallback %s.", statusPath) + + try: + status = xstransact.Read(statusPath) + + if status is not None: + if status == HOTPLUG_STATUS_ERROR: + result['status'] = Error + else: + result['status'] = Connected + else: + return 1 + except VmError: + result['status'] = Died + + log.debug("hotplugStatusCallback %d.", result['status']) + + ev.set() + return 0 diff --git a/tools/python/xen/xend/xenstore/xswatch.py b/tools/python/xen/xend/xenstore/xswatch.py index 3fc0f75645..b087aeacc5 100644 --- a/tools/python/xen/xend/xenstore/xswatch.py +++ b/tools/python/xen/xend/xenstore/xswatch.py @@ -5,9 +5,7 @@ # Public License. See the file "COPYING" in the main directory of # this archive for more details. -import select import threading -from xen.lowlevel import xs from xen.xend.xenstore.xsutil import xshandle from xen.xend.XendLogging import log @@ -20,37 +18,42 @@ class xswatch: xslock = threading.Lock() def __init__(self, path, fn, *args, **kwargs): + self.path = path self.fn = fn self.args = args self.kwargs = kwargs xswatch.watchStart() xswatch.xs.watch(path, self) + def watchStart(cls): cls.xslock.acquire() - if cls.watchThread: + try: + if cls.watchThread: + return + cls.xs = xshandle() + cls.watchThread = threading.Thread(name="Watcher", + target=cls.watchMain) + cls.watchThread.setDaemon(True) + cls.watchThread.start() + finally: cls.xslock.release() - return - cls.xs = xshandle() - cls.watchThread = threading.Thread(name="Watcher", - target=cls.watchMain) - cls.watchThread.setDaemon(True) - cls.watchThread.start() - cls.xslock.release() watchStart = classmethod(watchStart) + def watchMain(cls): while True: try: we = cls.xs.read_watch() watch = we[1] - watch.fn(*watch.args, **watch.kwargs) + res = watch.fn(*watch.args, **watch.kwargs) + if not res: + cls.xs.unwatch(watch.path, watch) except: log.exception("read_watch failed") # Ignore this exception -- there's no point throwing it # further on because that will just kill the watcher thread, # which achieves nothing. - watchMain = classmethod(watchMain) From 63852bee4735407c260a034cac29ee1a8f100300 Mon Sep 17 00:00:00 2001 From: "emellor@leeni.uk.xensource.com" Date: Tue, 15 Nov 2005 19:19:02 +0100 Subject: [PATCH 4/8] Make watchStart and watchMain global functions rather than classmethods, meaning that we no longer need to prefix all the field accesses with cls. Signed-off-by: Ewan Mellor --- tools/python/xen/xend/xenstore/xswatch.py | 70 +++++++++++------------ 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/tools/python/xen/xend/xenstore/xswatch.py b/tools/python/xen/xend/xenstore/xswatch.py index b087aeacc5..fbf165c019 100644 --- a/tools/python/xen/xend/xenstore/xswatch.py +++ b/tools/python/xen/xend/xenstore/xswatch.py @@ -13,47 +13,45 @@ from xen.xend.XendLogging import log class xswatch: - watchThread = None - xs = None - xslock = threading.Lock() - def __init__(self, path, fn, *args, **kwargs): self.path = path self.fn = fn self.args = args self.kwargs = kwargs - xswatch.watchStart() - xswatch.xs.watch(path, self) + watchStart() + xs.watch(path, self) - def watchStart(cls): - cls.xslock.acquire() +watchThread = None +xs = None +xslock = threading.Lock() + +def watchStart(): + global watchThread + global xs + + xslock.acquire() + try: + if watchThread: + return + xs = xshandle() + watchThread = threading.Thread(name="Watcher", target=watchMain) + watchThread.setDaemon(True) + watchThread.start() + finally: + xslock.release() + + +def watchMain(): + while True: try: - if cls.watchThread: - return - cls.xs = xshandle() - cls.watchThread = threading.Thread(name="Watcher", - target=cls.watchMain) - cls.watchThread.setDaemon(True) - cls.watchThread.start() - finally: - cls.xslock.release() - - watchStart = classmethod(watchStart) - - - def watchMain(cls): - while True: - try: - we = cls.xs.read_watch() - watch = we[1] - res = watch.fn(*watch.args, **watch.kwargs) - if not res: - cls.xs.unwatch(watch.path, watch) - except: - log.exception("read_watch failed") - # Ignore this exception -- there's no point throwing it - # further on because that will just kill the watcher thread, - # which achieves nothing. - - watchMain = classmethod(watchMain) + we = xs.read_watch() + watch = we[1] + res = watch.fn(*watch.args, **watch.kwargs) + if not res: + xs.unwatch(watch.path, watch) + except: + log.exception("read_watch failed") + # Ignore this exception -- there's no point throwing it + # further on because that will just kill the watcher thread, + # which achieves nothing. From 3741b397bda9822d589428214110330cf1c143de Mon Sep 17 00:00:00 2001 From: "kaf24@firebug.cl.cam.ac.uk" Date: Tue, 15 Nov 2005 19:46:48 +0100 Subject: [PATCH 5/8] Fix 64-bit build that I broke by making acpi table mapping fixup dependent on CONFIG_XEN_X86 (that only xen/i386 defines). Signed-off-by: Keir Fraser --- linux-2.6-xen-sparse/drivers/acpi/tables.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux-2.6-xen-sparse/drivers/acpi/tables.c b/linux-2.6-xen-sparse/drivers/acpi/tables.c index 59172d3e9d..a849e65c85 100644 --- a/linux-2.6-xen-sparse/drivers/acpi/tables.c +++ b/linux-2.6-xen-sparse/drivers/acpi/tables.c @@ -565,7 +565,7 @@ acpi_table_get_sdt ( * * result: sdt_entry[] is initialized */ -#ifdef CONFIG_XEN_X86 +#if defined(CONFIG_XEN_X86) || defined(CONFIG_XEN_X86_64) #define acpi_rsdp_phys_to_va(rsdp_phys) (__fix_to_virt(FIX_ACPI_RSDP_PAGE) + \ (rsdp_phys & ~PAGE_MASK)) #else From b30b10f57a08a690304375c56a52db04118a1916 Mon Sep 17 00:00:00 2001 From: "kaf24@firebug.cl.cam.ac.uk" Date: Wed, 16 Nov 2005 11:27:19 +0100 Subject: [PATCH 6/8] Replace 0 with NULL when assigning pointer fields/parameters. Signed-off-by: Vincent Hanquez --- linux-2.6-xen-sparse/arch/xen/kernel/evtchn.c | 4 ++-- linux-2.6-xen-sparse/drivers/char/tpm/tpm_xen.c | 2 +- linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c | 2 +- linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/linux-2.6-xen-sparse/arch/xen/kernel/evtchn.c b/linux-2.6-xen-sparse/arch/xen/kernel/evtchn.c index 728c275117..0f516e5373 100644 --- a/linux-2.6-xen-sparse/arch/xen/kernel/evtchn.c +++ b/linux-2.6-xen-sparse/arch/xen/kernel/evtchn.c @@ -752,7 +752,7 @@ void __init init_IRQ(void) irq_bindcount[dynirq_to_irq(i)] = 0; irq_desc[dynirq_to_irq(i)].status = IRQ_DISABLED; - irq_desc[dynirq_to_irq(i)].action = 0; + irq_desc[dynirq_to_irq(i)].action = NULL; irq_desc[dynirq_to_irq(i)].depth = 1; irq_desc[dynirq_to_irq(i)].handler = &dynirq_type; } @@ -770,7 +770,7 @@ void __init init_IRQ(void) #endif irq_desc[pirq_to_irq(i)].status = IRQ_DISABLED; - irq_desc[pirq_to_irq(i)].action = 0; + irq_desc[pirq_to_irq(i)].action = NULL; irq_desc[pirq_to_irq(i)].depth = 1; irq_desc[pirq_to_irq(i)].handler = &pirq_type; } diff --git a/linux-2.6-xen-sparse/drivers/char/tpm/tpm_xen.c b/linux-2.6-xen-sparse/drivers/char/tpm/tpm_xen.c index 1c6de43b45..ffb9cf2083 100644 --- a/linux-2.6-xen-sparse/drivers/char/tpm/tpm_xen.c +++ b/linux-2.6-xen-sparse/drivers/char/tpm/tpm_xen.c @@ -438,7 +438,7 @@ static struct attribute* xen_attrs[] = { &dev_attr_pcrs.attr, &dev_attr_caps.attr, &dev_attr_cancel.attr, - 0, + NULL, }; static struct attribute_group xen_attr_grp = { .attrs = xen_attrs }; diff --git a/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c b/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c index a179690a0b..34f06a1d86 100644 --- a/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c +++ b/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c @@ -543,7 +543,7 @@ static int __init blkif_init(void) spin_lock_init(&blkio_schedule_list_lock); INIT_LIST_HEAD(&blkio_schedule_list); - ret = kernel_thread(blkio_schedule, 0, CLONE_FS | CLONE_FILES); + ret = kernel_thread(blkio_schedule, NULL, CLONE_FS | CLONE_FILES); BUG_ON(ret < 0); blkif_xenbus_init(); diff --git a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c index 230f379cdd..5b20cb8f24 100644 --- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c +++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c @@ -884,7 +884,7 @@ void xenbus_probe(void *unused) register_xenbus_watch(&be_watch); /* Notify others that xenstore is up */ - notifier_call_chain(&xenstore_chain, 0, 0); + notifier_call_chain(&xenstore_chain, 0, NULL); } From 7448bc78c522caadf9d725cdde01282ad510354d Mon Sep 17 00:00:00 2001 From: "kaf24@firebug.cl.cam.ac.uk" Date: Wed, 16 Nov 2005 11:28:12 +0100 Subject: [PATCH 7/8] Add function declarations, and ensure we prevent mismatch of function definition vs. declaration. Signed-off-by: Vincent Hanquez --- linux-2.6-xen-sparse/arch/xen/kernel/xen_proc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/linux-2.6-xen-sparse/arch/xen/kernel/xen_proc.c b/linux-2.6-xen-sparse/arch/xen/kernel/xen_proc.c index f66099a031..050deec580 100644 --- a/linux-2.6-xen-sparse/arch/xen/kernel/xen_proc.c +++ b/linux-2.6-xen-sparse/arch/xen/kernel/xen_proc.c @@ -1,6 +1,7 @@ #include #include +#include static struct proc_dir_entry *xen_base; From 119496c00d39110b3597957a01ae69957295dc4f Mon Sep 17 00:00:00 2001 From: "kaf24@firebug.cl.cam.ac.uk" Date: Wed, 16 Nov 2005 11:29:52 +0100 Subject: [PATCH 8/8] Trivial coding style fix. Signed-off-by: Vincent Hanquez --- linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c index 5b20cb8f24..fd49ca898f 100644 --- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c +++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c @@ -854,7 +854,7 @@ int register_xenstore_notifier(struct notifier_block *nb) { int ret = 0; - if(xenstored_ready > 0) + if (xenstored_ready > 0) ret = nb->notifier_call(nb, 0, NULL); else notifier_chain_register(&xenstore_chain, nb);