| Message ID | 20190709140924.13291-4-richardw.yang@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration/savevm: move non SaveStateEntry condition check out of iteration | expand |
* Wei Yang (richardw.yang@linux.intel.com) wrote: > in_postcopy and iterable_only are not SaveStateEntry specific, it would > be more proper to check them out of iteration. > > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> Worth it just to make that big if statement simpler! Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > migration/savevm.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/migration/savevm.c b/migration/savevm.c > index c41e13e322..8a2ada529e 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -1247,8 +1247,7 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f) > } > > static > -int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy, > - bool iterable_only) > +int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy) > { > SaveStateEntry *se; > int ret; > @@ -1257,7 +1256,6 @@ int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy, > if (!se->ops || > (in_postcopy && se->ops->has_postcopy && > se->ops->has_postcopy(se->opaque)) || > - (in_postcopy && !iterable_only) || > !se->ops->save_live_complete_precopy) { > continue; > } > @@ -1369,10 +1367,11 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only, > > cpu_synchronize_all_states(); > > - ret = qemu_savevm_state_complete_precopy_iterable(f, in_postcopy, > - iterable_only); > - if (ret) { > - return ret; > + if (!in_postcopy || iterable_only) { > + ret = qemu_savevm_state_complete_precopy_iterable(f, in_postcopy); > + if (ret) { > + return ret; > + } > } > > if (iterable_only) { > -- > 2.17.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Fri, Jul 19, 2019 at 05:59:50PM +0100, Dr. David Alan Gilbert wrote: >* Wei Yang (richardw.yang@linux.intel.com) wrote: >> in_postcopy and iterable_only are not SaveStateEntry specific, it would >> be more proper to check them out of iteration. >> >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > >Worth it just to make that big if statement simpler! > Yep, the original one looks a little hard to understand. Thanks for your comment. > >Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > >> --- >> migration/savevm.c | 13 ++++++------- >> 1 file changed, 6 insertions(+), 7 deletions(-) >> >> diff --git a/migration/savevm.c b/migration/savevm.c >> index c41e13e322..8a2ada529e 100644 >> --- a/migration/savevm.c >> +++ b/migration/savevm.c >> @@ -1247,8 +1247,7 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f) >> } >> >> static >> -int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy, >> - bool iterable_only) >> +int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy) >> { >> SaveStateEntry *se; >> int ret; >> @@ -1257,7 +1256,6 @@ int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy, >> if (!se->ops || >> (in_postcopy && se->ops->has_postcopy && >> se->ops->has_postcopy(se->opaque)) || >> - (in_postcopy && !iterable_only) || >> !se->ops->save_live_complete_precopy) { >> continue; >> } >> @@ -1369,10 +1367,11 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only, >> >> cpu_synchronize_all_states(); >> >> - ret = qemu_savevm_state_complete_precopy_iterable(f, in_postcopy, >> - iterable_only); >> - if (ret) { >> - return ret; >> + if (!in_postcopy || iterable_only) { >> + ret = qemu_savevm_state_complete_precopy_iterable(f, in_postcopy); >> + if (ret) { >> + return ret; >> + } >> } >> >> if (iterable_only) { >> -- >> 2.17.1 >> >-- >Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/migration/savevm.c b/migration/savevm.c index c41e13e322..8a2ada529e 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1247,8 +1247,7 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f) } static -int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy, - bool iterable_only) +int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy) { SaveStateEntry *se; int ret; @@ -1257,7 +1256,6 @@ int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy, if (!se->ops || (in_postcopy && se->ops->has_postcopy && se->ops->has_postcopy(se->opaque)) || - (in_postcopy && !iterable_only) || !se->ops->save_live_complete_precopy) { continue; } @@ -1369,10 +1367,11 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only, cpu_synchronize_all_states(); - ret = qemu_savevm_state_complete_precopy_iterable(f, in_postcopy, - iterable_only); - if (ret) { - return ret; + if (!in_postcopy || iterable_only) { + ret = qemu_savevm_state_complete_precopy_iterable(f, in_postcopy); + if (ret) { + return ret; + } } if (iterable_only) {
in_postcopy and iterable_only are not SaveStateEntry specific, it would be more proper to check them out of iteration. Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> --- migration/savevm.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)