Fix segfault on migration completion
PATCH v2 1/1 Fix segfault on migration completion
A simple migration reproduces it:
1. Start the source VM with:
# qemu [...] -S
2. Start the destination VM with:
# qemu <source VM cmd-line> -incoming tcp:0:4444
3. In the source VM:
(qemu) migrate -d tcp:0:4444
4. The source VM will segfault as soon as migration completes (might not happen in the first try)
What is happening here is that qemu_file_put_notify() can end up closing 's->file' (in which case it's also set to NULL). The call stack is rather complex, but Eduardo helped tracking it to:
select loop -> migrate_fd_put_notify() -> qemu_file_put_notify() -> buffered_put_buffer() -> migrate_fd_put_ready() -> migrate_fd_completed() -> migrate_fd_cleanup().
To be honest, it's not completely clear to me in which cases 's->file' is not closed (on error maybe)? But I doubt this fix will make anything worse.
---
V2: better commit log
migration.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
migration.c +1 -1
@@ -252,7 +252,7 @@ static void migrate_fd_put_notify(void *opaque)qemu_set_fd_handler2 (s->fd,NULL ,NULL ,NULL ,NULL );qemu_file_put_notify (s->file); -if (qemu_file_get_error (s->file)) { +if (s->file &&qemu_file_get_error (s->file)) {migrate_fd_error (s); } }
Signature · 1 line
1.7.7.1.488.ge8e1c.dirty
Quoted 29 lines · Luiz Capitulino
A simple migration reproduces it:
1. Start the source VM with:
# qemu [...] -S2. Start the destination VM with:
# qemu <source VM cmd-line> -incoming tcp:0:44443. In the source VM:
(qemu) migrate -d tcp:0:44444. The source VM will segfault as soon as migration completes (might not happen in the first try)
What is happening here is that
qemu_file_put_notify()can end up closing 's->file' (in which case it's also set to NULL). The call stack is rather complex, but Eduardo helped tracking it to:select loop ->
migrate_fd_put_notify()->qemu_file_put_notify()->buffered_put_buffer()->migrate_fd_put_ready()->migrate_fd_completed()->migrate_fd_cleanup().To be honest, it's not completely clear to me in which cases '
s->file' is not closed (on error maybe)? But I doubt this fix will make anything worse.
Quoted 6 lines
Reviewed-by Paolo Bonzini <[email protected]>Acked-by Eduardo Habkost <[email protected]>Signed-off-by Luiz Capitulino <[email protected]>---
V2: better commit log
And people wonder why error handling on migration is difficult, sniff :-(
Quoted 44 lines · Luiz Capitulino
A simple migration reproduces it:
1. Start the source VM with:
# qemu [...] -S2. Start the destination VM with:
# qemu <source VM cmd-line> -incoming tcp:0:44443. In the source VM:
(qemu) migrate -d tcp:0:44444. The source VM will segfault as soon as migration completes (might not happen in the first try)
What is happening here is that
qemu_file_put_notify()can end up closing 's->file' (in which case it's also set to NULL). The call stack is rather complex, but Eduardo helped tracking it to:select loop ->
migrate_fd_put_notify()->qemu_file_put_notify()->buffered_put_buffer()->migrate_fd_put_ready()->migrate_fd_completed()->migrate_fd_cleanup().To be honest, it's not completely clear to me in which cases '
s->file' is not closed (on error maybe)? But I doubt this fix will make anything worse.Reviewed-by Paolo Bonzini <[email protected]>Acked-by Eduardo Habkost <[email protected]>Signed-off-by Luiz Capitulino <[email protected]>---
V2: better commit log
migration.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)migration.c +0
-0
migration.c static void migrate_fd_put_notify(void *opaque) · line 258
Quoted 8 lines
migration.c +1
-1@@ -252,7 +252,7 @@ static void migrate_fd_put_notify(void *opaque)qemu_set_fd_handler2 (s->fd,NULL ,NULL ,NULL ,NULL );qemu_file_put_notify (s->file); -if (qemu_file_get_error (s->file)) { +if (s->file &&qemu_file_get_error (s->file)) {migrate_fd_error (s); } }
I wonder whether we can lose the error in s->file by closing s->file before we get here. But even if we can, we still report more errors than before the series this patch fixes.
Anthony Liguori
AppliedQuoted 25 lines · Luiz Capitulino
A simple migration reproduces it:
1. Start the source VM with:
# qemu [...] -S2. Start the destination VM with:
# qemu<source VM cmd-line> -incoming tcp:0:4444
3. In the source VM:
(qemu) migrate -d tcp:0:44444. The source VM will segfault as soon as migration completes (might not happen in the first try)
What is happening here is that
qemu_file_put_notify()can end up closing 's->file' (in which case it's also set to NULL). The call stack is rather complex, but Eduardo helped tracking it to:select loop ->
migrate_fd_put_notify()->qemu_file_put_notify()->buffered_put_buffer()->migrate_fd_put_ready()->migrate_fd_completed()->migrate_fd_cleanup().
Quoted 7 lines
To be honest, it's not completely clear to me in which cases '
s->file' is not closed (on error maybe)? But I doubt this fix will make anything worse.Reviewed-by Paolo Bonzini<[email protected]>Acked-by Eduardo Habkost<[email protected]>Signed-off-by Luiz Capitulino<[email protected]>
Applied. Thanks.
Regards,
Anthony Liguori
Quoted 20 lines
---
V2: better commit log
migration.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)migration.c +1
-1@@ -252,7 +252,7 @@ static void migrate_fd_put_notify(void *opaque)qemu_set_fd_handler2 (s->fd,NULL ,NULL ,NULL ,NULL );qemu_file_put_notify (s->file); -if (qemu_file_get_error (s->file)) { +if (s->file&&qemu_file_get_error (s->file)) {migrate_fd_error (s); } }