Re: migration with exec giving truncated images

Re: migration with exec giving truncated images

[Jim Paris]

I think I've (finally!) tracked it down.  See the attached patches.

The main problem is this: when using "-monitor pty", all incoming
commands are terminated with CRLF even though they were sent with just
LF, probably because of the pty layer somewhere.  When qemu's readline
gets CR and LF without calling readline_start() in between, it
executes the same command twice in a row, which meant that _two_
migrations were running concurrently.

-jim

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[PATCH 1/3] qemu: fix freed pointer dereference

[Jim Paris]

If *has_error==0, s is freed before s->detach is used.  Save a copy of
s->detach earlier.

Signed-off-by: Jim Paris <jim-XrPbb/hENzg@public.gmane.org>
---

This shouldn't change much since the memory is most likely still
valid even after it's been freed, but it's still a bug.

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

diff --git a/qemu/migration.c b/qemu/migration.c
index 6053c98..4d7aa01 100644
--- a/qemu/migration.c
+++ b/qemu/migration.c
@@ -169,6 +169,7 @@ static void migrate_finish(MigrationState *s)
     int ret = 0;
     int *has_error = s->has_error;
     int saved_vm_running = vm_running;
+    int detach = s->detach;
 
     fcntl(s->fd, F_SETFL, 0);
 
@@ -194,7 +195,7 @@ static void migrate_finish(MigrationState *s)
 	if (saved_vm_running)
             vm_start();
     }
-    if (!s->detach)
+    if (!detach)
 	monitor_resume();
     qemu_free(has_error);
     cpu_physical_memory_set_dirty_tracking(0);
-- 
1.5.3.GIT


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[PATCH 2/3] qemu: don't start a new migration if one is already in progress

[Jim Paris]

Signed-off-by: Jim Paris <jim-XrPbb/hENzg@public.gmane.org>
---

Having two migrations run simultaneously was causing my crashes.
The command was sent twice because of a bug in the readline routines,
but adding a check here as well seems like a good idea.

 qemu/migration.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/qemu/migration.c b/qemu/migration.c
index 4d7aa01..96b0c2f 100644
--- a/qemu/migration.c
+++ b/qemu/migration.c
@@ -973,6 +973,12 @@ int migrate_incoming(const char *device)
 void do_migrate(int detach, const char *uri)
 {
     const char *ptr;
+    MigrationState *s = current_migration;
+
+    if (s) {
+	term_printf("Migration already active\n");
+	return;
+    }
 
     status = MIG_STAT_INVALID_PARAMS;
     if (strstart(uri, "exec:", &ptr)) {
-- 
1.5.3.GIT


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[PATCH 3/3] qemu: reset buffer pointers after CR/LF

[Jim Paris]

If readline_handle_byte() is sent both a CR and LF, and
readline_start() is not called after the first CR, then the LF will
cause the same command to be executed a second time.  Fix this by
explicitly resetting the buffer pointer when it is processed.

Signed-off-by: Jim Paris <jim-XrPbb/hENzg@public.gmane.org>
---

This should probably get pushed upstream too.

 qemu/readline.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/qemu/readline.c b/qemu/readline.c
index cbe33db..bde3342 100644
--- a/qemu/readline.c
+++ b/qemu/readline.c
@@ -335,6 +335,8 @@ void readline_handle_byte(int ch)
             if (!term_is_password)
                 term_hist_add(term_cmd_buf);
             term_printf("\n");
+            term_cmd_buf_index = 0;
+            term_cmd_buf_size = 0;
             /* NOTE: readline_start can be called here */
             term_readline_func(term_readline_opaque, term_cmd_buf);
             break;
-- 
1.5.3.GIT


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH 1/3] qemu: fix freed pointer dereference

[Avi Kivity]

Jim Paris wrote:
> If *has_error==0, s is freed before s->detach is used.  Save a copy of
> s->detach earlier.
>
>   

Applied all three, thanks.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: migration with exec giving truncated images

[Uri Lublin]

Thanks for the patches.

There is still the mystery of different file sizes for different 
migration-exec commands, all files are "valid saved image".
It seems to me that some unmodified pages are being marked as dirty, and 
are being saved twice (and later loaded twice).
I'm still chasing that.

Uri.

Jim Paris wrote:
> I think I've (finally!) tracked it down.  See the attached patches.
>
> The main problem is this: when using "-monitor pty", all incoming
> commands are terminated with CRLF even though they were sent with just
> LF, probably because of the pty layer somewhere.  When qemu's readline
> gets CR and LF without calling readline_start() in between, it
> executes the same command twice in a row, which meant that _two_
> migrations were running concurrently.
>
> -jim
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems?  Stop.
> Now Search log events and configuration files using AJAX and a browser.
> Download your FREE copy of Splunk now >>  http://get.splunk.com/
> _______________________________________________
> kvm-devel mailing list
> kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/kvm-devel
>   

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: migration with exec giving truncated images

[Jim Paris]

Uri Lublin wrote:
> There is still the mystery of different file sizes for different 
> migration-exec commands, all files are "valid saved image".
> It seems to me that some unmodified pages are being marked as dirty, and 
> are being saved twice (and later loaded twice).
> I'm still chasing that.

Hi Uri,

I looked into this a bit more and it seems that a big piece of
migration.c is missing or broken.

In migrate_write_buffer, it calls migrate_check_convergence, which
returns TRUE if the migration is "almost" complete (dirty pages < 50,
or too many iterations through memory).  At that point, it then calls
migrate_finish -- which finishes writing the current page, but never
actually writes the remaining 50 pages (!)  Am I missing something?

-jim



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: migration with exec giving truncated images

[Jim Paris]

I wrote
> I looked into this a bit more and it seems that a big piece of
> migration.c is missing or broken.
..
> Am I missing something?

Yes, I am.  Sorry, I missed the qemu_live_savevm_state call, which
saves the rest of the dirty pages, and explains why some migration
images are larger than others (ram_live_save doesn't compress
homogeneous pages like migrate_write does).

-jim

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/