[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Add domainCoreDump to libxl driver



Quoting Jim Fehlig <jfehlig novell com>:

Markus Groß wrote:
For core dumping to work correctly the following patch
for xen is needed:
http://lists.xensource.com/archives/html/xen-devel/2011-05/msg01469.html

This patch is in xen-unstable and is considered for backport to
the xen stable branches. Without this patch the mapped
memory pages of the pv guest are not unmapped after core-dump.

---
src/libxl/libxl_driver.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 87 insertions(+), 0 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index b2cc0e8..75008db 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1627,6 +1627,92 @@ libxlDomainGetState(virDomainPtr dom,
 }

 static int
+libxlDomainCoreDump(virDomainPtr dom, const char *to, int flags)
+{
+    libxlDriverPrivatePtr driver = dom->conn->privateData;
+    libxlDomainObjPrivatePtr priv;
+    virDomainObjPtr vm;
+    virDomainEventPtr event = NULL;
+    int paused = 0;
+    int ret = -1;
+
+    virCheckFlags(VIR_DUMP_LIVE | VIR_DUMP_CRASH, -1);
+
+    libxlDriverLock(driver);
+    vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+
+    if (!vm) {
+        char uuidstr[VIR_UUID_STRING_BUFLEN];
+        virUUIDFormat(dom->uuid, uuidstr);
+        libxlError(VIR_ERR_NO_DOMAIN,
+                   _("No domain with matching uuid '%s'"), uuidstr);
+        goto cleanup;
+    }
+
+    if (!virDomainObjIsActive(vm)) {
+ libxlError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running"));
+        goto cleanup;
+    }
+
+    priv = vm->privateData;
+
+    if (!(flags & VIR_DUMP_LIVE) &&
+        virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
+        if (libxl_domain_pause(&priv->ctx, dom->id) != 0) {
+            libxlError(VIR_ERR_INTERNAL_ERROR,
+                       _("Failed to suspend domain '%d' with libxenlight"),
+                       dom->id);


I think there could be a little more info in that error message, e.g.
"Before dumping core, failed to suspend ...".

+            goto cleanup;
+        }
+ virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_DUMP);
+        paused = 1;
+    }
+
+    if (libxl_domain_core_dump(&priv->ctx, dom->id, to) != 0) {
+        libxlError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to dump core of domain '%d' with libxenlight"),
+                   dom->id);
+        goto cleanup;
+    }


If core dumping fails and the domain was paused, it won't be unpaused by
jumping to cleanup.

+
+    if (flags & VIR_DUMP_CRASH) {
+        if (libxlVmReap(driver, vm, 1, VIR_DOMAIN_SHUTOFF_CRASHED) != 0) {
+            libxlError(VIR_ERR_INTERNAL_ERROR,
+                       _("Failed to destroy domain '%d'"), dom->id);
+            goto cleanup;
+        }
+
+        event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
+                                         VIR_DOMAIN_EVENT_STOPPED_CRASHED);
+
+    } else if (paused) {
+        if (libxl_domain_unpause(&priv->ctx, dom->id) != 0) {
+            libxlError(VIR_ERR_INTERNAL_ERROR,
+                       _("Failed to resume domain '%d' with libxenlight"),
+                       dom->id);


Here too more info in the error message, e.g. "After dumping core,
failed to resume ..."

+            goto cleanup;
+        }
+        virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
+                             VIR_DOMAIN_RUNNING_UNPAUSED);
+    }
+
+    if ((flags & VIR_DUMP_CRASH) && !vm->persistent) {
+        virDomainRemoveInactive(&driver->domains, vm);
+        vm = NULL;
+    }
+
+    ret = 0;
+
+cleanup:
+    if (vm)
+        virDomainObjUnlock(vm);
+    if (event)
+        libxlDomainEventQueue(driver, event);
+    libxlDriverUnlock(driver);


Do we need to have the driver locked during the core dump?  Most of the
driver functions use this pattern

    libxlDriverLock(driver);
    vm = virDomainFindByUUID(&driver->domains, dom->uuid);
    libxlDriverUnlock(driver);

Regards,
Jim

I think we do, since the calls to libxlVmReap and virDomainRemoveInactive both use the driver as parameter and at least virDomainRemoveInactive is able to modify the contents of it. But if you can convince me otherwise we can unlock the driver directly after obtaining the vm object.

I will post a reworked version of this patch on monday.


+    return ret;
+}
+
+static int
 libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
                          unsigned int flags)
 {
@@ -2722,6 +2808,7 @@ static virDriver libxlDriver = {
     .domainSetMemoryFlags = libxlDomainSetMemoryFlags, /* 0.9.0 */
     .domainGetInfo = libxlDomainGetInfo, /* 0.9.0 */
     .domainGetState = libxlDomainGetState, /* 0.9.2 */
+    .domainCoreDump = libxlDomainCoreDump, /* 0.9.2 */
     .domainSetVcpus = libxlDomainSetVcpus, /* 0.9.0 */
     .domainSetVcpusFlags = libxlDomainSetVcpusFlags, /* 0.9.0 */
     .domainGetVcpusFlags = libxlDomainGetVcpusFlags, /* 0.9.0 */






[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]