diff mbox

[21/30] migration: move exit condition to migration thread

Message ID 1350545426-23172-22-git-send-email-quintela@redhat.com
State New
Headers show

Commit Message

Juan Quintela Oct. 18, 2012, 7:30 a.m. UTC 
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

Comments

Paolo Bonzini Oct. 18, 2012, 8:34 a.m. UTC | #1 
Il 18/10/2012 09:30, Juan Quintela ha scritto:
> -        if (s->migration_state->complete) {
> +        qemu_mutex_lock_iothread();

So, was it a bug that we were accessing ->complete without the BQL?

> +        if (m->state != MIG_STATE_ACTIVE) {
> +            DPRINTF("put_ready returning because of non-active state\n");

The contents of the debug message obsolete.  Besides, I would just put
the two branches in the same "if (m->state != MIG_STATE_ACTIVE ||
m->complete)".

> +            qemu_mutex_unlock_iothread();
>              break;
>          }
> +        if (m->complete) {
> +            qemu_mutex_unlock_iothread();
> +            break;
> +        }
> +        qemu_mutex_unlock_iothread();
> +

Paolo
Juan Quintela Oct. 26, 2012, 11:43 a.m. UTC | #2 
Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 18/10/2012 09:30, Juan Quintela ha scritto:
>> -        if (s->migration_state->complete) {
>> +        qemu_mutex_lock_iothread();
>
> So, was it a bug that we were accessing ->complete without the BQL?
>
>> +        if (m->state != MIG_STATE_ACTIVE) {
>> +            DPRINTF("put_ready returning because of non-active state\n");
>
> The contents of the debug message obsolete.  Besides, I would just put
> the two branches in the same "if (m->state != MIG_STATE_ACTIVE ||
> m->complete)".

BQL is not needed.  Complete is only used by the migration thread.  but
maintaing it outside of the iothread lock, makes locking even more complicated.

>
>> +            qemu_mutex_unlock_iothread();
>>              break;
>>          }
>> +        if (m->complete) {
>> +            qemu_mutex_unlock_iothread();
>> +            break;
>> +        }
>> +        qemu_mutex_unlock_iothread();
>> +
>
> Paolo
diff mbox

Patch

diff --git a/migration.c b/migration.c
index 8cacbc3..7206866 100644
--- a/migration.c
+++ b/migration.c
@@ -651,12 +651,6 @@  static bool migrate_fd_put_ready(MigrationState *s, uint64_t max_size)
     bool last_round = false;

     qemu_mutex_lock_iothread();
-    if (s->state != MIG_STATE_ACTIVE) {
-        DPRINTF("put_ready returning because of non-active state\n");
-        qemu_mutex_unlock_iothread();
-        return false;
-    }
-
     DPRINTF("iterate\n");
     pending_size = qemu_savevm_state_pending(s->file, max_size);
     DPRINTF("pending size %lu max %lu\n", pending_size, max_size);
@@ -723,9 +717,18 @@  static void *buffered_file_thread(void *opaque)
     while (true) {
         int64_t current_time = qemu_get_clock_ms(rt_clock);

-        if (s->migration_state->complete) {
+        qemu_mutex_lock_iothread();
+        if (m->state != MIG_STATE_ACTIVE) {
+            DPRINTF("put_ready returning because of non-active state\n");
+            qemu_mutex_unlock_iothread();
             break;
         }
+        if (m->complete) {
+            qemu_mutex_unlock_iothread();
+            break;
+        }
+        qemu_mutex_unlock_iothread();
+
         if (current_time >= initial_time + BUFFER_DELAY) {
             uint64_t transferred_bytes = s->bytes_xfer;
             uint64_t time_spent = current_time - initial_time;