Re: kexec trouble

[Gerd Hoffmann <kraxel@suse.de>]

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 <kraxel@suse.de>