[libvirt] [PATCH] qemu: better detection of crashed domains

When libvirtd is started and there is an unusable/not-connectable leftover from earlier started machine, it's more reasonable to say that the machine "crashed" if we know it was started with "-no-shutdown". This patch fixes that and also changes the other result (when machine was started without "-no-shutdown") to "unknown", because the previous "failed" reason means (according to include/libvirt/libvirt.h.in:174), that the machine failed to start. --- src/qemu/qemu_process.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2adf570..604c31b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3141,7 +3141,17 @@ error: * to remove danger of it ending up running twice if * user tries to start it again later */ - qemuProcessStop(driver, obj, 0, VIR_DOMAIN_SHUTOFF_FAILED); + if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) + /* If we couldn't get the monitor and qemu supports + * no-shutdown, we can safely say that the domain + * crashed ... */ + state = VIR_DOMAIN_SHUTOFF_CRASHED; + else + /* ... but if it doesn't we can't say what the state + * really is and FAILED means "failed to start" */ + state = VIR_DOMAIN_SHUTOFF_UNKNOWN; + + qemuProcessStop(driver, obj, 0, state); if (!obj->persistent) qemuDomainRemoveInactive(driver, obj); else -- 1.7.8.6

On 06/06/2012 09:03 AM, Martin Kletzander wrote:
When libvirtd is started and there is an unusable/not-connectable leftover from earlier started machine, it's more reasonable to say that the machine "crashed" if we know it was started with "-no-shutdown". This patch fixes that and also changes the other result (when machine was started without "-no-shutdown") to "unknown", because the previous "failed" reason means (according to include/libvirt/libvirt.h.in:174), that the machine failed to start. --- src/qemu/qemu_process.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2adf570..604c31b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3141,7 +3141,17 @@ error: * to remove danger of it ending up running twice if * user tries to start it again later */ - qemuProcessStop(driver, obj, 0, VIR_DOMAIN_SHUTOFF_FAILED); + if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) + /* If we couldn't get the monitor and qemu supports + * no-shutdown, we can safely say that the domain + * crashed ... */ + state = VIR_DOMAIN_SHUTOFF_CRASHED; + else
Style nit - since you have lengthy comments, each branch now occupies multiple lines (even if it is only one statement); I would use {} on both the if and else branches for that reason. ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 06/06/2012 05:22 PM, Eric Blake wrote:
On 06/06/2012 09:03 AM, Martin Kletzander wrote:
When libvirtd is started and there is an unusable/not-connectable leftover from earlier started machine, it's more reasonable to say that the machine "crashed" if we know it was started with "-no-shutdown". This patch fixes that and also changes the other result (when machine was started without "-no-shutdown") to "unknown", because the previous "failed" reason means (according to include/libvirt/libvirt.h.in:174), that the machine failed to start. --- src/qemu/qemu_process.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2adf570..604c31b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3141,7 +3141,17 @@ error: * to remove danger of it ending up running twice if * user tries to start it again later */ - qemuProcessStop(driver, obj, 0, VIR_DOMAIN_SHUTOFF_FAILED); + if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) + /* If we couldn't get the monitor and qemu supports + * no-shutdown, we can safely say that the domain + * crashed ... */ + state = VIR_DOMAIN_SHUTOFF_CRASHED; + else
Style nit - since you have lengthy comments, each branch now occupies multiple lines (even if it is only one statement); I would use {} on both the if and else branches for that reason.
Fixed ...
ACK.
... and pushed, thanks.
participants (2)
-
Eric Blake
-
Martin Kletzander