[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] vl: Fix to create migration object before block
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] vl: Fix to create migration object before block backends again |
Date: |
Tue, 26 Mar 2019 18:48:54 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Anthony PERARD <address@hidden> writes:
> On Tue, Mar 26, 2019 at 03:58:52PM +0000, Anthony PERARD wrote:
>> On Wed, Mar 13, 2019 at 09:43:30AM +0100, Markus Armbruster wrote:
>> > Recent commit cda4aa9a5a0 moved block backend creation before machine
>> > property evaluation. This broke qemu-iotests 055. Turns out we need
>> > to create the migration object before block backends, so block
>> > backends can add migration blockers. Fix by calling
>> > migration_object_init() earlier, right before configure_blockdev().
>> >
>> > Fixes: cda4aa9a5a08777cf13e164c0543bd4888b8adce
>> > Reported-by: Kevin Wolf <address@hidden>
>> > Signed-off-by: Markus Armbruster <address@hidden>
>>
>> Hi,
>>
>> After this patch is applied, migration with Xen doesn't work anymore.
>> When QEMU on the receiving end is started, it prints:
>> qemu-system-i386: load of migration failed: Invalid argument
>
> I should have quote this from QEMU stderr instead of the single line
> abrove:
> qemu-system-i386: Configuration section missing
> qemu-system-i386: load of migration failed: Invalid argument
>
>> > + /*
>> > + * Migration object can only be created after global properties
>> > + * are applied correctly.
>> > + */
>> > + migration_object_init();
>>
>> I think it's because migration_object_init() is now called before
>> configure_accelerator(). xen_accel_class_init() have some compat_props
>> for migration.
I see.
Anything that gets created before a certain compat property becomes
known silently ignores the compat property[*].
Accelerator compat properties become known when we select the
accelerator. This is awfully late, because we want to pick the first
accelerator that initializes, which requires a machine.
>> Any idea how to fix this?
Known dependencies:
* migration object_init() must run before configure_blockdev(), so block
backends can register migration blockers
* configure_blockdev() must run before machine_set_property(), so
machine properties can refer to block backends.
* configure_accelerator() must run after machine creation, because it
needs a machine
* configure_accelerator() must run before migration_object_init(), so
migration_object_init() can see accelerator compat properties.
We're currently violating the last one, because weren't aware of it. To
fix that, rejigger some more: move configure_accelerator() before
migration_object_init() (with a suitable comment), then see what else
breaks. Could you try the appended patch and report whether it fixes
your problem?
[*] Silent ignore is nasty. I got experimental code to turn it into a
hard failure.
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]);
+
/*
* 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);