4:13 p.m.
libvirt 0.8.7
qemu 0.13
I'm looking into a problem with qemu save/restore via JSON monitor. On
restore, the vm is left in a paused state with following error returned
for 'cont' command
An incoming migration is expected before this command can be executed
I was trying to debug the issue in gdb, but stepping through the code
introduces enough delay between qemudStartVMDaemon() and doStartCPUs()
that the latter succeeds. Any suggestions on how to determine when it
is safe to call doStartCPUs() to prevent the above error? I don't see
this issue with the text monitor btw.
Thanks,
Jim
10:29 p.m.
On 01/12/2011 05:13 PM, Jim Fehlig wrote:
libvirt 0.8.7
qemu 0.13
I'm looking into a problem with qemu save/restore via JSON monitor. On
restore, the vm is left in a paused state with following error returned
for 'cont' command
An incoming migration is expected before this command can be executed
I was trying to debug the issue in gdb, but stepping through the code
introduces enough delay between qemudStartVMDaemon() and doStartCPUs()
that the latter succeeds. Any suggestions on how to determine when it
is safe to call doStartCPUs() to prevent the above error? I don't see
this issue with the text monitor btw.
I'm pretty sure this is related to a bug I reported on qemu-devel last
April:
http://lists.gnu.org/archive/html/qemu-devel/2010-04/msg00635.html
(be sure to read my own followup if you want a correct description of
the circumstances). In this case libvirt was using the text monitor, and
there was a race condition between qemudStartVMDaemon (which executes
qemu with '-S -incoming') and doStartCPUs() (which issues a 'cont'
command to the qemu monitor). The result would be that sometimes the
'cont' would be received and processed by qemu before the incoming
migration had started, meaning that qemu would be executing garbage
memory instead of the saved/restored image of the guest.
The solution to this was posted to upstream qemu in July:
http://lists.gnu.org/archive/html/qemu-devel/2010-07/msg01574.html
and I believe is in qemu 0.13. That patch adds a check to the 'cont'
command so that if '-incoming' was specified on the commandline, 'cont'
will only execute after a migration has successfully completed, but will
otherwise return an error.
Actually, thinking about this "fix", it seems that it isn't really a
solution, because instead of the guest starting up in an indeterminate
state, doStartCPUs() will just fail (as you've seen) making the entire
guest startup fail.
You can almost surely make it work properly by putting in a 250msec
delay between those two function calls in libvirt. It would be nice if
it could be totally fixed in qemu, though, so that libvirt didn't need
such a hack :-(
(I had unfortunately lost track of the bug by the time the patch was
posted - it had been there for so long I'd just gotten used to manually
pausing/unpausing any guest I wanted to save on the one machine that
displays the problem. Too bad I got so used to living with it, as I'd
have otherwise been forced to try it out (this machine is running F13,
which is still at qemu-0.12.5, which doesn't have the patch).
7:35 a.m.
On Wed, Jan 12, 2011 at 11:29:27PM -0500, Laine Stump wrote:
On 01/12/2011 05:13 PM, Jim Fehlig wrote:
>libvirt 0.8.7
>qemu 0.13
>
>I'm looking into a problem with qemu save/restore via JSON monitor. On
>restore, the vm is left in a paused state with following error returned
>for 'cont' command
>
>An incoming migration is expected before this command can be executed
>
>I was trying to debug the issue in gdb, but stepping through the code
>introduces enough delay between qemudStartVMDaemon() and doStartCPUs()
>that the latter succeeds. Any suggestions on how to determine when it
>is safe to call doStartCPUs() to prevent the above error? I don't see
>this issue with the text monitor btw.
I'm pretty sure this is related to a bug I reported on qemu-devel
last April:
http://lists.gnu.org/archive/html/qemu-devel/2010-04/msg00635.html
(be sure to read my own followup if you want a correct description
of the circumstances). In this case libvirt was using the text
monitor, and there was a race condition between qemudStartVMDaemon
(which executes qemu with '-S -incoming') and doStartCPUs() (which
issues a 'cont' command to the qemu monitor). The result would be
that sometimes the 'cont' would be received and processed by qemu
before the incoming migration had started, meaning that qemu would
be executing garbage memory instead of the saved/restored image of
the guest.
The solution to this was posted to upstream qemu in July:
http://lists.gnu.org/archive/html/qemu-devel/2010-07/msg01574.html
and I believe is in qemu 0.13. That patch adds a check to the 'cont'
command so that if '-incoming' was specified on the commandline,
'cont' will only execute after a migration has successfully
completed, but will otherwise return an error.
Actually, thinking about this "fix", it seems that it isn't really a
solution, because instead of the guest starting up in an
indeterminate state, doStartCPUs() will just fail (as you've seen)
making the entire guest startup fail.
Yep, it wasn't really intended as a fix. It was intended to make
the error scenario clearly detectable, which has succeeded as per
Jim's report. The fact that QMP returned an error in this way,
means we can now reliably detect, usleep(1000), and then retry
the 'cont' again at which point we should succeed.
You can almost surely make it work properly by putting in a 250msec
delay between those two function calls in libvirt. It would be nice
if it could be totally fixed in qemu, though, so that libvirt didn't
need such a hack :-(
I don't mind the sleep, since we only need todo it if we see the
explicit QMP error reported. We can do 250ms sleeps in a loop
until it succeeeds, so there won't be an artifical delay that
users will actually notice.
Regards,
Daniel
3:29 p.m.
Daniel P. Berrange wrote:
On Wed, Jan 12, 2011 at 11:29:27PM -0500, Laine Stump wrote:
> On 01/12/2011 05:13 PM, Jim Fehlig wrote:
>
>> libvirt 0.8.7
>> qemu 0.13
>>
>> I'm looking into a problem with qemu save/restore via JSON monitor. On
>> restore, the vm is left in a paused state with following error returned
>> for 'cont' command
>>
>> An incoming migration is expected before this command can be executed
>>
>> I was trying to debug the issue in gdb, but stepping through the code
>> introduces enough delay between qemudStartVMDaemon() and doStartCPUs()
>> that the latter succeeds. Any suggestions on how to determine when it
>> is safe to call doStartCPUs() to prevent the above error? I don't see
>> this issue with the text monitor btw.
>>
> I'm pretty sure this is related to a bug I reported on qemu-devel
> last April:
>
> http://lists.gnu.org/archive/html/qemu-devel/2010-04/msg00635.html
>
> (be sure to read my own followup if you want a correct description
> of the circumstances). In this case libvirt was using the text
> monitor, and there was a race condition between qemudStartVMDaemon
> (which executes qemu with '-S -incoming') and doStartCPUs() (which
> issues a 'cont' command to the qemu monitor). The result would be
> that sometimes the 'cont' would be received and processed by qemu
> before the incoming migration had started, meaning that qemu would
> be executing garbage memory instead of the saved/restored image of
> the guest.
>
> The solution to this was posted to upstream qemu in July:
>
> http://lists.gnu.org/archive/html/qemu-devel/2010-07/msg01574.html
>
> and I believe is in qemu 0.13. That patch adds a check to the 'cont'
> command so that if '-incoming' was specified on the commandline,
> 'cont' will only execute after a migration has successfully
> completed, but will otherwise return an error.
>
> Actually, thinking about this "fix", it seems that it isn't really a
> solution, because instead of the guest starting up in an
> indeterminate state, doStartCPUs() will just fail (as you've seen)
> making the entire guest startup fail.
>
Yep, it wasn't really intended as a fix. It was intended to make
the error scenario clearly detectable, which has succeeded as per
Jim's report. The fact that QMP returned an error in this way,
means we can now reliably detect, usleep(1000), and then retry
the 'cont' again at which point we should succeed.
Something like the attached patch? I'm not quite sure about the retry
bounds (currently 1 second). In my testing, it always succeeds on the
second attempt - even with large memory vms.
Regards,
Jim
2:20 p.m.
On 01/13/2011 04:29 PM, Jim Fehlig wrote:
Daniel P. Berrange wrote:
> >
> > Yep, it wasn't really intended as a fix. It was intended to make
> > the error scenario clearly detectable, which has succeeded as per
> > Jim's report. The fact that QMP returned an error in this way,
> > means we can now reliably detect, usleep(1000), and then retry
> > the 'cont' again at which point we should succeed.
> >
Something like the attached patch? I'm not quite sure about the retry
bounds (currently 1 second). In my testing, it always succeeds on the
second attempt - even with large memory vms.
In my tests, 250ms was more than enough, so I'm guessing it's okay,
although there's probably no hurt in making it a bit larger - it's not
like this is something that repeats all day every day :-)
Would it be possible for you to add the same thing into the text monitor
version of the code, so both fixes would travel together as the patch
gets cherry-picked around?
0001-qemu-Retry-JSON-monitor-cont-cmd-on-MigrationExpecte.patch
> From ec9109b40a4b2c45035495e0e4f65824a92dcf3d Mon Sep 17 00:00:00 2001
From: Jim Fehlig<jfehlig(a)novell.com>
Date: Thu, 13 Jan 2011 12:52:23 -0700
Subject: [PATCH] qemu: Retry JSON monitor cont cmd on MigrationExpected error
When restoring a saved qemu instance via JSON monitor, the vm is
left in a paused state. Turns out the 'cont' cmd was failing with
"MigrationExpected" error class and "An incoming migration is
expected before this command can be executed" error description
due to migration (restore) not yet complete.
Detect if 'cont' cmd fails with "MigrationExpecte" error class and
retry 'cont' cmd.
---
src/qemu/qemu_monitor_json.c | 20 +++++++++++++++++---
1 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 7877731..63b736e 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -702,13 +702,27 @@ qemuMonitorJSONStartCPUs(qemuMonitorPtr mon,
int ret;
virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("cont", NULL);
virJSONValuePtr reply = NULL;
+ int i = 0, timeout = 3;
if (!cmd)
return -1;
- ret = qemuMonitorJSONCommand(mon, cmd,&reply);
+ do {
+ ret = qemuMonitorJSONCommand(mon, cmd,&reply);
- if (ret == 0)
- ret = qemuMonitorJSONCheckError(cmd, reply);
+ if (ret != 0)
+ break;
+
+ /* If no error, we're done */
+ if ((ret = qemuMonitorJSONCheckError(cmd, reply)) == 0)
+ break;
+
+ /* If error class is not MigrationExpected, we're done.
+ * Otherwise try 'cont' cmd again */
+ if (!qemuMonitorJSONHasError(reply, "MigrationExpected"))
+ break;
+
+ virJSONValueFree(reply);
+ } while ((++i<= timeout)&& (usleep(250000)<=0));
virJSONValueFree(cmd);
virJSONValueFree(reply);
Doesn't this end up doing a double-free of reply if it times out?
virJSONValueFree doesn't update the pointer that's free'd like VIR_FREE
does (it can't, since it's a function call rather than a macro).
3:26 p.m.
Laine Stump wrote:
On 01/13/2011 04:29 PM, Jim Fehlig wrote:
> Daniel P. Berrange wrote:
>> >
>> > Yep, it wasn't really intended as a fix. It was intended to make
>> > the error scenario clearly detectable, which has succeeded as per
>> > Jim's report. The fact that QMP returned an error in this way,
>> > means we can now reliably detect, usleep(1000), and then retry
>> > the 'cont' again at which point we should succeed.
>> >
> Something like the attached patch? I'm not quite sure about the retry
> bounds (currently 1 second). In my testing, it always succeeds on the
> second attempt - even with large memory vms.
In my tests, 250ms was more than enough, so I'm guessing it's okay,
although there's probably no hurt in making it a bit larger - it's not
like this is something that repeats all day every day :-)
Would it be possible for you to add the same thing into the text
monitor version of the code, so both fixes would travel together as
the patch gets cherry-picked around?
I can, but have not seen this issue with the text monitor. And the
error reporting is not so fine grained correct?
> 0001-qemu-Retry-JSON-monitor-cont-cmd-on-MigrationExpecte.patch
>
>
> > From ec9109b40a4b2c45035495e0e4f65824a92dcf3d Mon Sep 17 00:00:00 2001
> From: Jim Fehlig<jfehlig(a)novell.com>
> Date: Thu, 13 Jan 2011 12:52:23 -0700
> Subject: [PATCH] qemu: Retry JSON monitor cont cmd on
> MigrationExpected error
>
> When restoring a saved qemu instance via JSON monitor, the vm is
> left in a paused state. Turns out the 'cont' cmd was failing with
> "MigrationExpected" error class and "An incoming migration is
> expected before this command can be executed" error description
> due to migration (restore) not yet complete.
>
> Detect if 'cont' cmd fails with "MigrationExpecte" error class and
> retry 'cont' cmd.
> ---
> src/qemu/qemu_monitor_json.c | 20 +++++++++++++++++---
> 1 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 7877731..63b736e 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -702,13 +702,27 @@ qemuMonitorJSONStartCPUs(qemuMonitorPtr mon,
> int ret;
> virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("cont", NULL);
> virJSONValuePtr reply = NULL;
> + int i = 0, timeout = 3;
> if (!cmd)
> return -1;
>
> - ret = qemuMonitorJSONCommand(mon, cmd,&reply);
> + do {
> + ret = qemuMonitorJSONCommand(mon, cmd,&reply);
>
> - if (ret == 0)
> - ret = qemuMonitorJSONCheckError(cmd, reply);
> + if (ret != 0)
> + break;
> +
> + /* If no error, we're done */
> + if ((ret = qemuMonitorJSONCheckError(cmd, reply)) == 0)
> + break;
> +
> + /* If error class is not MigrationExpected, we're done.
> + * Otherwise try 'cont' cmd again */
> + if (!qemuMonitorJSONHasError(reply, "MigrationExpected"))
> + break;
> +
> + virJSONValueFree(reply);
> + } while ((++i<= timeout)&& (usleep(250000)<=0));
>
> virJSONValueFree(cmd);
> virJSONValueFree(reply);
Doesn't this end up doing a double-free of reply if it times out?
virJSONValueFree doesn't update the pointer that's free'd like
VIR_FREE does (it can't, since it's a function call rather than a macro).
virJSONValueFree() calls VIR_FREE() on the value passed to it, so reply
should be set to NULL when virJSONValueFree() returns.
While looking at the patch again, not sure I'm fond of the
while ((++i<= timeout) && (usleep(250000)<=0));
Perhaps should move the usleep inside the loop and clean that up a bit.
Regards,
Jim
4:24 a.m.
On 01/14/2011 04:26 PM, Jim Fehlig wrote:
Laine Stump wrote:
> On 01/13/2011 04:29 PM, Jim Fehlig wrote:
>> Daniel P. Berrange wrote:
>>>> Yep, it wasn't really intended as a fix. It was intended to make
>>>> the error scenario clearly detectable, which has succeeded as per
>>>> Jim's report. The fact that QMP returned an error in this way,
>>>> means we can now reliably detect, usleep(1000), and then retry
>>>> the 'cont' again at which point we should succeed.
>>>>
>> Something like the attached patch? I'm not quite sure about the retry
>> bounds (currently 1 second). In my testing, it always succeeds on the
>> second attempt - even with large memory vms.
> In my tests, 250ms was more than enough, so I'm guessing it's okay,
> although there's probably no hurt in making it a bit larger - it's not
> like this is something that repeats all day every day :-)
>
> Would it be possible for you to add the same thing into the text
> monitor version of the code, so both fixes would travel together as
> the patch gets cherry-picked around?
I can, but have not seen this issue with the text monitor. And the
error reporting is not so fine grained correct?
Truthfully I haven't spent any time with the JSON code, and only enough
with the text monitor to (locally) put in a rough hack for the same
problem - I just delayed 250ms unconditionally.
Maybe it is safest for your patch to just have what you're able to test for.
Of course my system isn't setup to test exactly this fix either - the
machine that displays the problem when resuming is still running
qemu-kvm-0.12.5.
>> - if (ret == 0)
>> - ret = qemuMonitorJSONCheckError(cmd, reply);
>> + if (ret != 0)
>> + break;
>> +
>> + /* If no error, we're done */
>> + if ((ret = qemuMonitorJSONCheckError(cmd, reply)) == 0)
>> + break;
>> +
>> + /* If error class is not MigrationExpected, we're done.
>> + * Otherwise try 'cont' cmd again */
>> + if (!qemuMonitorJSONHasError(reply, "MigrationExpected"))
>> + break;
>> +
>> + virJSONValueFree(reply);
>> + } while ((++i<= timeout)&& (usleep(250000)<=0));
>>
>> virJSONValueFree(cmd);
>> virJSONValueFree(reply);
> Doesn't this end up doing a double-free of reply if it times out?
> virJSONValueFree doesn't update the pointer that's free'd like
> VIR_FREE does (it can't, since it's a function call rather than a macro).
virJSONValueFree() calls VIR_FREE() on the value passed to it, so reply
should be set to NULL when virJSONValueFree() returns.
Yes, but it's pass by value, not reference, so VIR_FREE() is NULLing the
copy of the pointer that was passed as an argument, not the original
pointer itself.
While looking at the patch again, not sure I'm fond of the
while ((++i<= timeout)&& (usleep(250000)<=0));
Perhaps should move the usleep inside the loop and clean that up a bit.
12:15 p.m.
Laine Stump wrote:
On 01/14/2011 04:26 PM, Jim Fehlig wrote:
> Laine Stump wrote:
>> On 01/13/2011 04:29 PM, Jim Fehlig wrote:
>>> Daniel P. Berrange wrote:
>>>>> Yep, it wasn't really intended as a fix. It was intended to
make
>>>>> the error scenario clearly detectable, which has succeeded as per
>>>>> Jim's report. The fact that QMP returned an error in this way,
>>>>> means we can now reliably detect, usleep(1000), and then retry
>>>>> the 'cont' again at which point we should succeed.
>>>>>
>>> Something like the attached patch? I'm not quite sure about the retry
>>> bounds (currently 1 second). In my testing, it always succeeds on the
>>> second attempt - even with large memory vms.
>> In my tests, 250ms was more than enough, so I'm guessing it's okay,
>> although there's probably no hurt in making it a bit larger - it's not
>> like this is something that repeats all day every day :-)
>>
>> Would it be possible for you to add the same thing into the text
>> monitor version of the code, so both fixes would travel together as
>> the patch gets cherry-picked around?
> I can, but have not seen this issue with the text monitor. And the
> error reporting is not so fine grained correct?
Truthfully I haven't spent any time with the JSON code, and only
enough with the text monitor to (locally) put in a rough hack for the
same problem - I just delayed 250ms unconditionally.
Maybe it is safest for your patch to just have what you're able to
test for.
Agreed. What error do you get back from the text monitor when cont cmd
fails? I'd be interested in the following output from
$root/src/qemu/qemu_monitor_text.c:qemuMonitorCommandWithHandler()
VIR_DEBUG("Receive command reply ret=%d errno=%d %d bytes '%s'",
ret, msg.lastErrno, msg.rxLength, msg.rxBuffer);
Of course my system isn't setup to test exactly this fix either - the
machine that displays the problem when resuming is still running
qemu-kvm-0.12.5.
>>> - if (ret == 0)
>>> - ret = qemuMonitorJSONCheckError(cmd, reply);
>>> + if (ret != 0)
>>> + break;
>>> +
>>> + /* If no error, we're done */
>>> + if ((ret = qemuMonitorJSONCheckError(cmd, reply)) == 0)
>>> + break;
>>> +
>>> + /* If error class is not MigrationExpected, we're done.
>>> + * Otherwise try 'cont' cmd again */
>>> + if (!qemuMonitorJSONHasError(reply, "MigrationExpected"))
>>> + break;
>>> +
>>> + virJSONValueFree(reply);
>>> + } while ((++i<= timeout)&& (usleep(250000)<=0));
>>>
>>> virJSONValueFree(cmd);
>>> virJSONValueFree(reply);
>> Doesn't this end up doing a double-free of reply if it times out?
>> virJSONValueFree doesn't update the pointer that's free'd like
>> VIR_FREE does (it can't, since it's a function call rather than a
>> macro).
> virJSONValueFree() calls VIR_FREE() on the value passed to it, so reply
> should be set to NULL when virJSONValueFree() returns.
Yes, but it's pass by value, not reference, so VIR_FREE() is NULLing
the copy of the pointer that was passed as an argument, not the
original pointer itself.
Err, right. I'll fix this in v2 - but need to understand the text
monitor error first.
Thanks,
Jim
9:09 a.m.
On 01/18/2011 01:15 PM, Jim Fehlig wrote:
Laine Stump wrote:
> On 01/14/2011 04:26 PM, Jim Fehlig wrote:
>> Laine Stump wrote:
>>> On 01/13/2011 04:29 PM, Jim Fehlig wrote:
>>>> Daniel P. Berrange wrote:
>>>>>> Yep, it wasn't really intended as a fix. It was intended
to make
>>>>>> the error scenario clearly detectable, which has succeeded as
per
>>>>>> Jim's report. The fact that QMP returned an error in this
way,
>>>>>> means we can now reliably detect, usleep(1000), and then
retry
>>>>>> the 'cont' again at which point we should succeed.
>>>>>>
>>>> Something like the attached patch? I'm not quite sure about the
retry
>>>> bounds (currently 1 second). In my testing, it always succeeds on the
>>>> second attempt - even with large memory vms.
>>> In my tests, 250ms was more than enough, so I'm guessing it's okay,
>>> although there's probably no hurt in making it a bit larger - it's
not
>>> like this is something that repeats all day every day :-)
>>>
>>> Would it be possible for you to add the same thing into the text
>>> monitor version of the code, so both fixes would travel together as
>>> the patch gets cherry-picked around?
>> I can, but have not seen this issue with the text monitor. And the
>> error reporting is not so fine grained correct?
> Truthfully I haven't spent any time with the JSON code, and only
> enough with the text monitor to (locally) put in a rough hack for the
> same problem - I just delayed 250ms unconditionally.
>
> Maybe it is safest for your patch to just have what you're able to
> test for.
Agreed. What error do you get back from the text monitor when cont cmd
fails?
Right now the machine that experiences this failure is stuck on
qemu-0.12.5, so it doesn't fail, it "succeeds" :-) (since it doesn't
have the necessary qemu patch, when "cont" is run before the migration
has been able to start, it just believes that it was successful, and
starts the CPU up with garbage in the guest memory.)
I'll try to get qemu-0.13 installed on this machine as soon as I can,
but I'm trying to get something else fixed before Thursday.
I'd be interested in the following output from
$root/src/qemu/qemu_monitor_text.c:qemuMonitorCommandWithHandler()
VIR_DEBUG("Receive command reply ret=%d errno=%d %d bytes '%s'",
ret, msg.lastErrno, msg.rxLength, msg.rxBuffer);
> Of course my system isn't setup to test exactly this fix either - the
> machine that displays the problem when resuming is still running
> qemu-kvm-0.12.5.
>
>
>>>> - if (ret == 0)
>>>> - ret = qemuMonitorJSONCheckError(cmd, reply);
>>>> + if (ret != 0)
>>>> + break;
>>>> +
>>>> + /* If no error, we're done */
>>>> + if ((ret = qemuMonitorJSONCheckError(cmd, reply)) == 0)
>>>> + break;
>>>> +
>>>> + /* If error class is not MigrationExpected, we're done.
>>>> + * Otherwise try 'cont' cmd again */
>>>> + if (!qemuMonitorJSONHasError(reply,
"MigrationExpected"))
>>>> + break;
>>>> +
>>>> + virJSONValueFree(reply);
>>>> + } while ((++i<= timeout)&& (usleep(250000)<=0));
>>>>
>>>> virJSONValueFree(cmd);
>>>> virJSONValueFree(reply);
>>> Doesn't this end up doing a double-free of reply if it times out?
>>> virJSONValueFree doesn't update the pointer that's free'd like
>>> VIR_FREE does (it can't, since it's a function call rather than a
>>> macro).
>> virJSONValueFree() calls VIR_FREE() on the value passed to it, so reply
>> should be set to NULL when virJSONValueFree() returns.
>
> Yes, but it's pass by value, not reference, so VIR_FREE() is NULLing
> the copy of the pointer that was passed as an argument, not the
> original pointer itself.
Err, right. I'll fix this in v2 - but need to understand the text
monitor error first.
I'll do my best to get to it as soon as possible.
Thanks,
Jim
9:05 p.m.
Jim Fehlig wrote:
Agreed. What error do you get back from the text monitor when cont
cmd
fails? I'd be interested in the following output from
$root/src/qemu/qemu_monitor_text.c:qemuMonitorCommandWithHandler()
VIR_DEBUG("Receive command reply ret=%d errno=%d %d bytes '%s'",
ret, msg.lastErrno, msg.rxLength, msg.rxBuffer);
Since I cannot reproduce the issue with qemu 0.13 using the text
monitor, I hacked qemu to simulate the error and got the following from
the debug message
Receive command reply ret=0 errno=0 71 bytes 'An incoming migration is
expected before this command can be executed'
msg.lastErrno is 0 (which is why ret is 0), but we've received the "An
incoming migration ..." error message. The message is available in
reply variable in
$root/src/qemu/qemu_monitor_text.c:qemuMonitorTextStartCPUs(), but
that's not a very robust way to check for error. Seems qemu should
return an error in addition to the message. I'm not familiar with the
qemu monitor code, but I suspect the qemu patch you referenced [1] was
insufficient wrt the text monitor.
I think a patch addressing the issue in the JSON monitor, where we can
reliably detect the error, is a good start. Would that be okay? Also,
recall that I cannot reproduce the issue with qemu 0.13. Perhaps other
post-0.12.5 changes have "fixed" the bug in the text monitor.
Regards,
Jim
[1] http://lists.gnu.org/archive/html/qemu-devel/2010-07/msg01574.html
11:35 a.m.
Hello,
Am Donnerstag 20 Januar 2011 04:05:46 schrieb Jim Fehlig:
I think a patch addressing the issue in the JSON monitor, where we
can
reliably detect the error, is a good start. Would that be okay? Also,
recall that I cannot reproduce the issue with qemu 0.13.
We can reliably reproduct the error condition: It happens when the host is
loaded and an additional VM is started. Sometime you have to start a VM
several times, but we regularly experience paused VMs, which can only be
unpaused via
virsh qemu-monitor-command "$vm" "cont"
since libvirt thinks they are already running.
I normally hard-link the save image files to another name (*.PMH), so I can
repeatly restore the same state again and again. Since the disk image is NOT
snapshotted, it's content gets corrupted over time, but for testing it's
okay. I than execute the following loop until I detect the broken state:
while virsh qemu-monitor-command "$vm" "info status" | grep running
do
virsh destroy "$vm"
ln "$vm.save.PMH" "$vm.save"
virsh start "$vm"
sleep 1
done
Perhaps other post-0.12.5 changes have "fixed" the bug in
the text monitor.
This still happens with qemu-0.14 (and in this case libvirt-0.8.7):
17:50:19.586: 29813: warning : qemudStartVMDaemon:2864 : Executing
done /usr/bin/kvm
17:50:19.787: 29813: debug : qemuMonitorOpen:682 : New mon 0x7fad280068c0 fd
=29 watch=61
17:50:19.787: 29813: debug : qemuMonitorSetCapabilities:905 :
mon=0x7fad280068c0
17:50:19.787: 29813: debug : qemuMonitorGetPtyPaths:1706 : mon=0x7fad280068c0
17:50:19.787: 29813: debug : qemuMonitorCommandWithHandler:231 : Send
command 'info chardev' for write with FD -1
17:50:19.909: 29813: debug : qemuMonitorCommandWithHandler:236 : Receive
command reply ret=0 errno=0 83 bytes 'monitor:
filename=unix:/var/lib/libvirt/qemu/XXX.monitor,server
'
17:50:19.909: 29813: debug : qemuMonitorGetCPUInfo:985 : mon=0x7fad280068c0
17:50:19.909: 29813: debug : qemuMonitorCommandWithHandler:231 : Send
command 'info cpus' for write with FD -1
17:50:19.909: 29813: debug : qemuMonitorCommandWithHandler:236 : Receive
command reply ret=0 errno=0 49 bytes '* CPU #0: pc=0x00000000fffffff0
thread_id=5011
'
17:50:19.909: 29813: debug : qemuMonitorTextGetCPUInfo:448 : vcpu=0 pid=5011
17:50:19.909: 29813: debug : qemuMonitorSetBalloon:1116 : mon=0x7fad280068c0
newmem=1048576
17:50:19.909: 29813: debug : qemuMonitorCommandWithHandler:231 : Send
command 'balloon 1024' for write with FD -1
17:50:19.909: 29813: debug : qemuMonitorCommandWithHandler:236 : Receive
command reply ret=0 errno=0 0 bytes '(null)'
17:50:19.909: 29813: debug : qemuMonitorStartCPUs:926 : mon=0x7fad280068c0
17:50:19.910: 29813: debug : qemuMonitorCommandWithHandler:231 : Send
command 'cont' for write with FD -1
17:50:19.911: 29813: debug : qemuMonitorCommandWithHandler:236 : Receive
command reply ret=0 errno=0 71 bytes 'An incoming migration is expected
before this command can be executed
'
Sincerely
Philipp
--
Philipp Hahn Open Source Software Engineer hahn(a)univention.de
Univention GmbH Linux for Your Business fon: +49 421 22 232- 0
Mary-Somerville-Str.1 D-28359 Bremen fax: +49 421 22 232-99
http://www.univention.de/
4800
days inactive
4876
days old
11 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Jim Fehlig -
Laine Stump -
Philipp Hahn