Re: [Qemu-devel] [PATCH 1/4] migration/savevm: remove duplicate check of migration_is_blocked

From: Daniel Henrique Barboza <danielhb413@gmail.com>
To: Wei Yang <richardw.yang@linux.intel.com>, qemu-devel@nongnu.org
Cc: dgilbert@redhat.com, quintela@redhat.com
Subject: Re: [Qemu-devel] [PATCH 1/4] migration/savevm: remove duplicate check of migration_is_blocked
Date: Thu, 25 Apr 2019 16:20:57 -0300	[thread overview]
Message-ID: <e9cd3438-345f-2a68-daec-1ce4cba37ef1@gmail.com> (raw)
In-Reply-To: <20190424004700.12766-2-richardw.yang@linux.intel.com>

On 4/23/19 9:46 PM, Wei Yang wrote:
> Current call flow of save_snapshot is:
>
>    save_snapshot
>      migration_is_blocked
>        qemu_savevm_state
>          migration_is_blocked
>
> Since qemu_savevm_state is only called in save_snapshot, this means
> migration_is_blocked has been already checked.

I think it would be a nice touch to add a comment in qemu_savevm_state,
saying that the function must be called with migration_is_blocked()
context. Just in case someone else in the future ends up re-using the
function.

Other than that, +1 for less code duplication.

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
>   migration/savevm.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 92af2471cd..2eea604624 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1412,10 +1412,6 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
>           return -EINVAL;
>       }
>   
> -    if (migration_is_blocked(errp)) {
> -        return -EINVAL;
> -    }
> -
>       if (migrate_use_block()) {
>           error_setg(errp, "Block migration and snapshots are incompatible");
>           return -EINVAL;