Skip to contentSkip to conversation

reenable vm_clock when resuming all vcpus

PATCH Applied Wen Congyang 5 participants 9 messages Nov 4, 2011 – Nov 10, 2011

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 reenable vm_clock when resuming all vcpus

We disable vm_clock when pausing all vcpus, but we forget to reenable it when resuming all vcpus. It will cause that the guest can not be rebooted.

---
 cpus.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
cpus.c +1 -0
index 6aff425..82530c4 100644
@@ -891,6 +891,7 @@ void resume_all_vcpus(void)
 {
     CPUState *penv = first_cpu;
 
+    qemu_clock_enable(vm_clock, true);
     while (penv) {
         penv->stop = 0;
         penv->stopped = 0;
Signature · 1 line
1.7.1
#1 lore raw
Quoted 4 lines · Wen Congyang

We disable vm_clock when pausing all vcpus, but we forget to reenable it when resuming all vcpus. It will cause that the guest can not be rebooted.

Actually i also met this issue when rebooting the guest. When the guest is rebooted, it will crash.

Quoted 20 lines

---
 cpus.c |    1 +

 1 files changed, 1 insertions(+), 0 deletions(-)
cpus.c +1 -1
index 6aff425..82530c4 100644
@@ -891,6 +891,7 @@ void resume_all_vcpus(void)
  {
     CPUState *penv = first_cpu;
 
+    qemu_clock_enable(vm_clock, true);
     while (penv) {
         penv->stop = 0;
         penv->stopped = 0;
--
Quoted 2 lines

1.7.1

--
Regards,

Zhi Yong Wu

#2 lore raw

At 11/04/2011 10:48 AM, Zhi Yong Wu Write:

Quoted 7 lines · Wen Congyang
Quoted 4 lines

We disable vm_clock when pausing all vcpus, but we forget to reenable it when resuming all vcpus. It will cause that the guest can not be rebooted.

Actually i also met this issue when rebooting the guest. When the guest is rebooted, it will crash.

When I reboot the guest, it will hang.

Does this patch solve your problem?

Quoted 31 lines
Quoted 21 lines
---
 cpus.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
cpus.c +1 -1
index 6aff425..82530c4 100644
@@ -891,6 +891,7 @@ void resume_all_vcpus(void)
 {
    CPUState *penv = first_cpu;
 
+    qemu_clock_enable(vm_clock, true);
    while (penv) {
        penv->stop = 0;
        penv->stopped = 0;
--
Quoted 2 lines

1.7.1

--
Regards,

Zhi Yong Wu

#3 lore raw
Quoted 6 lines · Wen Congyang

At 11/04/2011 10:48 AM, Zhi Yong Wu Write:

Quoted 5 lines · Wen Congyang
Quoted 4 lines

We disable vm_clock when pausing all vcpus, but we forget to reenable it when resuming all vcpus. It will cause that the guest can not be rebooted.

Quoted 6 lines
Quoted 2 lines

Actually i also met this issue when rebooting the guest. When the guest is rebooted, it will crash.

When I reboot the guest, it will hang.

Does this patch solve your problem?

Next Monday, i will do one simple testing for this.

Quoted 34 lines
Quoted 32 lines
Quoted 22 lines

---
 cpus.c |    1 +

 1 files changed, 1 insertions(+), 0 deletions(-)
cpus.c +1 -1
index 6aff425..82530c4 100644
@@ -891,6 +891,7 @@ void resume_all_vcpus(void)
  {
     CPUState *penv = first_cpu;
 
+    qemu_clock_enable(vm_clock, true);
     while (penv) {
         penv->stop = 0;
         penv->stopped = 0;
--
Quoted 2 lines

1.7.1

--
Regards,

Zhi Yong Wu

Signature · 3 lines
Regards,

Zhi Yong Wu
Tested-by
#4 lore raw
In reply to Wen Congyang
Quoted 6 lines · Wen Congyang

At 11/04/2011 10:48 AM, Zhi Yong Wu Write:

Quoted 5 lines · Wen Congyang
Quoted 4 lines

We disable vm_clock when pausing all vcpus, but we forget to reenable it when resuming all vcpus. It will cause that the guest can not be rebooted.

Quoted 6 lines
Quoted 2 lines

Actually i also met this issue when rebooting the guest. When the guest is rebooted, it will crash.

When I reboot the guest, it will hang.

Does this patch solve your problem?

Yeah, it can work for my guest now.

BTW: If the patch is not applied, when guest is rebooted, fedora guest will crash, while rh6.1 guest will hang up.

Quoted 34 lines
Quoted 32 lines
Quoted 22 lines

---
 cpus.c |    1 +

 1 files changed, 1 insertions(+), 0 deletions(-)
cpus.c +1 -1
index 6aff425..82530c4 100644
@@ -891,6 +891,7 @@ void resume_all_vcpus(void)
  {
     CPUState *penv = first_cpu;
 
+    qemu_clock_enable(vm_clock, true);
     while (penv) {
         penv->stop = 0;
         penv->stopped = 0;
--
Quoted 2 lines

1.7.1

--
Regards,

Zhi Yong Wu

Signature · 3 lines
Regards,

Zhi Yong Wu
Reviewed-by on v1 1/1
#5 lore raw
In reply to Wen Congyang
Quoted 15 lines · Wen Congyang

We disable vm_clock when pausing all vcpus, but we forget to reenable it when resuming all vcpus. It will cause that the guest can not be rebooted.

---
  cpus.c |    1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
cpus.c +0 -0
index 6aff425..82530c4 100644

cpus.c void resume_all_vcpus(void) · line 897

Quoted 7 lines
cpus.c +1 -0
@@ -891,6 +891,7 @@ void resume_all_vcpus(void)
  {
      CPUState *penv = first_cpu;
 
+    qemu_clock_enable(vm_clock, true);
      while (penv) {
          penv->stop = 0;
          penv->stopped = 0;

The code I have now works for TCG, but of course not for KVM. Thanks.

Paolo

#6 lore raw
In reply to Wen Congyang
Quoted 5 lines · Wen Congyang

We disable vm_clock when pausing all vcpus, but we forget to reenable it when resuming all vcpus. It will cause that the guest can not be rebooted.

Applied. Thanks.

Regards,

Anthony Liguori

Quoted 17 lines
---
  cpus.c |    1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
cpus.c +1 -0
index 6aff425..82530c4 100644
@@ -891,6 +891,7 @@ void resume_all_vcpus(void)
  {
      CPUState *penv = first_cpu;
 
+    qemu_clock_enable(vm_clock, true);
      while (penv) {
          penv->stop = 0;
          penv->stopped = 0;
#7 lore raw
Quoted 8 lines · Anthony Liguori
Quoted 5 lines · Wen Congyang

We disable vm_clock when pausing all vcpus, but we forget to reenable it when resuming all vcpus. It will cause that the guest can not be rebooted.

Applied. Thanks.

FYI, I found out that this patch solved an issue we were having in our Autotest runs: https://bugs.launchpad.net/qemu/+bug/886255

But the reason it fixed the issue are a mystery to me. The symptoms were:

- A RHEL 6.1 guest was installed
- After booting, sshd created the ssh keys normally
- Guest was rebooted (I was not seeing the "guest can't be rebooted" issue mentioned on the description of this patch)
- After rebooting, the ssh key files on /etc/ssh had size 0 for some unknown reason
- sshd failed to re-create the keys because of the empty key files, and failed to start
- The immediate reason was because the (empty) public key file has selinux context that can't be written by ssh-keygen, but that only happened because the files become empty.

For some reason, the problem stopped happening after this commit. I have no idea why it fixed the issue. I don't know if it fixed the actual bug that caused the problem, or the bug is timing-dependent and for some reason this committed just changed the timing conditions and the bug is still hidden in qemu.

The bug was 100% reproducible on the machine I was testing, previously, and after this commit it didn't happen anymore. I double-checked by re-testing this commit (47113ab6), and the previous one (f67ab77a) twice, after bisecting.

(If anybody has some possible explanation on how this patch could have any impact on the above symptoms, or want to investigate further, please let me know. I think I will stop debugging this, move on, and finally stress-test my qemu_fclose() series, now.)

Signature · 1 line
Eduardo
#8 lore raw
Quoted 9 lines · Eduardo Habkost

The bug was 100% reproducible on the machine I was testing, previously, and after this commit it didn't happen anymore. I double-checked by re-testing this commit (47113ab6), and the previous one (f67ab77a) twice, after bisecting.

(If anybody has some possible explanation on how this patch could have any impact on the above symptoms, or want to investigate further, please let me know. I think I will stop debugging this, move on, and finally stress-test my qemu_fclose() series, now.)

I'm not sure about how the patch could fix this, but I'm not surprised that Linux shows the bug in more mysterious ways than Windows. That's because Windows uses the (in-QEMU) HPET for timing and Linux by default uses the (in-kernel) kvmclock.

Paolo