Re: [Qemu-devel] [PATCH] migration: avoid segmentfault when take a snapshot of a VM which being migrated

[Dr. David Alan Gilbert]

* 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?
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).

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