Re: [kvm-unit-tests PATCH 0/3] check multiple init for vm/smp

[Peter Xu <peterx@redhat.com>]

On Tue, Oct 25, 2016 at 12:12:47PM +0200, Andrew Jones wrote:
> On Tue, Oct 25, 2016 at 05:00:27PM +0800, Peter Xu wrote:
> > The first two patches check whether there are multiple init for
> > vm/smp, assertion fail will be triggered if so.
> > 
> > Patch 3 is to cleanup existing setup_igt() when smp_init() is called
> > (suggested by Drew).
> > 
> > Peter Xu (3):
> >   x86: vm: add assert to avoid multiple vm init
> >   x86: smp: assert to avoid multiple init of smp
> >   tests: don't call setup_idt() if with smp_init()
> > 
> >  lib/x86/smp.c             | 1 +
> >  lib/x86/vm.c              | 1 +
> >  x86/apic.c                | 1 -
> >  x86/hyperv_stimer.c       | 1 -
> >  x86/hyperv_synic.c        | 1 -
> >  x86/ioapic.c              | 1 -
> >  x86/tscdeadline_latency.c | 1 -
> >  7 files changed, 2 insertions(+), 5 deletions(-)
> > 
> > -- 
> > 2.7.4
> >
> 
> I actually looked at the code this time around and think that only
> patch 1/3 is needed. While it would be pointless to call smp_init
> multiple times, it doesn't hurt (that means 2/3 is OK, but not
> necessary). And, now I see 3/3 is just a cleanup. No assert would
> fire without it. So it's not necessary either.
> 
> Actually, wrt to setup_idt, I'd say the only problems with it is
> that there's the 'if (idt_initialized) return' and that it isn't
> named something like setup_idt_defaults. Isn't it conceivable that
> a unit test may want to revert its handlers back to the defaults
> after installing custom handlers by simply calling this again?

Yeah, that's possible. Maybe we can let anyone who needs this first to
introduce the setup_idt_defaults().

For this series: I have no strong willingness to push this in, and yes
all three patches are not essential at all. I just picked them out
from vt-d unit test series, in case any of us would like to pick it.

Thanks,

-- peterx