qemu-devel
[Top][All Lists]
Advanced

[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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]