On Fri, 29 Mar 2019 15:34:59 +0100 Markus Armbruster <[email protected]> wrote:
> Igor Mammedov <[email protected]> writes: > > > On Wed, 27 Mar 2019 16:03:46 +0100 > > Markus Armbruster <[email protected]> wrote: > > > >> Recent commit cda4aa9a5a0 moved block backend creation before machine > >> property evaluation. This broke block backends registering migration > >> blockers. Commit e60483f2f84 fixed it by moving migration object > >> creation before block backend creation. This broke migration with > >> Xen. Turns out we need to configure the accelerator before we create > >> the migration object so that Xen's accelerator compat properties get > >> applied. Fix by calling configure_accelerator() earlier, right before > >> migration_object_init(). > >> > >> Fixes: e60483f2f8498ae08ae79ca4c6fb03a3317f5e1e > >> Reported-by: Anthony PERARD <[email protected]> > >> Signed-off-by: Markus Armbruster <[email protected]> > >> --- > >> vl.c | 8 ++++++-- > >> 1 file changed, 6 insertions(+), 2 deletions(-) > >> > >> diff --git a/vl.c b/vl.c > >> index d61d5604e5..28b9e9a170 100644 > >> --- a/vl.c > >> +++ b/vl.c > >> @@ -4276,6 +4276,12 @@ int main(int argc, char **argv, char **envp) > >> exit(0); > >> } > >> > >> + /* > >> + * Must run before migration_object_init() to make Xen's > >> + * accelerator compat properties stick > >> + */ > >> + configure_accelerator(current_machine, argv[0]); > >> + > > Setting machine properties with this change happens after > > accelerator was initialized. > > > > It probably will break initialization of accelerator features > > (to name a couple kernel-irqchip, memory-encryption) that depend > > on machine properties being set before accelerator is initialized. > > I'm blind. Can you suggest a potential reproducer for me to try? sure (it' difficult to move things around in vl.c and spot implicit dependency or not to break something while at it) diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index 241db49..9b0d89a 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -1724,6 +1724,7 @@ static int kvm_init(MachineState *ms) } if (machine_kernel_irqchip_allowed(ms)) { +fprintf(stderr, "kvm_irqchip_create\n"); kvm_irqchip_create(ms, s); } with: qemu-system-x86_64 -machine accel=kvm,kernel-irqchip=off prints "kvm_irqchip_create" after patch and doesn't with current master > > >> /* > >> * Migration object can only be created after global properties > >> * are applied correctly. > >> @@ -4297,8 +4303,6 @@ int main(int argc, char **argv, char **envp) > >> current_machine->maxram_size = maxram_size; > >> current_machine->ram_slots = ram_slots; > >> > >> - configure_accelerator(current_machine, argv[0]); > >> - > >> if (!qtest_enabled() && machine_class->deprecation_reason) { > >> error_report("Machine type '%s' is deprecated: %s", > >> machine_class->name, > >> machine_class->deprecation_reason);
