On Thu, Apr 25, 2019 at 04:20:57PM -0300, Daniel Henrique Barboza wrote: > > >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. >
That's reasonable, let me add a comment for qemu_savevm_state(). > >Other than that, +1 for less code duplication. > > >Reviewed-by: Daniel Henrique Barboza <danielhb...@gmail.com> > > > > >> >> Signed-off-by: Wei Yang <richardw.y...@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; -- Wei Yang Help you, Help me