Re: [Qemu-devel] [PATCH] vl: Fix to create migration object before block backends again
Anthony PERARD writes: > On Tue, Mar 26, 2019 at 06:48:54PM +0100, Markus Armbruster wrote: >> 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? > > Yes, that patch works and doesn't break anything else with Xen. Thanks! I posted this fix together with another one as "[PATCH 0/2] Compat props bug fixes". I fogot to add your Tested-by, sorry.
Re: [Qemu-devel] [PATCH] vl: Fix to create migration object before block backends again
On Tue, Mar 26, 2019 at 06:48:54PM +0100, Markus Armbruster wrote:
> 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?
Yes, that patch works and doesn't break anything else with Xen.
> 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);
--
Anthony PERARD
Re: [Qemu-devel] [PATCH] vl: Fix to create migration object before block backends again
Anthony PERARD writes:
> On Tue, Mar 26, 2019 at 03:58:52PM +, 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
>> > Signed-off-by: Markus Armbruster
>>
>> 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);
Re: [Qemu-devel] [PATCH] vl: Fix to create migration object before block backends again
On Tue, Mar 26, 2019 at 03:58:52PM +, 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 > > Signed-off-by: Markus Armbruster > > 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. > > Any idea how to fix this? -- Anthony PERARD
Re: [Qemu-devel] [PATCH] vl: Fix to create migration object before block backends again
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 > Signed-off-by: Markus Armbruster 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 > +/* > + * 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. Any idea how to fix this? Thanks, -- Anthony PERARD
Re: [Qemu-devel] [PATCH] vl: Fix to create migration object before block backends again
Am 13.03.2019 um 09:43 hat Markus Armbruster geschrieben: > 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 > Signed-off-by: Markus Armbruster Thanks, applied to the block branch. Kevin
[Qemu-devel] [PATCH] vl: Fix to create migration object before block backends again
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
Signed-off-by: Markus Armbruster
---
vl.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/vl.c b/vl.c
index 027b853d92..b3e48eff27 100644
--- a/vl.c
+++ b/vl.c
@@ -4276,10 +4276,17 @@ int main(int argc, char **argv, char **envp)
exit(0);
}
+/*
+ * Migration object can only be created after global properties
+ * are applied correctly.
+ */
+migration_object_init();
+
/*
* Note: we need to create block backends before
* machine_set_property(), so machine properties can refer to
- * them.
+ * them, and after migration_object_init(), so we can create
+ * migration blockers.
*/
configure_blockdev(_queue, machine_class, snapshot);
@@ -4297,12 +4304,6 @@ int main(int argc, char **argv, char **envp)
machine_class->name, machine_class->deprecation_reason);
}
-/*
- * Migration object can only be created after global properties
- * are applied correctly.
- */
-migration_object_init();
-
if (qtest_chrdev) {
qtest_init(qtest_chrdev, qtest_log, _fatal);
}
--
2.17.2
[Qemu-devel] [PATCH] vl: Fix to create migration object before block backends again
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
Signed-off-by: Markus Armbruster
---
vl.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/vl.c b/vl.c
index 027b853d92..b3e48eff27 100644
--- a/vl.c
+++ b/vl.c
@@ -4276,10 +4276,17 @@ int main(int argc, char **argv, char **envp)
exit(0);
}
+/*
+ * Migration object can only be created after global properties
+ * are applied correctly.
+ */
+migration_object_init();
+
/*
* Note: we need to create block backends before
* machine_set_property(), so machine properties can refer to
- * them.
+ * them, and after migration_object_init(), so we can create
+ * migration blockers.
*/
configure_blockdev(_queue, machine_class, snapshot);
@@ -4297,12 +4304,6 @@ int main(int argc, char **argv, char **envp)
machine_class->name, machine_class->deprecation_reason);
}
-/*
- * Migration object can only be created after global properties
- * are applied correctly.
- */
-migration_object_init();
-
if (qtest_chrdev) {
qtest_init(qtest_chrdev, qtest_log, _fatal);
}
--
2.17.2
Re: [Qemu-devel] [PATCH] vl: Fix to create migration object before block backends again
Butterfingers... Please use Message-Id: <20190313084330.16015-1-arm...@redhat.com> and ignore this one.
