From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gerd Hoffmann Subject: Re: kexec trouble Date: Wed, 06 Dec 2006 09:48:34 +0100 Message-ID: <457683E2.6000205@suse.de> References: <45758427.9010803@suse.de> <4575A46F.9090001@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Magnus Damm Cc: Magnus Damm , Xen devel list , Horms List-Id: xen-devel@lists.xenproject.org Magnus Damm wrote: > For you a runtime check makes sense, especially now when our code is > merged and you have a conflict. It does however sound like you are > pissed because the conflict, but I don't think you should blame that > on us. Yes, a bit, especially as we've talked a bit about dom0/domU kexec at the Xen Summit, so I assumed you are aware of the problem. The sparse/patches split of the code also makes it hard to change it. > Simon and I reposted the patches at least 10 times over the > last half a year - so you had your time to come with feedback. Yes, I should have checked before. -ENOTIME. Bad decision nevertheless, now it probably costs even more time to fix it up afterwards .... > That aside, what about doing as little as possible now? Use > is_initial_xendomain() or something like that to switch between the > different dom0 and domU implementations. And whenever domU and dom0 > runs under paravirt we fix up to code to remove the #ifdef and add > native mode support. I'd go for the function pointer approach. I think it is easier to maintain in the long run. Wrapper functions which look at is_initial_xendomain() then call either xen0_machine_kexec or xenU_machine_kexec quickly get messy with lots of #ifdef CONFIG_FOOBAR, and it would be a temporary solution only anyway. I think you compile in native code too, although it is dead code, right? So we can make machine_kexec() + friends function pointers, rename the native functions and initialize the function pointers to the native versions. I think it should even be possible to make them function pointers for i386/x86_64 archs only. Things keep working with CONFIG_XEN=n then, and with CONFIG_XEN=y the initialization function just switches the function pointers (depending on is_initial_domain()). This also eliminates the first set of #ifdefs in kernel/kexec.c ;) > Replacing the #ifdefs with a runtime check that is fine by me. I'm > think it's nice to avoid #ifdefs if possible, but again - our scope of > implementation was simply to add dom0 support. We did not care about > domU support or paravirt that wasn't included at that time. Having "#ifdef CONFIG_XEN" in kernel/kexec.c most likely never ever is accepted mainline (and we do seek mainline merge, don't we?). IMHO that is enough reason to avoid it in the first place. cheers, Gerd -- Gerd Hoffmann