[Qemu-devel] [PATCH v2] migration: cleanup check on ops in savevm.handlers iterations

Wei Yang posted 1 patch 6 years, 2 months ago
Test FreeBSD passed
Test docker-mingw@fedora passed
Test asan passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test s390x failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190819032804.8579-1-richardw.yang@linux.intel.com
Maintainers: "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Juan Quintela <quintela@redhat.com>
migration/savevm.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
[Qemu-devel] [PATCH v2] migration: cleanup check on ops in savevm.handlers iterations
Posted by Wei Yang 6 years, 2 months ago
During migration, there are several places to iterate on
savevm.handlers. And on each iteration, we need to check its ops and
related callbacks before invoke it.

Generally, ops is the first element to check, and it is only necessary
to check it once.

This patch clean all the related part in savevm.c to check ops only once
in those iterations.

Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>

---
v2: rebase on latest upstream
---
 migration/savevm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 4a86128ac4..35426d1db8 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1100,7 +1100,7 @@ void qemu_savevm_state_setup(QEMUFile *f)
         if (!se->ops || !se->ops->save_setup) {
             continue;
         }
-        if (se->ops && se->ops->is_active) {
+        if (se->ops->is_active) {
             if (!se->ops->is_active(se->opaque)) {
                 continue;
             }
@@ -1131,7 +1131,7 @@ int qemu_savevm_state_resume_prepare(MigrationState *s)
         if (!se->ops || !se->ops->resume_prepare) {
             continue;
         }
-        if (se->ops && se->ops->is_active) {
+        if (se->ops->is_active) {
             if (!se->ops->is_active(se->opaque)) {
                 continue;
             }
@@ -1227,7 +1227,7 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f)
         if (!se->ops || !se->ops->save_live_complete_postcopy) {
             continue;
         }
-        if (se->ops && se->ops->is_active) {
+        if (se->ops->is_active) {
             if (!se->ops->is_active(se->opaque)) {
                 continue;
             }
@@ -1264,7 +1264,7 @@ int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy)
             continue;
         }
 
-        if (se->ops && se->ops->is_active) {
+        if (se->ops->is_active) {
             if (!se->ops->is_active(se->opaque)) {
                 continue;
             }
@@ -1413,7 +1413,7 @@ void qemu_savevm_state_pending(QEMUFile *f, uint64_t threshold_size,
         if (!se->ops || !se->ops->save_live_pending) {
             continue;
         }
-        if (se->ops && se->ops->is_active) {
+        if (se->ops->is_active) {
             if (!se->ops->is_active(se->opaque)) {
                 continue;
             }
@@ -2334,7 +2334,7 @@ static int qemu_loadvm_state_setup(QEMUFile *f)
         if (!se->ops || !se->ops->load_setup) {
             continue;
         }
-        if (se->ops && se->ops->is_active) {
+        if (se->ops->is_active) {
             if (!se->ops->is_active(se->opaque)) {
                 continue;
             }
-- 
2.17.1


Re: [Qemu-devel] [PATCH v2] migration: cleanup check on ops in savevm.handlers iterations
Posted by Dr. David Alan Gilbert 6 years, 2 months ago
* Wei Yang (richardw.yang@linux.intel.com) wrote:
> During migration, there are several places to iterate on
> savevm.handlers. And on each iteration, we need to check its ops and
> related callbacks before invoke it.
> 
> Generally, ops is the first element to check, and it is only necessary
> to check it once.
> 
> This patch clean all the related part in savevm.c to check ops only once
> in those iterations.
> 
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>

Queued

> 
> ---
> v2: rebase on latest upstream
> ---
>  migration/savevm.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 4a86128ac4..35426d1db8 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1100,7 +1100,7 @@ void qemu_savevm_state_setup(QEMUFile *f)
>          if (!se->ops || !se->ops->save_setup) {
>              continue;
>          }
> -        if (se->ops && se->ops->is_active) {
> +        if (se->ops->is_active) {
>              if (!se->ops->is_active(se->opaque)) {
>                  continue;
>              }
> @@ -1131,7 +1131,7 @@ int qemu_savevm_state_resume_prepare(MigrationState *s)
>          if (!se->ops || !se->ops->resume_prepare) {
>              continue;
>          }
> -        if (se->ops && se->ops->is_active) {
> +        if (se->ops->is_active) {
>              if (!se->ops->is_active(se->opaque)) {
>                  continue;
>              }
> @@ -1227,7 +1227,7 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f)
>          if (!se->ops || !se->ops->save_live_complete_postcopy) {
>              continue;
>          }
> -        if (se->ops && se->ops->is_active) {
> +        if (se->ops->is_active) {
>              if (!se->ops->is_active(se->opaque)) {
>                  continue;
>              }
> @@ -1264,7 +1264,7 @@ int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy)
>              continue;
>          }
>  
> -        if (se->ops && se->ops->is_active) {
> +        if (se->ops->is_active) {
>              if (!se->ops->is_active(se->opaque)) {
>                  continue;
>              }
> @@ -1413,7 +1413,7 @@ void qemu_savevm_state_pending(QEMUFile *f, uint64_t threshold_size,
>          if (!se->ops || !se->ops->save_live_pending) {
>              continue;
>          }
> -        if (se->ops && se->ops->is_active) {
> +        if (se->ops->is_active) {
>              if (!se->ops->is_active(se->opaque)) {
>                  continue;
>              }
> @@ -2334,7 +2334,7 @@ static int qemu_loadvm_state_setup(QEMUFile *f)
>          if (!se->ops || !se->ops->load_setup) {
>              continue;
>          }
> -        if (se->ops && se->ops->is_active) {
> +        if (se->ops->is_active) {
>              if (!se->ops->is_active(se->opaque)) {
>                  continue;
>              }
> -- 
> 2.17.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2] migration: cleanup check on ops in savevm.handlers iterations
Posted by Juan Quintela 6 years, 2 months ago
Wei Yang <richardw.yang@linux.intel.com> wrote:
> During migration, there are several places to iterate on
> savevm.handlers. And on each iteration, we need to check its ops and
> related callbacks before invoke it.
>
> Generally, ops is the first element to check, and it is only necessary
> to check it once.
>
> This patch clean all the related part in savevm.c to check ops only once
> in those iterations.
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Nice cleanup.

Re: [Qemu-devel] [PATCH v2] migration: cleanup check on ops in savevm.handlers iterations
Posted by Wei Yang 6 years, 2 months ago
On Mon, Aug 19, 2019 at 03:50:44PM +0200, Juan Quintela wrote:
>Wei Yang <richardw.yang@linux.intel.com> wrote:
>> During migration, there are several places to iterate on
>> savevm.handlers. And on each iteration, we need to check its ops and
>> related callbacks before invoke it.
>>
>> Generally, ops is the first element to check, and it is only necessary
>> to check it once.
>>
>> This patch clean all the related part in savevm.c to check ops only once
>> in those iterations.
>>
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>
>Reviewed-by: Juan Quintela <quintela@redhat.com>
>
>Nice cleanup.

Thanks

-- 
Wei Yang
Help you, Help me