1. Patchew
  2. QEMU
  3. View series

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

Wei Yang posted 1 patch 5 years, 1 month ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190401061457.9393-1-richardw.yang@linux.intel.com
Maintainers: Juan Quintela <quintela@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>
migration/savevm.c | 35 ++++++++++++++---------------------
1 file changed, 14 insertions(+), 21 deletions(-)
[Qemu-devel] [PATCH] migration: cleanup check on ops in savevm.handlers iteration
Posted by Wei Yang 5 years, 1 month 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>
---
 migration/savevm.c | 35 ++++++++++++++---------------------
 1 file changed, 14 insertions(+), 21 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 5f0ca7fac2..92af2471cd 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1096,10 +1096,9 @@ 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(se->opaque)) {
+        if (se->ops->is_active &&
+            !se->ops->is_active(se->opaque)) {
                 continue;
-            }
         }
         save_section_header(f, se, QEMU_VM_SECTION_START);
 
@@ -1127,10 +1126,9 @@ 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(se->opaque)) {
+        if (se->ops->is_active &&
+            !se->ops->is_active(se->opaque)) {
                 continue;
-            }
         }
         ret = se->ops->resume_prepare(s, se->opaque);
         if (ret < 0) {
@@ -1223,10 +1221,9 @@ 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(se->opaque)) {
+        if (se->ops->is_active &&
+            !se->ops->is_active(se->opaque)) {
                 continue;
-            }
         }
         trace_savevm_section_start(se->idstr, se->section_id);
         /* Section type */
@@ -1265,18 +1262,16 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only,
     cpu_synchronize_all_states();
 
     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
-        if (!se->ops ||
+        if (!se->ops || !se->ops->save_live_complete_precopy ||
             (in_postcopy && se->ops->has_postcopy &&
              se->ops->has_postcopy(se->opaque)) ||
-            (in_postcopy && !iterable_only) ||
-            !se->ops->save_live_complete_precopy) {
+            (in_postcopy && !iterable_only)) {
             continue;
         }
 
-        if (se->ops && se->ops->is_active) {
-            if (!se->ops->is_active(se->opaque)) {
+        if (se->ops->is_active &&
+            !se->ops->is_active(se->opaque)) {
                 continue;
-            }
         }
         trace_savevm_section_start(se->idstr, se->section_id);
 
@@ -1377,10 +1372,9 @@ 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(se->opaque)) {
+        if (se->ops->is_active &&
+            !se->ops->is_active(se->opaque)) {
                 continue;
-            }
         }
         se->ops->save_live_pending(f, se->opaque, threshold_size,
                                    res_precopy_only, res_compatible,
@@ -2276,10 +2270,9 @@ 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(se->opaque)) {
+        if (se->ops->is_active &&
+            !se->ops->is_active(se->opaque)) {
                 continue;
-            }
         }
 
         ret = se->ops->load_setup(f, se->opaque);
-- 
2.19.1
Re: [Qemu-devel] [PATCH] migration: cleanup check on ops in savevm.handlers iteration
Posted by Wei Yang 4 years, 10 months ago 
On Mon, Apr 01, 2019 at 02:14:57PM +0800, Wei Yang 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>

Hi, David

Are you willing to pick up this one?

>---
> migration/savevm.c | 35 ++++++++++++++---------------------
> 1 file changed, 14 insertions(+), 21 deletions(-)
>
>diff --git a/migration/savevm.c b/migration/savevm.c
>index 5f0ca7fac2..92af2471cd 100644
>--- a/migration/savevm.c
>+++ b/migration/savevm.c
>@@ -1096,10 +1096,9 @@ 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(se->opaque)) {
>+        if (se->ops->is_active &&
>+            !se->ops->is_active(se->opaque)) {
>                 continue;
>-            }
>         }
>         save_section_header(f, se, QEMU_VM_SECTION_START);
> 
>@@ -1127,10 +1126,9 @@ 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(se->opaque)) {
>+        if (se->ops->is_active &&
>+            !se->ops->is_active(se->opaque)) {
>                 continue;
>-            }
>         }
>         ret = se->ops->resume_prepare(s, se->opaque);
>         if (ret < 0) {
>@@ -1223,10 +1221,9 @@ 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(se->opaque)) {
>+        if (se->ops->is_active &&
>+            !se->ops->is_active(se->opaque)) {
>                 continue;
>-            }
>         }
>         trace_savevm_section_start(se->idstr, se->section_id);
>         /* Section type */
>@@ -1265,18 +1262,16 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only,
>     cpu_synchronize_all_states();
> 
>     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>-        if (!se->ops ||
>+        if (!se->ops || !se->ops->save_live_complete_precopy ||
>             (in_postcopy && se->ops->has_postcopy &&
>              se->ops->has_postcopy(se->opaque)) ||
>-            (in_postcopy && !iterable_only) ||
>-            !se->ops->save_live_complete_precopy) {
>+            (in_postcopy && !iterable_only)) {
>             continue;
>         }
> 
>-        if (se->ops && se->ops->is_active) {
>-            if (!se->ops->is_active(se->opaque)) {
>+        if (se->ops->is_active &&
>+            !se->ops->is_active(se->opaque)) {
>                 continue;
>-            }
>         }
>         trace_savevm_section_start(se->idstr, se->section_id);
> 
>@@ -1377,10 +1372,9 @@ 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(se->opaque)) {
>+        if (se->ops->is_active &&
>+            !se->ops->is_active(se->opaque)) {
>                 continue;
>-            }
>         }
>         se->ops->save_live_pending(f, se->opaque, threshold_size,
>                                    res_precopy_only, res_compatible,
>@@ -2276,10 +2270,9 @@ 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(se->opaque)) {
>+        if (se->ops->is_active &&
>+            !se->ops->is_active(se->opaque)) {
>                 continue;
>-            }
>         }
> 
>         ret = se->ops->load_setup(f, se->opaque);
>-- 
>2.19.1

-- 
Wei Yang
Help you, Help me
Re: [Qemu-devel] [PATCH] migration: cleanup check on ops in savevm.handlers iteration
Posted by Juan Quintela 4 years, 10 months ago 
Wei Yang <richardw.yang@linux.intel.com> wrote:
> On Mon, Apr 01, 2019 at 02:14:57PM +0800, Wei Yang 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>
>
> Hi, David
>
> Are you willing to pick up this one?

also will pick up this one.

Later, Juan.

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.