Skip to contentSkip to conversation

set proper migration status on ->write error

PATCH In review Marcelo Tosatti 4 participants 12 messages Jul 7, 2010 – Jul 15, 2010

lore
Share & tools
lore

Marcelo Tosatti

author
#0 lore raw

PATCH 1/1 set proper migration status on ->write error

If ->write fails, declare migration status as MIG_STATE_ERROR.

Also, in buffered_file.c, ->close the object in case of an error.

Fixes "migrate -d "exec:dd of=file", where dd fails to open file.

Signed-off-by Marcelo Tosatti <[email protected]>
buffered_file.c +3 -1
index 54dc6c2..be147d6 100644
@@ -222,8 +222,10 @@ static void buffered_rate_tick(void *opaque)
 {
     QEMUFileBuffered *s = opaque;
 
-    if (s->has_error)
+    if (s->has_error) {
+        buffered_close(s);
         return;
+    }
 
     qemu_mod_timer(s->timer, qemu_get_clock(rt_clock) + 100);
migration.c +2 -0
index b49964c..5bf2f4e 100644
@@ -318,6 +318,8 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
 
     if (ret == -EAGAIN)
         qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
+    else
+        s->state = MIG_STATE_ERROR;
 
     return ret;
 }
#1 lore raw
Quoted 31 lines · Marcelo Tosatti

If ->write fails, declare migration status as MIG_STATE_ERROR.

Also, in buffered_file.c, ->close the object in case of an error.

Fixes "migrate -d "exec:dd of=file", where dd fails to open file.

Signed-off-by Marcelo Tosatti <[email protected]>
buffered_file.c +3 -1
index 54dc6c2..be147d6 100644
@@ -222,8 +222,10 @@ static void buffered_rate_tick(void *opaque)
  {
     QEMUFileBuffered *s = opaque;
 
-    if (s->has_error)
+    if (s->has_error) {
+        buffered_close(s);
         return;
+    }
 
     qemu_mod_timer(s->timer, qemu_get_clock(rt_clock) + 100);
migration.c +0 -0
index b49964c..5bf2f4e 100644

migration.c ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size) · line 322

Quoted 5 lines
migration.c +2 -0
@@ -318,6 +318,8 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
 
     if (ret == -EAGAIN)
         qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
+    else
+        s->state = MIG_STATE_ERROR;

Please check CODING_STYLE. It's different from kernel's version.

author
#2 lore raw

PATCH 1/1 set proper migration status on ->write error (v2)

If ->write fails, declare migration status as MIG_STATE_ERROR.

Also, in buffered_file.c, ->close the object in case of an error.

Fixes "migrate -d "exec:dd of=file", where dd fails to open file.

Signed-off-by Marcelo Tosatti <[email protected]>
buffered_file.c +3 -1
index 54dc6c2..be147d6 100644
@@ -222,8 +222,10 @@ static void buffered_rate_tick(void *opaque)
 {
     QEMUFileBuffered *s = opaque;
 
-    if (s->has_error)
+    if (s->has_error) {
+        buffered_close(s);
         return;
+    }
 
     qemu_mod_timer(s->timer, qemu_get_clock(rt_clock) + 100);
migration.c +4 -1
index b49964c..433f08f 100644
@@ -316,8 +316,11 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
     if (ret == -1)
         ret = -(s->get_error(s));
 
-    if (ret == -EAGAIN)
+    if (ret == -EAGAIN) {
         qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
+    } else {
+        s->state = MIG_STATE_ERROR;
+    }
 
     return ret;
 }
Acked-by on v1 1/1
#3 lore raw

[Qemu-devel] Re: [PATCH] set proper migration status on ->write error (v2)

Quoted 8 lines · Marcelo Tosatti

If ->write fails, declare migration status as MIG_STATE_ERROR.

Also, in buffered_file.c, ->close the object in case of an error.

Fixes "migrate -d "exec:dd of=file", where dd fails to open file.

Signed-off-by Marcelo Tosatti <[email protected]>
Acked-by Juan Quintela <[email protected]>

Again, just in case O:-)

Acked-by on v1 1/1
#4 lore raw
In reply to Marcelo Tosatti
Quoted 8 lines · Marcelo Tosatti

If ->write fails, declare migration status as MIG_STATE_ERROR.

Also, in buffered_file.c, ->close the object in case of an error.

Fixes "migrate -d "exec:dd of=file", where dd fails to open file.

Signed-off-by Marcelo Tosatti <[email protected]>
Acked-by Juan Quintela <[email protected]>
Quoted 29 lines
buffered_file.c +3 -1
index 54dc6c2..be147d6 100644
@@ -222,8 +222,10 @@ static void buffered_rate_tick(void *opaque)
 {
     QEMUFileBuffered *s = opaque;
 
-    if (s->has_error)
+    if (s->has_error) {
+        buffered_close(s);
         return;
+    }
 
     qemu_mod_timer(s->timer, qemu_get_clock(rt_clock) + 100);
migration.c +2 -0
index b49964c..5bf2f4e 100644
@@ -318,6 +318,8 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
 
     if (ret == -EAGAIN)
         qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
+    else
+        s->state = MIG_STATE_ERROR;
 
     return ret;
 }
author
#5 lore raw

PATCH 1/1 set proper migration status on ->write error (v3)

In reply to Marcelo Tosatti

If ->write fails, declare migration status as MIG_STATE_ERROR.

Also, in buffered_file.c, ->close the object in case of an error.

Fixes "migrate -d "exec:dd of=file", where dd fails to open file.

Signed-off-by Marcelo Tosatti <[email protected]>
buffered_file.c +3 -1
index 54dc6c2..be147d6 100644
@@ -222,8 +222,10 @@ static void buffered_rate_tick(void *opaque)
 {
     QEMUFileBuffered *s = opaque;
 
-    if (s->has_error)
+    if (s->has_error) {
+        buffered_close(s);
         return;
+    }
 
     qemu_mod_timer(s->timer, qemu_get_clock(rt_clock) + 100);
migration.c +5 -1
index b49964c..862dc4f 100644
@@ -316,8 +316,12 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
     if (ret == -1)
         ret = -(s->get_error(s));
 
-    if (ret == -EAGAIN)
+    if (ret == -EAGAIN) {
         qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
+    } else if (ret < 0) {
+        monitor_resume(s);
+        s->state = MIG_STATE_ERROR;
+    }
 
     return ret;
 }
#6 lore raw

[Qemu-devel] Re: [PATCH] set proper migration status on ->write error (v3)

Quoted 31 lines · On Fri, 9 Jul 2010 15:18:51 -0300 Marcelo Tosatti <[email protected]> wrote:

If ->write fails, declare migration status as MIG_STATE_ERROR.

Also, in buffered_file.c, ->close the object in case of an error.

Fixes "migrate -d "exec:dd of=file", where dd fails to open file.

Signed-off-by Marcelo Tosatti <[email protected]>
buffered_file.c +3 -1
index 54dc6c2..be147d6 100644
@@ -222,8 +222,10 @@ static void buffered_rate_tick(void *opaque)
 {
     QEMUFileBuffered *s = opaque;
 
-    if (s->has_error)
+    if (s->has_error) {
+        buffered_close(s);
         return;
+    }
 
     qemu_mod_timer(s->timer, qemu_get_clock(rt_clock) + 100);
migration.c +0 -0
index b49964c..862dc4f 100644

migration.c ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size) · line 322

Quoted 8 lines
migration.c +3 -1
@@ -316,8 +316,12 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
     if (ret == -1)
         ret = -(s->get_error(s));
 
-    if (ret == -EAGAIN)
+    if (ret == -EAGAIN) {
         qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
+    } else if (ret < 0) {
+        monitor_resume(s);

You have to pass s->mon, but I forgot to mention that you also have to check for NULL like migrate_fd_cleanup():

if (s->mon) {
    monitor_resume(s->mon);
}

Also, there's a comment in migrate_fd_cleanup() saying that we should only resume the monitor when all buffers have been flushed. I assume buffred_close() will do that.

Quoted 6 lines
migration.c +2 -0
+        s->state = MIG_STATE_ERROR;
+    }
 
     return ret;
 }
author
#7 lore raw

PATCH 1/1 set proper migration status on ->write error (v4)

If ->write fails, declare migration status as MIG_STATE_ERROR.

Also, in buffered_file.c, ->close the object in case of an error.

Fixes "migrate -d "exec:dd of=file", where dd fails to open file.

Signed-off-by Marcelo Tosatti <[email protected]>
buffered_file.c +3 -1
index 54dc6c2..be147d6 100644
@@ -222,8 +222,10 @@ static void buffered_rate_tick(void *opaque)
 {
     QEMUFileBuffered *s = opaque;
 
-    if (s->has_error)
+    if (s->has_error) {
+        buffered_close(s);
         return;
+    }
 
     qemu_mod_timer(s->timer, qemu_get_clock(rt_clock) + 100);
migration.c +7 -1
index b49964c..f8e6325 100644
@@ -316,8 +316,14 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
     if (ret == -1)
         ret = -(s->get_error(s));
 
-    if (ret == -EAGAIN)
+    if (ret == -EAGAIN) {
         qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
+    } else if (ret < 0) {
+        if (s->mon) {
+            monitor_resume(s);
+        }
+        s->state = MIG_STATE_ERROR;
+    }
 
     return ret;
 }
#8 lore raw

[Qemu-devel] Re: [PATCH] set proper migration status on ->write error (v4)

Quoted 31 lines · On Tue, 13 Jul 2010 14:30:22 -0300 Marcelo Tosatti <[email protected]> wrote:

If ->write fails, declare migration status as MIG_STATE_ERROR.

Also, in buffered_file.c, ->close the object in case of an error.

Fixes "migrate -d "exec:dd of=file", where dd fails to open file.

Signed-off-by Marcelo Tosatti <[email protected]>
buffered_file.c +3 -1
index 54dc6c2..be147d6 100644
@@ -222,8 +222,10 @@ static void buffered_rate_tick(void *opaque)
 {
     QEMUFileBuffered *s = opaque;
 
-    if (s->has_error)
+    if (s->has_error) {
+        buffered_close(s);
         return;
+    }
 
     qemu_mod_timer(s->timer, qemu_get_clock(rt_clock) + 100);
migration.c +0 -0
index b49964c..f8e6325 100644

migration.c ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size) · line 323

Quoted 9 lines
migration.c +4 -1
@@ -316,8 +316,14 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
     if (ret == -1)
         ret = -(s->get_error(s));
 
-    if (ret == -EAGAIN)
+    if (ret == -EAGAIN) {
         qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
+    } else if (ret < 0) {
+        if (s->mon) {
+            monitor_resume(s);

s/monitor_resume(s)/monitor_resume(s->mon)

Quoted 7 lines
migration.c +3 -0
+        }
+        s->state = MIG_STATE_ERROR;
+    }
 
     return ret;
 }
author
#9 lore raw

PATCH 1/1 set proper migration status on ->write error (v5)

If ->write fails, declare migration status as MIG_STATE_ERROR.

Also, in buffered_file.c, ->close the object in case of an error.

Fixes "migrate -d "exec:dd of=file", where dd fails to open file.

Signed-off-by Marcelo Tosatti <[email protected]>
buffered_file.c +3 -1
index 54dc6c2..be147d6 100644
@@ -222,8 +222,10 @@ static void buffered_rate_tick(void *opaque)
 {
     QEMUFileBuffered *s = opaque;
 
-    if (s->has_error)
+    if (s->has_error) {
+        buffered_close(s);
         return;
+    }
 
     qemu_mod_timer(s->timer, qemu_get_clock(rt_clock) + 100);
migration.c +7 -1
index b49964c..f8e6325 100644
@@ -316,8 +316,14 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
     if (ret == -1)
         ret = -(s->get_error(s));
 
-    if (ret == -EAGAIN)
+    if (ret == -EAGAIN) {
         qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
+    } else if (ret < 0) {
+        if (s->mon) {
+            monitor_resume(s->mon);
+        }
+        s->state = MIG_STATE_ERROR;
+    }
 
     return ret;
 }
Reviewed-by on v1 1/1
#10 lore raw

[Qemu-devel] Re: [PATCH] set proper migration status on ->write error (v5)

Quoted 9 lines · On Tue, 13 Jul 2010 21:01:33 -0300 Marcelo Tosatti <[email protected]> wrote:

If ->write fails, declare migration status as MIG_STATE_ERROR.

Also, in buffered_file.c, ->close the object in case of an error.

Fixes "migrate -d "exec:dd of=file", where dd fails to open file.

Signed-off-by Marcelo Tosatti <[email protected]>

Looks good now, I guess it's good for stable too, isn't it?

Reviewed-by Luiz Capitulino <[email protected]>
Quoted 38 lines
buffered_file.c +3 -1
index 54dc6c2..be147d6 100644
@@ -222,8 +222,10 @@ static void buffered_rate_tick(void *opaque)
 {
     QEMUFileBuffered *s = opaque;
 
-    if (s->has_error)
+    if (s->has_error) {
+        buffered_close(s);
         return;
+    }
 
     qemu_mod_timer(s->timer, qemu_get_clock(rt_clock) + 100);
migration.c +7 -1
index b49964c..f8e6325 100644
@@ -316,8 +316,14 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
     if (ret == -1)
         ret = -(s->get_error(s));
 
-    if (ret == -EAGAIN)
+    if (ret == -EAGAIN) {
         qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
+    } else if (ret < 0) {
+        if (s->mon) {
+            monitor_resume(s->mon);
+        }
+        s->state = MIG_STATE_ERROR;
+    }
 
     return ret;
 }
author
#11 lore raw

[Qemu-devel] Re: [PATCH] set proper migration status on ->write error (v5)

Quoted 9 lines · Luiz Capitulino
Quoted 6 lines · On Tue, 13 Jul 2010 21:01:33 -0300 Marcelo Tosatti <[email protected]> wrote:

If ->write fails, declare migration status as MIG_STATE_ERROR.

Also, in buffered_file.c, ->close the object in case of an error.

Quoted 7 lines
Quoted 3 lines

Fixes "migrate -d "exec:dd of=file", where dd fails to open file.

Signed-off-by Marcelo Tosatti <[email protected]>

Looks good now, I guess it's good for stable too, isn't it?

Reviewed-by Luiz Capitulino <[email protected]>

Yep, good for stable.