[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] migration: avoid segmentfault when take a snaps
From: |
Chai,Wen |
Subject: |
Re: [Qemu-devel] [PATCH] migration: avoid segmentfault when take a snapshot of a VM which being migrated |
Date: |
Mon, 15 Oct 2018 03:01:27 +0000 |
在 2018/10/12 下午5:01, "Dr. David Alan Gilbert" <address@hidden> 写入:
>* jialina01 (address@hidden) wrote:
>> During an active background migraion, snapshot will trigger a
>> segmentfault. As snapshot clears the "current_migration" struct
>> and updates "to_dst_file" before it finds out that there is a
>> migration task, Migration accesses the null pointer in
>> "current_migration" struct and qemu crashes eventually.
>>
>> Signed-off-by: jialina01 <address@hidden>
>> Signed-off-by: chaiwen <address@hidden>
>> Signed-off-by: zhangyu <address@hidden>
>
>Yes, that looks like an improvement, but is it enough?
Indeed it’s not enough, this patch fails to handle the case that doing
a snapshot under an active non-block migration.
>With that change does qemu_savevm_state fail cleanly if attempted
>during a migration? I suspect it will still try and do a snapshot,
>because I don't see where it's checking the current state
>(like the check in migrate_prepare that does a QERR_MIGRATIION_ACTIVE).
Yes, migration_is_setup_or_active looks like a more sensible detection for
this case,
and a proper error message it better to be set.
Thanks for your comment, we will send a v2 patch later.
Thanks
Chai Wen
>
>Dave
>
>
>> ---
>> migration/savevm.c | 14 +++++---------
>> 1 file changed, 5 insertions(+), 9 deletions(-)
>>
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 2d10e45582..9cb97ca343 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -1319,21 +1319,18 @@ static int qemu_savevm_state(QEMUFile *f, Error
>>**errp)
>> MigrationState *ms = migrate_get_current();
>> MigrationStatus status;
>>
>> - migrate_init(ms);
>> -
>> - ms->to_dst_file = f;
>> -
>> if (migration_is_blocked(errp)) {
>> - ret = -EINVAL;
>> - goto done;
>> + return -EINVAL;
>> }
>>
>> if (migrate_use_block()) {
>> error_setg(errp, "Block migration and snapshots are
>>incompatible");
>> - ret = -EINVAL;
>> - goto done;
>> + return -EINVAL;
>> }
>>
>> + migrate_init(ms);
>> + ms->to_dst_file = f;
>> +
>> qemu_mutex_unlock_iothread();
>> qemu_savevm_state_header(f);
>> qemu_savevm_state_setup(f);
>> @@ -1355,7 +1352,6 @@ static int qemu_savevm_state(QEMUFile *f, Error
>>**errp)
>> error_setg_errno(errp, -ret, "Error while writing VM state");
>> }
>>
>> -done:
>> if (ret != 0) {
>> status = MIGRATION_STATUS_FAILED;
>> } else {
>> --
>> 2.13.2.windows.1
>>
>--
>Dr. David Alan Gilbert / address@hidden / Manchester, UK