[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] vl: Fix to create accel compat props before
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] vl: Fix to create accel compat props before migration object again |
Date: |
Fri, 29 Mar 2019 16:25:26 +0100 |
On Fri, 29 Mar 2019 15:34:59 +0100
Markus Armbruster <address@hidden> wrote:
> Igor Mammedov <address@hidden> writes:
>
> > On Wed, 27 Mar 2019 16:03:46 +0100
> > Markus Armbruster <address@hidden> 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 <address@hidden>
> >> Signed-off-by: Markus Armbruster <address@hidden>
> >> ---
> >> 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);