[Qemu-devel] [PATCH 0/3] [For 2.5] Migration fixes

[Qemu-devel] [PATCH 0/3] [For 2.5] Migration fixes

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Hi,
  These are 3 migration fixes that fix things I messed up.
Only the 1st one (set last_sent_block) fixes a problem, the other
two fix coverity findings.

Dave

Dr. David Alan Gilbert (3):
  Set last_sent_block
  migration: Dead assignment of current_time
  Unneeded NULL check

 migration/migration.c | 3 +--
 migration/ram.c       | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCH 1/3] Set last_sent_block

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

In a82d593b61054b3dea43 I accidentally removed the setting of
last_sent_block,  put it back.

Symptoms:
  Multithreaded compression only uses one thread.
  Migration is a bit less efficient since it won't use 'cont' flags.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Fixes: a82d593b61054b3dea43
---
 migration/ram.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/migration/ram.c b/migration/ram.c
index 7f32696..1eb155a 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1249,6 +1249,7 @@ static int ram_save_target_page(MigrationState *ms, QEMUFile *f,
         if (unsentmap) {
             clear_bit(dirty_ram_abs >> TARGET_PAGE_BITS, unsentmap);
         }
+        last_sent_block = block;
     }
 
     return res;
-- 
2.5.0

Re: [Qemu-devel] [PATCH 1/3] Set last_sent_block

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> In a82d593b61054b3dea43 I accidentally removed the setting of
> last_sent_block,  put it back.
>
> Symptoms:
>   Multithreaded compression only uses one thread.
>   Migration is a bit less efficient since it won't use 'cont' flags.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Fixes: a82d593b61054b3dea43

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

[Qemu-devel] [PATCH 2/3] migration: Dead assignment of current_time

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

I set current_time before the postcopy test but never use it;
(I think this was from the original version where it was time based).
Spotted by coverity, CID 1339208

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 7e4e27b..265d13a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1643,7 +1643,6 @@ static void *migration_thread(void *opaque)
             if (pending_size && pending_size >= max_size) {
                 /* Still a significant amount to transfer */
 
-                current_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
                 if (migrate_postcopy_ram() &&
                     s->state != MIGRATION_STATUS_POSTCOPY_ACTIVE &&
                     pend_nonpost <= max_size &&
-- 
2.5.0

Re: [Qemu-devel] [PATCH 2/3] migration: Dead assignment of current_time

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> I set current_time before the postcopy test but never use it;
> (I think this was from the original version where it was time based).
> Spotted by coverity, CID 1339208
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

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

[Qemu-devel] [PATCH 3/3] Unneeded NULL check

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The check is unneccesary, we read the value at the start of the
thread, use it, and never change it.  The value is checked to be
non-NULL before thread creation.

Spotted by coverity, CID 1339211

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 265d13a..1a42aee 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1345,7 +1345,7 @@ static void *source_return_path_thread(void *opaque)
             break;
         }
     }
-    if (rp && qemu_file_get_error(rp)) {
+    if (qemu_file_get_error(rp)) {
         trace_source_return_path_thread_bad_end();
         mark_source_rp_bad(ms);
     }
-- 
2.5.0

Re: [Qemu-devel] [PATCH 3/3] Unneeded NULL check

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The check is unneccesary, we read the value at the start of the
> thread, use it, and never change it.  The value is checked to be
> non-NULL before thread creation.
>
> Spotted by coverity, CID 1339211
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

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