[libvirt] [PATCH 1/2] add --crash support to "virsh dump"
This patch adds the --crash option (already present in "xm dump-core") to "virsh dump". virDomainCoreDump already has a flags argument, so the API/ABI is untouched. * include/libvirt/libvirt.h.in (virDomainCoreDumpFlags): New. * src/test/test_driver.c (testDomainCoreDump): Do not crash after dump unless VIR_DUMP_CRASH is given. * src/qemu/qemu_driver.c (qemudDomainCoreDump): Shutdown the domain instead of restarting it if --crash is passed. * src/xen/xend_internal.c (xenDaemonDomainCoreDump): Support --crash. * tools/virsh.c (opts_dump): Add --crash. (cmdDump): Map it to flags for virDomainCoreDump and pass them. --- include/libvirt/libvirt.h.in | 5 +++++ src/qemu/qemu_driver.c | 17 ++++++++++++++++- src/test/test_driver.c | 19 ++++++++++--------- src/xen/xend_internal.c | 6 ++++-- tools/virsh.c | 7 ++++++- 5 files changed, 41 insertions(+), 13 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5bc7694..c04b552 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -334,6 +334,11 @@ struct _virDomainInterfaceStats { typedef virDomainInterfaceStatsStruct *virDomainInterfaceStatsPtr; +/* Domain core dump flags. */ +typedef enum { + VIR_DUMP_CRASH = (1 << 0), /* crash after dump */ +} virDomainCoreDumpFlags; + /* Domain migration flags. */ typedef enum { VIR_MIGRATE_LIVE = (1 << 0), /* live migration */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 92d4629..8e80144 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3543,6 +3543,7 @@ static int qemudDomainCoreDump(virDomainPtr dom, virDomainObjPtr vm; int resume = 0, paused = 0; int ret = -1, fd = -1; + virDomainEventPtr event = NULL; const char *args[] = { "cat", NULL, @@ -3633,10 +3634,17 @@ static int qemudDomainCoreDump(virDomainPtr dom, goto endjob; endjob: + if ((ret == 0) && (flags & VIR_DUMP_CRASH)) { + qemudShutdownVMDaemon(dom->conn, driver, vm); + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_CRASHED); + } + /* Since the monitor is always attached to a pty for libvirt, it will support synchronous operations so we always get here after the migration is complete. */ - if (resume && paused) { + else if (resume && paused) { qemuDomainObjEnterMonitor(vm); if (qemuMonitorStartCPUs(priv->mon, dom->conn) < 0) { if (virGetLastError() == NULL) @@ -3647,12 +3655,19 @@ endjob: } qemuDomainObjEndJob(vm); + if ((ret == 0) && (flags & VIR_DUMP_CRASH) && !vm->persistent) { + virDomainRemoveInactive(&driver->domains, + vm); + vm = NULL; + } cleanup: if (ret != 0) unlink(path); if (vm) virDomainObjUnlock(vm); + if (event) + qemuDomainEventQueue(driver, event); return ret; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 35f7571..7db9a4c 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1911,15 +1911,16 @@ static int testDomainCoreDump(virDomainPtr domain, goto cleanup; } - testDomainShutdownState(domain, privdom); - event = virDomainEventNewFromObj(privdom, - VIR_DOMAIN_EVENT_STOPPED, - VIR_DOMAIN_EVENT_STOPPED_CRASHED); - - if (!privdom->persistent) { - virDomainRemoveInactive(&privconn->domains, - privdom); - privdom = NULL; + if (flags & VIR_DUMP_CRASH) { + testDomainShutdownState(domain, privdom); + event = virDomainEventNewFromObj(privdom, + VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_CRASHED); + if (!privdom->persistent) { + virDomainRemoveInactive(&privconn->domains, + privdom); + privdom = NULL; + } } ret = 0; diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 66d2e7f..360390d 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -3243,8 +3243,10 @@ xenDaemonDomainCoreDump(virDomainPtr domain, const char *filename, return(-1); } - return xend_op(domain->conn, domain->name, "op", "dump", "file", filename, - "live", "0", "crash", "0", NULL); + return xend_op(domain->conn, domain->name, + "op", "dump", "file", filename, "live", "0", + "crash", (flags & VIR_DUMP_CRASH ? "1" : "0"), + NULL); } /** diff --git a/tools/virsh.c b/tools/virsh.c index 9faac35..65eaa3b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1431,6 +1431,7 @@ static const vshCmdInfo info_dump[] = { }; static const vshCmdOptDef opts_dump[] = { + {"crash", VSH_OT_BOOL, 0, gettext_noop("crash the domain after core dump")}, {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")}, {"file", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("where to dump the core")}, {NULL, 0, 0, NULL} @@ -1443,6 +1444,7 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) char *name; char *to; int ret = TRUE; + int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -1453,7 +1455,10 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return FALSE; - if (virDomainCoreDump(dom, to, 0) == 0) { + if (vshCommandOptBool (cmd, "crash")) + flags |= VIR_DUMP_CRASH; + + if (virDomainCoreDump(dom, to, flags) == 0) { vshPrint(ctl, _("Domain %s dumped to %s\n"), name, to); } else { vshError(ctl, _("Failed to core dump domain %s to %s"), name, to); -- 1.6.5.2
This is trivial for QEMU since you just have to not stop the vm before starting the dump. In Xen it is buggy, so I chose to not support it. * src/qemu/qemu_driver.c (qemudDomainCoreDump): Support live dumping. --- include/libvirt/libvirt.h.in | 1 + src/qemu/qemu_driver.c | 7 +++---- tools/virsh.c | 3 +++ 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c04b552..b4a7ef1 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -337,6 +337,7 @@ typedef virDomainInterfaceStatsStruct *virDomainInterfaceStatsPtr; /* Domain core dump flags. */ typedef enum { VIR_DUMP_CRASH = (1 << 0), /* crash after dump */ + VIR_DUMP_LIVE = (1 << 1), /* live dump */ } virDomainCoreDumpFlags; /* Domain migration flags. */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8e80144..7de3c45 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3597,15 +3597,14 @@ static int qemudDomainCoreDump(virDomainPtr dom, driver->securityDriver->domainSetSavedStateLabel(dom->conn, vm, path) == -1) goto endjob; - /* Migrate will always stop the VM, so once we support live dumping - the resume condition will stay the same, independent of whether - the stop command is issued. */ + /* Migrate will always stop the VM, so the resume condition is + independent of whether the stop command is issued. */ resume = (vm->state == VIR_DOMAIN_RUNNING); qemuDomainObjPrivatePtr priv = vm->privateData; /* Pause domain for non-live dump */ - if (vm->state == VIR_DOMAIN_RUNNING) { + if (!(flags & VIR_DUMP_LIVE) && vm->state == VIR_DOMAIN_RUNNING) { qemuDomainObjEnterMonitor(vm); if (qemuMonitorStopCPUs(priv->mon) < 0) { qemuDomainObjExitMonitor(vm); diff --git a/tools/virsh.c b/tools/virsh.c index 65eaa3b..fcbd4e6 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1431,6 +1431,7 @@ static const vshCmdInfo info_dump[] = { }; static const vshCmdOptDef opts_dump[] = { + {"live", VSH_OT_BOOL, 0, gettext_noop("perform a live core dump if supported")}, {"crash", VSH_OT_BOOL, 0, gettext_noop("crash the domain after core dump")}, {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")}, {"file", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("where to dump the core")}, @@ -1455,6 +1456,8 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return FALSE; + if (vshCommandOptBool (cmd, "live")) + flags |= VIR_DUMP_LIVE; if (vshCommandOptBool (cmd, "crash")) flags |= VIR_DUMP_CRASH; -- 1.6.5.2
On Mon, Nov 30, 2009 at 10:11:11AM +0100, Paolo Bonzini wrote:
This is trivial for QEMU since you just have to not stop the vm before starting the dump.
In Xen it is buggy, so I chose to not support it.
And for the test driver there ain't any difference anyway ...
* src/qemu/qemu_driver.c (qemudDomainCoreDump): Support live dumping. --- include/libvirt/libvirt.h.in | 1 + src/qemu/qemu_driver.c | 7 +++---- tools/virsh.c | 3 +++ 3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c04b552..b4a7ef1 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -337,6 +337,7 @@ typedef virDomainInterfaceStatsStruct *virDomainInterfaceStatsPtr; /* Domain core dump flags. */ typedef enum { VIR_DUMP_CRASH = (1 << 0), /* crash after dump */ + VIR_DUMP_LIVE = (1 << 1), /* live dump */ } virDomainCoreDumpFlags;
/* Domain migration flags. */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8e80144..7de3c45 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3597,15 +3597,14 @@ static int qemudDomainCoreDump(virDomainPtr dom, driver->securityDriver->domainSetSavedStateLabel(dom->conn, vm, path) == -1) goto endjob;
- /* Migrate will always stop the VM, so once we support live dumping - the resume condition will stay the same, independent of whether - the stop command is issued. */ + /* Migrate will always stop the VM, so the resume condition is + independent of whether the stop command is issued. */ resume = (vm->state == VIR_DOMAIN_RUNNING);
qemuDomainObjPrivatePtr priv = vm->privateData;
/* Pause domain for non-live dump */ - if (vm->state == VIR_DOMAIN_RUNNING) { + if (!(flags & VIR_DUMP_LIVE) && vm->state == VIR_DOMAIN_RUNNING) { qemuDomainObjEnterMonitor(vm); if (qemuMonitorStopCPUs(priv->mon) < 0) { qemuDomainObjExitMonitor(vm); diff --git a/tools/virsh.c b/tools/virsh.c index 65eaa3b..fcbd4e6 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1431,6 +1431,7 @@ static const vshCmdInfo info_dump[] = { };
static const vshCmdOptDef opts_dump[] = { + {"live", VSH_OT_BOOL, 0, gettext_noop("perform a live core dump if supported")}, {"crash", VSH_OT_BOOL, 0, gettext_noop("crash the domain after core dump")}, {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")}, {"file", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("where to dump the core")}, @@ -1455,6 +1456,8 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return FALSE;
+ if (vshCommandOptBool (cmd, "live")) + flags |= VIR_DUMP_LIVE; if (vshCommandOptBool (cmd, "crash")) flags |= VIR_DUMP_CRASH;
Looks fine to me too, depends on previous being applied, 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/
On Mon, Nov 30, 2009 at 10:11:11AM +0100, Paolo Bonzini wrote:
This is trivial for QEMU since you just have to not stop the vm before starting the dump.
In Xen it is buggy, so I chose to not support it.
Well some versions of Xen are buggy, but not neccessarily all of them. IMHO we should support it in libvirt because it is a trivial change to make. This at least lets users try the functionality and see if there are bugs in their version of Xen eg, our code already does return xend_op(domain->conn, domain->name, "op", "dump", "file", filename, "live", "0", "crash", "0", NULL); so it is easy to set "live", "1" based on flags Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 11/30/2009 03:29 PM, Daniel P. Berrange wrote:
On Mon, Nov 30, 2009 at 10:11:11AM +0100, Paolo Bonzini wrote:
This is trivial for QEMU since you just have to not stop the vm before starting the dump.
In Xen it is buggy, so I chose to not support it.
Well some versions of Xen are buggy, but not neccessarily all of them. IMHO we should support it in libvirt because it is a trivial change to make. This at least lets users try the functionality and see if there are bugs in their version of Xen
eg, our code already does
return xend_op(domain->conn, domain->name, "op", "dump", "file", filename, "live", "0", "crash", "0", NULL);
so it is easy to set "live", "1" based on flags
That's fine by me, but I would appreciate a second opinion (it can also be done as a follow up, anyway). Paolo
In Xen it is buggy, so I chose to not support it.
Well some versions of Xen are buggy, but not neccessarily all of them. IMHO we should support it in libvirt because it is a trivial change to make. This at least lets users try the functionality and see if there are bugs in their version of Xen
eg, our code already does
return xend_op(domain->conn, domain->name, "op", "dump", "file", filename, "live", "0", "crash", "0", NULL);
so it is easy to set "live", "1" based on flags
Well, the live dump implementation in xen is just bogus even upstream but I agree we should support it in xen driver because xen claims to support it and it's not our problem that their code for that is buggy. Furthermore they could fix it in the future and then it will just work. Jirka
This is trivial for QEMU since you just have to not stop the vm before starting the dump. And for Xen, you just pass the flag down to xend. * include/libvirt/libvirt.h.in (virDomainCoreDumpFlags): Add VIR_DUMP_LIVE. * src/qemu/qemu_driver.c (qemudDomainCoreDump): Support live dumping. * src/xen/xend_internal.c (xenDaemonDomainCoreDump): Support live dumping. * tools/virsh.c (opts_dump): Add --live. (cmdDump): Map it to VIR_DUMP_LIVE. --- Since there was consensus, here is the patch with the Xen driver support. Applies on top of the --crash patch. include/libvirt/libvirt.h.in | 1 + src/qemu/qemu_driver.c | 7 +++---- src/xen/xend_internal.c | 3 ++- tools/virsh.c | 3 +++ 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c04b552..b4a7ef1 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -337,6 +337,7 @@ typedef virDomainInterfaceStatsStruct *virDomainInterfaceStatsPtr; /* Domain core dump flags. */ typedef enum { VIR_DUMP_CRASH = (1 << 0), /* crash after dump */ + VIR_DUMP_LIVE = (1 << 1), /* live dump */ } virDomainCoreDumpFlags; /* Domain migration flags. */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8e80144..7de3c45 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3597,15 +3597,14 @@ static int qemudDomainCoreDump(virDomainPtr dom, driver->securityDriver->domainSetSavedStateLabel(dom->conn, vm, path) == -1) goto endjob; - /* Migrate will always stop the VM, so once we support live dumping - the resume condition will stay the same, independent of whether - the stop command is issued. */ + /* Migrate will always stop the VM, so the resume condition is + independent of whether the stop command is issued. */ resume = (vm->state == VIR_DOMAIN_RUNNING); qemuDomainObjPrivatePtr priv = vm->privateData; /* Pause domain for non-live dump */ - if (vm->state == VIR_DOMAIN_RUNNING) { + if (!(flags & VIR_DUMP_LIVE) && vm->state == VIR_DOMAIN_RUNNING) { qemuDomainObjEnterMonitor(vm); if (qemuMonitorStopCPUs(priv->mon) < 0) { qemuDomainObjExitMonitor(vm); diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 360390d..ce7b9db 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -3244,7 +3244,8 @@ xenDaemonDomainCoreDump(virDomainPtr domain, const char *filename, } return xend_op(domain->conn, domain->name, - "op", "dump", "file", filename, "live", "0", + "op", "dump", "file", filename, + "live", (flags & VIR_DUMP_LIVE ? "1" : "0"), "crash", (flags & VIR_DUMP_CRASH ? "1" : "0"), NULL); } diff --git a/tools/virsh.c b/tools/virsh.c index 65eaa3b..fcbd4e6 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1431,6 +1431,7 @@ static const vshCmdInfo info_dump[] = { }; static const vshCmdOptDef opts_dump[] = { + {"live", VSH_OT_BOOL, 0, gettext_noop("perform a live core dump if supported")}, {"crash", VSH_OT_BOOL, 0, gettext_noop("crash the domain after core dump")}, {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")}, {"file", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("where to dump the core")}, @@ -1455,6 +1456,8 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return FALSE; + if (vshCommandOptBool (cmd, "live")) + flags |= VIR_DUMP_LIVE; if (vshCommandOptBool (cmd, "crash")) flags |= VIR_DUMP_CRASH; -- 1.6.5.2
On Tue, Dec 01, 2009 at 10:19:55AM +0100, Paolo Bonzini wrote:
This is trivial for QEMU since you just have to not stop the vm before starting the dump. And for Xen, you just pass the flag down to xend.
* include/libvirt/libvirt.h.in (virDomainCoreDumpFlags): Add VIR_DUMP_LIVE. * src/qemu/qemu_driver.c (qemudDomainCoreDump): Support live dumping. * src/xen/xend_internal.c (xenDaemonDomainCoreDump): Support live dumping. * tools/virsh.c (opts_dump): Add --live. (cmdDump): Map it to VIR_DUMP_LIVE. --- Since there was consensus, here is the patch with the Xen driver support. Applies on top of the --crash patch.
ACK, looks good now Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
http://permalink.gmane.org/gmane.comp.emulators.libvirt/18779 [libvirt] [PATCH 1/2] add --crash support to "virsh dump" http://permalink.gmane.org/gmane.comp.emulators.libvirt/18811 [libvirt] [PATCH 2/2 v2] add --live support to "virsh dump" Thanks! Paolo
On Tue, Dec 08, 2009 at 04:00:14PM +0100, Paolo Bonzini wrote:
http://permalink.gmane.org/gmane.comp.emulators.libvirt/18779 [libvirt] [PATCH 1/2] add --crash support to "virsh dump"
http://permalink.gmane.org/gmane.comp.emulators.libvirt/18811 [libvirt] [PATCH 2/2 v2] add --live support to "virsh dump"
Okay, I have pushed the 2 patches, I got a merge error in the cleanup section of the QEmu dump entry point, which I manually applied hope it's fine, I also had to cleanup some TAB used for indentation (hint use "make syntax-check" to catch those), thanks ! 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/
If there are no references remaining to the object, vm is set to NULL and vm->persistent cannot be accessed. Fixed by this trivial patch. * src/qemu/qemu_driver.c (qemudDomainCoreDump): Avoid possible NULL pointer dereference on --crash dump. --- > Okay, I have pushed the 2 patches, I got a merge error in the > cleanup section of the QEmu dump entry point, which I manually > applied hope it's fine, I also had to cleanup some TAB used for > indentation (hint use "make syntax-check" to catch those), Here is a little adjustment. Thanks! src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3946c27..faeb838 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3914,7 +3914,7 @@ endjob: if (qemuDomainObjEndJob(vm) == 0) vm = NULL; - if ((ret == 0) && (flags & VIR_DUMP_CRASH) && !vm->persistent) { + else if ((ret == 0) && (flags & VIR_DUMP_CRASH) && !vm->persistent) { virDomainRemoveInactive(&driver->domains, vm); vm = NULL; -- 1.6.5.2
On Mon, Dec 14, 2009 at 12:22:40PM +0100, Paolo Bonzini wrote:
If there are no references remaining to the object, vm is set to NULL and vm->persistent cannot be accessed. Fixed by this trivial patch.
* src/qemu/qemu_driver.c (qemudDomainCoreDump): Avoid possible NULL pointer dereference on --crash dump. --- > Okay, I have pushed the 2 patches, I got a merge error in the > cleanup section of the QEmu dump entry point, which I manually > applied hope it's fine, I also had to cleanup some TAB used for > indentation (hint use "make syntax-check" to catch those),
Here is a little adjustment. Thanks!
src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3946c27..faeb838 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3914,7 +3914,7 @@ endjob:
if (qemuDomainObjEndJob(vm) == 0) vm = NULL; - if ((ret == 0) && (flags & VIR_DUMP_CRASH) && !vm->persistent) { + else if ((ret == 0) && (flags & VIR_DUMP_CRASH) && !vm->persistent) { virDomainRemoveInactive(&driver->domains, vm); vm = NULL;
Oops I didn't realized that vm could be NULL there when fixing the merge conflict ! thanks for catching this, pushed ! 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/
On Mon, Nov 30, 2009 at 10:11:10AM +0100, Paolo Bonzini wrote:
This patch adds the --crash option (already present in "xm dump-core") to "virsh dump". virDomainCoreDump already has a flags argument, so the API/ABI is untouched.
* include/libvirt/libvirt.h.in (virDomainCoreDumpFlags): New. * src/test/test_driver.c (testDomainCoreDump): Do not crash after dump unless VIR_DUMP_CRASH is given. * src/qemu/qemu_driver.c (qemudDomainCoreDump): Shutdown the domain instead of restarting it if --crash is passed. * src/xen/xend_internal.c (xenDaemonDomainCoreDump): Support --crash. * tools/virsh.c (opts_dump): Add --crash. (cmdDump): Map it to flags for virDomainCoreDump and pass them. --- include/libvirt/libvirt.h.in | 5 +++++ src/qemu/qemu_driver.c | 17 ++++++++++++++++- src/test/test_driver.c | 19 ++++++++++--------- src/xen/xend_internal.c | 6 ++++-- tools/virsh.c | 7 ++++++- 5 files changed, 41 insertions(+), 13 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5bc7694..c04b552 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -334,6 +334,11 @@ struct _virDomainInterfaceStats { typedef virDomainInterfaceStatsStruct *virDomainInterfaceStatsPtr;
+/* Domain core dump flags. */ +typedef enum { + VIR_DUMP_CRASH = (1 << 0), /* crash after dump */ +} virDomainCoreDumpFlags; + /* Domain migration flags. */ typedef enum { VIR_MIGRATE_LIVE = (1 << 0), /* live migration */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 92d4629..8e80144 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3543,6 +3543,7 @@ static int qemudDomainCoreDump(virDomainPtr dom, virDomainObjPtr vm; int resume = 0, paused = 0; int ret = -1, fd = -1; + virDomainEventPtr event = NULL; const char *args[] = { "cat", NULL, @@ -3633,10 +3634,17 @@ static int qemudDomainCoreDump(virDomainPtr dom, goto endjob;
endjob: + if ((ret == 0) && (flags & VIR_DUMP_CRASH)) { + qemudShutdownVMDaemon(dom->conn, driver, vm); + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_CRASHED); + } + /* Since the monitor is always attached to a pty for libvirt, it will support synchronous operations so we always get here after the migration is complete. */ - if (resume && paused) { + else if (resume && paused) { qemuDomainObjEnterMonitor(vm); if (qemuMonitorStartCPUs(priv->mon, dom->conn) < 0) { if (virGetLastError() == NULL) @@ -3647,12 +3655,19 @@ endjob: }
qemuDomainObjEndJob(vm); + if ((ret == 0) && (flags & VIR_DUMP_CRASH) && !vm->persistent) { + virDomainRemoveInactive(&driver->domains, + vm); + vm = NULL; + }
cleanup: if (ret != 0) unlink(path); if (vm) virDomainObjUnlock(vm); + if (event) + qemuDomainEventQueue(driver, event); return ret; }
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 35f7571..7db9a4c 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1911,15 +1911,16 @@ static int testDomainCoreDump(virDomainPtr domain, goto cleanup; }
- testDomainShutdownState(domain, privdom); - event = virDomainEventNewFromObj(privdom, - VIR_DOMAIN_EVENT_STOPPED, - VIR_DOMAIN_EVENT_STOPPED_CRASHED); - - if (!privdom->persistent) { - virDomainRemoveInactive(&privconn->domains, - privdom); - privdom = NULL; + if (flags & VIR_DUMP_CRASH) { + testDomainShutdownState(domain, privdom); + event = virDomainEventNewFromObj(privdom, + VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_CRASHED); + if (!privdom->persistent) { + virDomainRemoveInactive(&privconn->domains, + privdom); + privdom = NULL; + } }
ret = 0; diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 66d2e7f..360390d 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -3243,8 +3243,10 @@ xenDaemonDomainCoreDump(virDomainPtr domain, const char *filename, return(-1); }
- return xend_op(domain->conn, domain->name, "op", "dump", "file", filename, - "live", "0", "crash", "0", NULL); + return xend_op(domain->conn, domain->name, + "op", "dump", "file", filename, "live", "0", + "crash", (flags & VIR_DUMP_CRASH ? "1" : "0"), + NULL); }
/** diff --git a/tools/virsh.c b/tools/virsh.c index 9faac35..65eaa3b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1431,6 +1431,7 @@ static const vshCmdInfo info_dump[] = { };
static const vshCmdOptDef opts_dump[] = { + {"crash", VSH_OT_BOOL, 0, gettext_noop("crash the domain after core dump")}, {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")}, {"file", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("where to dump the core")}, {NULL, 0, 0, NULL} @@ -1443,6 +1444,7 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) char *name; char *to; int ret = TRUE; + int flags = 0;
if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -1453,7 +1455,10 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return FALSE;
- if (virDomainCoreDump(dom, to, 0) == 0) { + if (vshCommandOptBool (cmd, "crash")) + flags |= VIR_DUMP_CRASH; + + if (virDomainCoreDump(dom, to, flags) == 0) { vshPrint(ctl, _("Domain %s dumped to %s\n"), name, to); } else { vshError(ctl, _("Failed to core dump domain %s to %s"), name, to);
Looks fine to me, ACK, nice addition ! 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/
On Mon, Nov 30, 2009 at 10:11:10AM +0100, Paolo Bonzini wrote:
This patch adds the --crash option (already present in "xm dump-core") to "virsh dump". virDomainCoreDump already has a flags argument, so the API/ABI is untouched.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 92d4629..8e80144 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3543,6 +3543,7 @@ static int qemudDomainCoreDump(virDomainPtr dom, virDomainObjPtr vm; int resume = 0, paused = 0; int ret = -1, fd = -1; + virDomainEventPtr event = NULL; const char *args[] = { "cat", NULL, @@ -3633,10 +3634,17 @@ static int qemudDomainCoreDump(virDomainPtr dom, goto endjob;
endjob: + if ((ret == 0) && (flags & VIR_DUMP_CRASH)) { + qemudShutdownVMDaemon(dom->conn, driver, vm); + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_CRASHED); + } +
Shouldn't we be setting 'resume=0' here otherwise....
/* Since the monitor is always attached to a pty for libvirt, it will support synchronous operations so we always get here after the migration is complete. */ - if (resume && paused) { + else if (resume && paused) { qemuDomainObjEnterMonitor(vm); if (qemuMonitorStartCPUs(priv->mon, dom->conn) < 0) { if (virGetLastError() == NULL)
....this will try to resume CPUs on a guest we just shutdown
@@ -3647,12 +3655,19 @@ endjob: }
qemuDomainObjEndJob(vm); + if ((ret == 0) && (flags & VIR_DUMP_CRASH) && !vm->persistent) { + virDomainRemoveInactive(&driver->domains, + vm); + vm = NULL; + }
cleanup: if (ret != 0) unlink(path); if (vm) virDomainObjUnlock(vm); + if (event) + qemuDomainEventQueue(driver, event); return ret; }
Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 11/30/2009 03:27 PM, Daniel P. Berrange wrote:
Shouldn't we be setting 'resume=0' here otherwise....
/* Since the monitor is always attached to a pty for libvirt, it will support synchronous operations so we always get here after the migration is complete. */ - if (resume&& paused) { + else if (resume&& paused) { qemuDomainObjEnterMonitor(vm); if (qemuMonitorStartCPUs(priv->mon, dom->conn)< 0) { if (virGetLastError() == NULL)
....this will try to resume CPUs on a guest we just shutdown
That's why I changed "if" to "else if". Paolo
On Mon, Nov 30, 2009 at 03:44:13PM +0100, Paolo Bonzini wrote:
On 11/30/2009 03:27 PM, Daniel P. Berrange wrote:
Shouldn't we be setting 'resume=0' here otherwise....
/* Since the monitor is always attached to a pty for libvirt, it will support synchronous operations so we always get here
after
the migration is complete. */ - if (resume&& paused) { + else if (resume&& paused) { qemuDomainObjEnterMonitor(vm); if (qemuMonitorStartCPUs(priv->mon, dom->conn)< 0) { if (virGetLastError() == NULL)
....this will try to resume CPUs on a guest we just shutdown
That's why I changed "if" to "else if".
Ah, I didn't notice that was part of the same control flow Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Jiri Denemark -
Paolo Bonzini