[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 16/19] migration: Export tls-[creds|hostname|authz] params
From: |
Peter Xu |
Subject: |
Re: [PATCH v3 16/19] migration: Export tls-[creds|hostname|authz] params to cmdline too |
Date: |
Thu, 31 Mar 2022 11:07:02 -0400 |
On Wed, Mar 30, 2022 at 05:39:05PM -0400, Peter Xu wrote:
> It's useful for specifying tls credentials all in the cmdline (along with
> the -object tls-creds-*), especially for debugging purpose.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> migration/migration.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 899084f993..d99a0ecb7b 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -4349,6 +4349,9 @@ static Property migration_properties[] = {
> DEFAULT_MIGRATE_ANNOUNCE_STEP),
> DEFINE_PROP_BOOL("x-postcopy-preempt-break-huge", MigrationState,
> postcopy_preempt_break_huge, true),
> + DEFINE_PROP_STRING("tls-creds", MigrationState, parameters.tls_creds),
> + DEFINE_PROP_STRING("tls-hostname", MigrationState,
> parameters.tls_hostname),
> + DEFINE_PROP_STRING("tls-authz", MigrationState, parameters.tls_authz),
>
> /* Migration capabilities */
> DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
> --
This simple patch will cause a double-free issue on tls-creds, etc. because
the properties are released before migration object, and since we also
explicitly free tls_* fields in migration_instance_finalize() then it hits
double free. The details is in object_finalize() where the order is:
object_property_del_all(obj);
object_deinit(obj, ti);
It was overlooked when I was testing the preempt+tls functionality and
it'll trigger when we need a graceful quit of qemu.
Meanwhile there's another patch ordering issue I didn't notice when I post
this version: patch "migration: Add helpers to detect TLS capability" needs
to be before "migration: Enable TLS for preempt channel". No code change
for this, it just needs some re-ordering to guarantee per-commit builds to
be successful.
I'll respin another version..
--
Peter Xu
- [PATCH v3 04/19] migration: Move migrate_allow_multifd and helpers into migration.c, (continued)
- [PATCH v3 04/19] migration: Move migrate_allow_multifd and helpers into migration.c, Peter Xu, 2022/03/30
- [PATCH v3 05/19] migration: Export ram_load_postcopy(), Peter Xu, 2022/03/30
- [PATCH v3 06/19] migration: Move channel setup out of postcopy_try_recover(), Peter Xu, 2022/03/30
- [PATCH v3 09/19] migration: Postcopy preemption preparation on channel creation, Peter Xu, 2022/03/30
- [PATCH v3 08/19] migration: Add postcopy-preempt capability, Peter Xu, 2022/03/30
- [PATCH v3 10/19] migration: Postcopy preemption enablement, Peter Xu, 2022/03/30
- [PATCH v3 07/19] migration: Allow migrate-recover to run multiple times, Peter Xu, 2022/03/30
- [PATCH v3 12/19] migration: Create the postcopy preempt channel asynchronously, Peter Xu, 2022/03/30
- [PATCH v3 15/19] migration: Add helpers to detect TLS capability, Peter Xu, 2022/03/30
- [PATCH v3 16/19] migration: Export tls-[creds|hostname|authz] params to cmdline too, Peter Xu, 2022/03/30
- Re: [PATCH v3 16/19] migration: Export tls-[creds|hostname|authz] params to cmdline too,
Peter Xu <=
- [PATCH v3 13/19] migration: Enable TLS for preempt channel, Peter Xu, 2022/03/30
- [PATCH v3 11/19] migration: Postcopy recover with preempt enabled, Peter Xu, 2022/03/30
- [PATCH v3 18/19] tests: Add postcopy tls recovery migration test, Peter Xu, 2022/03/30
- [PATCH v3 14/19] migration: Parameter x-postcopy-preempt-break-huge, Peter Xu, 2022/03/30
- [PATCH v3 17/19] tests: Add postcopy tls migration test, Peter Xu, 2022/03/30
- [PATCH v3 19/19] tests: Add postcopy preempt tests, Peter Xu, 2022/03/30