在 2018/10/12 下午5:01， "Dr. David Alan Gilbert" <dgilb...@redhat.com> 写入:
>* jialina01 (jialin...@baidu.com) 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 <jialin...@baidu.com>
>> Signed-off-by: chaiwen <chai...@baidu.com>
>> Signed-off-by: zhangyu <zhangy...@baidu.com>
>
>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 / dgilb...@redhat.com / Manchester, UK
