[libvirt] [PATCH] Ignore qemu STOP event when stopping CPUs
From: Jiri Denemark <jdenemar@redhat.com> With JSON qemu monitor, we get a STOP event from qemu whenever qemu stops guests CPUs. The downside of it is that vm->state is changed to PAUSED and a new generic paused event is send to applications. However, when we ask qemu to stop the CPUs we are not really interested in qemu event and we usually want to issue a more specific event. By setting vm->status to PAUSED before actually sending the request to qemu (and resetting it back if the request fails) we can ignore the event since the event handler does nothing when the guest is already paused. This solution is quite hacky but unfortunately it's the best solution which I was able to come up with and it doesn't introduce a race condition. --- src/qemu/qemu_driver.c | 25 +++++++++++++++++++------ 1 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3c4876e..8ccf0b7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4117,13 +4117,16 @@ static int qemudDomainSuspend(virDomainPtr dom) { } if (vm->state != VIR_DOMAIN_PAUSED) { int rc; + int state = vm->state; + vm->state = VIR_DOMAIN_PAUSED; qemuDomainObjEnterMonitorWithDriver(driver, vm); rc = qemuMonitorStopCPUs(priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); - if (rc < 0) + if (rc < 0) { + vm->state = state; goto endjob; - vm->state = VIR_DOMAIN_PAUSED; + } event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_SUSPENDED, VIR_DOMAIN_EVENT_SUSPENDED_PAUSED); @@ -4491,8 +4494,10 @@ qemuDomainMigrateOffline(struct qemud_driver *driver, virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; + int state = vm->state; int ret; + vm->state = VIR_DOMAIN_PAUSED; qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorStopCPUs(priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -4500,13 +4505,13 @@ qemuDomainMigrateOffline(struct qemud_driver *driver, if (ret == 0) { virDomainEventPtr event; - vm->state = VIR_DOMAIN_PAUSED; event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_SUSPENDED, VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED); if (event) qemuDomainEventQueue(driver, event); - } + } else + vm->state = state; return ret; } @@ -4743,13 +4748,14 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path, /* Pause */ if (vm->state == VIR_DOMAIN_RUNNING) { header.was_running = 1; + vm->state = VIR_DOMAIN_PAUSED; qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuMonitorStopCPUs(priv->mon) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); + vm->state = VIR_DOMAIN_RUNNING; goto endjob; } qemuDomainObjExitMonitorWithDriver(driver, vm); - vm->state = VIR_DOMAIN_PAUSED; } /* Get XML for the domain */ @@ -5167,9 +5173,11 @@ static int qemudDomainCoreDump(virDomainPtr dom, /* Pause domain for non-live dump */ if (!(flags & VIR_DUMP_LIVE) && vm->state == VIR_DOMAIN_RUNNING) { + vm->state = VIR_DOMAIN_PAUSED; qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuMonitorStopCPUs(priv->mon) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); + vm->state = VIR_DOMAIN_RUNNING; goto endjob; } qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -5214,6 +5222,7 @@ endjob: "%s", _("resuming after dump failed")); } qemuDomainObjExitMonitorWithDriver(driver, vm); + vm->state = VIR_DOMAIN_RUNNING; } if (qemuDomainObjEndJob(vm) == 0) @@ -11056,12 +11065,16 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, /* qemu unconditionally starts the domain running again after * loadvm, so let's pause it to keep consistency */ + int state = vm->state; priv = vm->privateData; + vm->state = VIR_DOMAIN_PAUSED; qemuDomainObjEnterMonitorWithDriver(driver, vm); rc = qemuMonitorStopCPUs(priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); - if (rc < 0) + if (rc < 0) { + vm->state = state; goto cleanup; + } } event = virDomainEventNewFromObj(vm, -- 1.7.0.4
On Tue, Apr 27, 2010 at 11:20:39AM +0200, jdenemar@redhat.com wrote:
From: Jiri Denemark <jdenemar@redhat.com>
With JSON qemu monitor, we get a STOP event from qemu whenever qemu stops guests CPUs. The downside of it is that vm->state is changed to PAUSED and a new generic paused event is send to applications. However, when we ask qemu to stop the CPUs we are not really interested in qemu event and we usually want to issue a more specific event.
By setting vm->status to PAUSED before actually sending the request to qemu (and resetting it back if the request fails) we can ignore the event since the event handler does nothing when the guest is already paused. This solution is quite hacky but unfortunately it's the best solution which I was able to come up with and it doesn't introduce a race condition. --- src/qemu/qemu_driver.c | 25 +++++++++++++++++++------ 1 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3c4876e..8ccf0b7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4117,13 +4117,16 @@ static int qemudDomainSuspend(virDomainPtr dom) { } if (vm->state != VIR_DOMAIN_PAUSED) { int rc; + int state = vm->state;
+ vm->state = VIR_DOMAIN_PAUSED; qemuDomainObjEnterMonitorWithDriver(driver, vm); rc = qemuMonitorStopCPUs(priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); - if (rc < 0) + if (rc < 0) { + vm->state = state; goto endjob; - vm->state = VIR_DOMAIN_PAUSED; + } event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_SUSPENDED, VIR_DOMAIN_EVENT_SUSPENDED_PAUSED); @@ -4491,8 +4494,10 @@ qemuDomainMigrateOffline(struct qemud_driver *driver, virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; + int state = vm->state; int ret;
+ vm->state = VIR_DOMAIN_PAUSED; qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorStopCPUs(priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -4500,13 +4505,13 @@ qemuDomainMigrateOffline(struct qemud_driver *driver, if (ret == 0) { virDomainEventPtr event;
- vm->state = VIR_DOMAIN_PAUSED; event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_SUSPENDED, VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED); if (event) qemuDomainEventQueue(driver, event); - } + } else + vm->state = state;
return ret; } @@ -4743,13 +4748,14 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path, /* Pause */ if (vm->state == VIR_DOMAIN_RUNNING) { header.was_running = 1; + vm->state = VIR_DOMAIN_PAUSED; qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuMonitorStopCPUs(priv->mon) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); + vm->state = VIR_DOMAIN_RUNNING; goto endjob; } qemuDomainObjExitMonitorWithDriver(driver, vm); - vm->state = VIR_DOMAIN_PAUSED; }
/* Get XML for the domain */ @@ -5167,9 +5173,11 @@ static int qemudDomainCoreDump(virDomainPtr dom,
/* Pause domain for non-live dump */ if (!(flags & VIR_DUMP_LIVE) && vm->state == VIR_DOMAIN_RUNNING) { + vm->state = VIR_DOMAIN_PAUSED; qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuMonitorStopCPUs(priv->mon) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); + vm->state = VIR_DOMAIN_RUNNING; goto endjob; } qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -5214,6 +5222,7 @@ endjob: "%s", _("resuming after dump failed")); } qemuDomainObjExitMonitorWithDriver(driver, vm); + vm->state = VIR_DOMAIN_RUNNING; }
if (qemuDomainObjEndJob(vm) == 0) @@ -11056,12 +11065,16 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, /* qemu unconditionally starts the domain running again after * loadvm, so let's pause it to keep consistency */ + int state = vm->state; priv = vm->privateData; + vm->state = VIR_DOMAIN_PAUSED; qemuDomainObjEnterMonitorWithDriver(driver, vm); rc = qemuMonitorStopCPUs(priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); - if (rc < 0) + if (rc < 0) { + vm->state = state; goto cleanup; + } }
event = virDomainEventNewFromObj(vm,
Hum, that sounds reasonnable, yes. All those operations should already emit a more specific event, if that's the case, ACK. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
With JSON qemu monitor, we get a STOP event from qemu whenever qemu stops guests CPUs. The downside of it is that vm->state is changed to PAUSED and a new generic paused event is send to applications. However, when we ask qemu to stop the CPUs we are not really interested in qemu event and we usually want to issue a more specific event.
By setting vm->status to PAUSED before actually sending the request to qemu (and resetting it back if the request fails) we can ignore the event since the event handler does nothing when the guest is already paused. This solution is quite hacky but unfortunately it's the best solution which I was able to come up with and it doesn't introduce a race condition.
Hum, that sounds reasonnable, yes. All those operations should already emit a more specific event, if that's the case, ACK.
The only place where we don't emit any event after stopping CPUs is in qemudDomainCoreDump when making non-live dump. But we don't do that when talking to qemu using text monitor either. If we decide we should emit such event followed by a resume event after dump completes, we can make a separate patch for that. Jirka
With JSON qemu monitor, we get a STOP event from qemu whenever qemu stops guests CPUs. The downside of it is that vm->state is changed to PAUSED and a new generic paused event is send to applications. However, when we ask qemu to stop the CPUs we are not really interested in qemu event and we usually want to issue a more specific event.
By setting vm->status to PAUSED before actually sending the request to qemu (and resetting it back if the request fails) we can ignore the event since the event handler does nothing when the guest is already paused. This solution is quite hacky but unfortunately it's the best solution which I was able to come up with and it doesn't introduce a race condition.
Hum, that sounds reasonnable, yes. All those operations should already emit a more specific event, if that's the case, ACK.
OK, I pushed this patch. Thanks. Jirka
On Tue, 27 Apr 2010 11:20:39 +0200 jdenemar@redhat.com wrote:
From: Jiri Denemark <jdenemar@redhat.com>
With JSON qemu monitor, we get a STOP event from qemu whenever qemu stops guests CPUs. The downside of it is that vm->state is changed to PAUSED and a new generic paused event is send to applications. However, when we ask qemu to stop the CPUs we are not really interested in qemu event and we usually want to issue a more specific event.
Can you give an example? Say, "guest is suspended"? If this is the case, something you have to consider is whether libvirt should add high-level events on top of low-level ones. Or, to make it more general, what should libvirt report to applications wrt events?
By setting vm->status to PAUSED before actually sending the request to qemu (and resetting it back if the request fails) we can ignore the event since the event handler does nothing when the guest is already paused. This solution is quite hacky but unfortunately it's the best solution which I was able to come up with and it doesn't introduce a race condition.
You could extend the low-level driver to accept a mask of events to be ignored.
On Tue, Apr 27, 2010 at 03:50:20PM -0300, Luiz Capitulino wrote:
On Tue, 27 Apr 2010 11:20:39 +0200 jdenemar@redhat.com wrote:
From: Jiri Denemark <jdenemar@redhat.com>
With JSON qemu monitor, we get a STOP event from qemu whenever qemu stops guests CPUs. The downside of it is that vm->state is changed to PAUSED and a new generic paused event is send to applications. However, when we ask qemu to stop the CPUs we are not really interested in qemu event and we usually want to issue a more specific event.
Can you give an example? Say, "guest is suspended"?
If this is the case, something you have to consider is whether libvirt should add high-level events on top of low-level ones.
Or, to make it more general, what should libvirt report to applications wrt events?
By setting vm->status to PAUSED before actually sending the request to qemu (and resetting it back if the request fails) we can ignore the event since the event handler does nothing when the guest is already paused. This solution is quite hacky but unfortunately it's the best solution which I was able to come up with and it doesn't introduce a race condition.
You could extend the low-level driver to accept a mask of events to be ignored.
The core problem here is that QEMU only emits a STOP event, with no corresponding CONT event. If QEMU had been doing both, this patch would not be neccessary, because we'd see both transitions from QEMU instead of just one direction. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Tue, 27 Apr 2010 20:01:06 +0100 "Daniel P. Berrange" <berrange@redhat.com> wrote:
On Tue, Apr 27, 2010 at 03:50:20PM -0300, Luiz Capitulino wrote:
On Tue, 27 Apr 2010 11:20:39 +0200 jdenemar@redhat.com wrote:
From: Jiri Denemark <jdenemar@redhat.com>
With JSON qemu monitor, we get a STOP event from qemu whenever qemu stops guests CPUs. The downside of it is that vm->state is changed to PAUSED and a new generic paused event is send to applications. However, when we ask qemu to stop the CPUs we are not really interested in qemu event and we usually want to issue a more specific event.
Can you give an example? Say, "guest is suspended"?
If this is the case, something you have to consider is whether libvirt should add high-level events on top of low-level ones.
Or, to make it more general, what should libvirt report to applications wrt events?
By setting vm->status to PAUSED before actually sending the request to qemu (and resetting it back if the request fails) we can ignore the event since the event handler does nothing when the guest is already paused. This solution is quite hacky but unfortunately it's the best solution which I was able to come up with and it doesn't introduce a race condition.
You could extend the low-level driver to accept a mask of events to be ignored.
The core problem here is that QEMU only emits a STOP event, with no corresponding CONT event. If QEMU had been doing both, this patch would not be neccessary, because we'd see both transitions from QEMU instead of just one direction.
Oh, I already have a patch that introduces it, it's part of the savevm/loadvm/delvm series (which is giving me a bit of headache ATM). I will cherry-pick the patch and submit it later today.
By setting vm->status to PAUSED before actually sending the request to qemu (and resetting it back if the request fails) we can ignore the event since the event handler does nothing when the guest is already paused. This solution is quite hacky but unfortunately it's the best solution which I was able to come up with and it doesn't introduce a race condition.
You could extend the low-level driver to accept a mask of events to be ignored.
The core problem here is that QEMU only emits a STOP event, with no corresponding CONT event. If QEMU had been doing both, this patch would not be neccessary, because we'd see both transitions from QEMU instead of just one direction.
Actually, I don't think CONT event would help in this case. When libvirt is issuing a command to stop qemu emulation, it knows if it failed or succeeded synchronously and may set guest's state appropriately. Having asynchronously processed handler of STOP/CONT event set the guest state is not a good idea. We dealt with code written in such a way before and I guess no-one wants to get back to it :-) So I think we will eventually need to introduce event mask into monitor code. Jirka
participants (5)
-
Daniel P. Berrange -
Daniel Veillard -
jdenemar＠redhat.com -
Jiri Denemark
-
Luiz Capitulino