[libvirt] [PATCH v4 0/7] Support of auto-dump on watchdog event in libvirtd

This patch series adds a new watchdog action `dump' which lets libvirtd can do auto-dump when receiving a watchdog event from qemu guest. In order to make the function work, there must be a watchdog device added to guest, and guest must have a watchdog daemon running, for example, /etc/init.d/watchdog start or auto-started on boot. Changes: v4: - getCompressionType returns type of enum qemud_save_formats rather than int - use virThread api in thread pool - fix an error that qemuDomainObjBeginJobWithDriver() get lost in qemuDomainCoreDump() v3: - let default auto-dump dir be /var/lib/libvirt/qemu/dump Hu Tao (7): bug: local var referenced in another thread Add a threadpool implementation Fall back to QEMUD_SAVE_FORMAT_RAW if compression method fails. Add a new function doCoreDump Add a watchdog action `dump' Using threadpool API to manage qemud worker Add me to AUTHORS to make `make syntax-check' happy AUTHORS | 1 + cfg.mk | 3 +- daemon/libvirtd.c | 168 +++++------------------------ daemon/libvirtd.h | 4 + po/af.po | 14 ++-- po/am.po | 14 ++-- po/ar.po | 14 ++-- po/as.po | 14 ++-- po/be.po | 14 ++-- po/bg.po | 18 ++-- po/bn.po | 14 ++-- po/bn_IN.po | 18 ++-- po/bs.po | 14 ++-- po/ca.po | 42 ++++---- po/cs.po | 14 ++-- po/cy.po | 14 ++-- po/da.po | 14 ++-- po/de.po | 22 ++-- po/el.po | 22 ++-- po/en_GB.po | 14 ++-- po/es.po | 54 +++++----- po/et.po | 14 ++-- po/eu_ES.po | 14 ++-- po/fa.po | 14 ++-- po/fi.po | 14 ++-- po/fr.po | 94 ++++++++-------- po/gl.po | 14 ++-- po/gu.po | 14 ++-- po/he.po | 14 ++-- po/hi.po | 14 ++-- po/hr.po | 18 ++-- po/hu.po | 14 ++-- po/hy.po | 14 ++-- po/id.po | 14 ++-- po/is.po | 14 ++-- po/it.po | 54 +++++----- po/ja.po | 18 ++-- po/ka.po | 14 ++-- po/kn.po | 14 ++-- po/ko.po | 14 ++-- po/ku.po | 14 ++-- src/Makefile.am | 3 +- src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/qemu.conf | 5 + src/qemu/qemu_conf.c | 16 +++- src/qemu/qemu_conf.h | 5 + src/qemu/qemu_driver.c | 259 ++++++++++++++++++++++++++++++++------------ src/util/threadpool.c | 175 ++++++++++++++++++++++++++++++ src/util/threadpool.h | 62 +++++++++++ src/util/threads-pthread.c | 13 ++- 51 files changed, 868 insertions(+), 586 deletions(-) create mode 100644 src/util/threadpool.c create mode 100644 src/util/threadpool.h -- 1.7.3 -- Thanks, Hu Tao

--- src/util/threads-pthread.c | 13 +++++++++++-- 1 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/util/threads-pthread.c b/src/util/threads-pthread.c index 02070ae..54a3808 100644 --- a/src/util/threads-pthread.c +++ b/src/util/threads-pthread.c @@ -26,6 +26,7 @@ # include <sys/syscall.h> #endif +#include "memory.h" /* Nothing special required for pthreads */ int virThreadInitialize(void) @@ -143,6 +144,7 @@ static void *virThreadHelper(void *data) { struct virThreadArgs *args = data; args->func(args->opaque); + VIR_FREE(args); return NULL; } @@ -151,13 +153,20 @@ int virThreadCreate(virThreadPtr thread, virThreadFunc func, void *opaque) { - struct virThreadArgs args = { func, opaque }; + struct virThreadArgs *args; pthread_attr_t attr; + + if (VIR_ALLOC(args) < 0) + return -1; + + args->func = func; + args->opaque = opaque; + pthread_attr_init(&attr); if (!joinable) pthread_attr_setdetachstate(&attr, 1); - int ret = pthread_create(&thread->thread, &attr, virThreadHelper, &args); + int ret = pthread_create(&thread->thread, &attr, virThreadHelper, args); if (ret != 0) { errno = ret; return -1; -- 1.7.3 -- Thanks, Hu Tao

When dumping a domain, it's reasonable to save dump-file in raw format if dump format is misconfigured or the corresponding compress program is not available rather then fail dumping. --- src/qemu/qemu_driver.c | 41 +++++++++++++++++++++++++---------------- 1 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4877692..e6ba1a7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6057,16 +6057,10 @@ cleanup: return ret; } -static int qemudDomainCoreDump(virDomainPtr dom, - const char *path, - int flags ATTRIBUTE_UNUSED) { - struct qemud_driver *driver = dom->conn->privateData; - virDomainObjPtr vm; - int resume = 0, paused = 0; - int ret = -1, fd = -1; - virDomainEventPtr event = NULL; - int compress; - qemuDomainObjPrivatePtr priv; +static enum qemud_save_formats getCompressionType(struct qemud_driver *driver) +{ + enum qemud_save_formats compress; + /* * We reuse "save" flag for "dump" here. Then, we can support the same * format in "save" and "dump". @@ -6074,19 +6068,34 @@ static int qemudDomainCoreDump(virDomainPtr dom, compress = QEMUD_SAVE_FORMAT_RAW; if (driver->dumpImageFormat) { compress = qemudSaveCompressionTypeFromString(driver->dumpImageFormat); - if (compress < 0) { - qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("Invalid dump image format specified in " - "configuration file")); - return -1; + if ((signed)compress < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Invalid dump image format specified in " + "configuration file")); + return QEMUD_SAVE_FORMAT_RAW; } if (!qemudCompressProgramAvailable(compress)) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Compression program for dump image format " "in configuration file isn't available")); - return -1; + return QEMUD_SAVE_FORMAT_RAW; } } + return compress; +} + +static int qemudDomainCoreDump(virDomainPtr dom, + const char *path, + int flags ATTRIBUTE_UNUSED) { + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + int resume = 0, paused = 0; + int ret = -1, fd = -1; + virDomainEventPtr event = NULL; + enum qemud_save_formats compress; + qemuDomainObjPrivatePtr priv; + + compress = getCompressionType(driver); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); -- 1.7.3 -- Thanks, Hu Tao

This patch prepares for the next patch. --- src/qemu/qemu_driver.c | 129 ++++++++++++++++++++++++++--------------------- 1 files changed, 71 insertions(+), 58 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e6ba1a7..cc6891e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6057,6 +6057,75 @@ cleanup: return ret; } +static int doCoreDump(struct qemud_driver *driver, virDomainObjPtr vm, const char *path, enum qemud_save_formats compress) +{ + int fd = -1; + int ret = -1; + qemuDomainObjPrivatePtr priv; + + priv = vm->privateData; + + /* Create an empty file with appropriate ownership. */ + if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("failed to create '%s'"), path); + goto cleanup; + } + + if (VIR_CLOSE(fd) < 0) { + virReportSystemError(errno, + _("unable to save file %s"), + path); + goto cleanup; + } + + if (driver->securityDriver && + driver->securityDriver->domainSetSavedStateLabel && + driver->securityDriver->domainSetSavedStateLabel(driver->securityDriver, + vm, path) == -1) + goto cleanup; + + qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (compress == QEMUD_SAVE_FORMAT_RAW) { + const char *args[] = { + "cat", + NULL, + }; + ret = qemuMonitorMigrateToFile(priv->mon, + QEMU_MONITOR_MIGRATE_BACKGROUND, + args, path, 0); + } else { + const char *prog = qemudSaveCompressionTypeToString(compress); + const char *args[] = { + prog, + "-c", + NULL, + }; + ret = qemuMonitorMigrateToFile(priv->mon, + QEMU_MONITOR_MIGRATE_BACKGROUND, + args, path, 0); + } + qemuDomainObjExitMonitorWithDriver(driver, vm); + if (ret < 0) + goto cleanup; + + ret = qemuDomainWaitForMigrationComplete(driver, vm); + + if (ret < 0) + goto cleanup; + + if (driver->securityDriver && + driver->securityDriver->domainRestoreSavedStateLabel && + driver->securityDriver->domainRestoreSavedStateLabel(driver->securityDriver, + vm, path) == -1) + goto cleanup; + +cleanup: + if (ret != 0) + unlink(path); + return ret; +} + static enum qemud_save_formats getCompressionType(struct qemud_driver *driver) { enum qemud_save_formats compress; @@ -6090,13 +6159,10 @@ static int qemudDomainCoreDump(virDomainPtr dom, struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; int resume = 0, paused = 0; - int ret = -1, fd = -1; + int ret = -1; virDomainEventPtr event = NULL; - enum qemud_save_formats compress; qemuDomainObjPrivatePtr priv; - compress = getCompressionType(driver); - qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -6118,26 +6184,6 @@ static int qemudDomainCoreDump(virDomainPtr dom, goto endjob; } - /* Create an empty file with appropriate ownership. */ - if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - _("failed to create '%s'"), path); - goto endjob; - } - - if (VIR_CLOSE(fd) < 0) { - virReportSystemError(errno, - _("unable to save file %s"), - path); - goto endjob; - } - - if (driver->securityDriver && - driver->securityDriver->domainSetSavedStateLabel && - driver->securityDriver->domainSetSavedStateLabel(driver->securityDriver, - vm, path) == -1) - goto endjob; - /* 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); @@ -6161,43 +6207,12 @@ static int qemudDomainCoreDump(virDomainPtr dom, } } - qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (compress == QEMUD_SAVE_FORMAT_RAW) { - const char *args[] = { - "cat", - NULL, - }; - ret = qemuMonitorMigrateToFile(priv->mon, - QEMU_MONITOR_MIGRATE_BACKGROUND, - args, path, 0); - } else { - const char *prog = qemudSaveCompressionTypeToString(compress); - const char *args[] = { - prog, - "-c", - NULL, - }; - ret = qemuMonitorMigrateToFile(priv->mon, - QEMU_MONITOR_MIGRATE_BACKGROUND, - args, path, 0); - } - qemuDomainObjExitMonitorWithDriver(driver, vm); - if (ret < 0) - goto endjob; - - ret = qemuDomainWaitForMigrationComplete(driver, vm); - + ret = doCoreDump(driver, vm, path, getCompressionType(driver)); if (ret < 0) goto endjob; paused = 1; - if (driver->securityDriver && - driver->securityDriver->domainRestoreSavedStateLabel && - driver->securityDriver->domainRestoreSavedStateLabel(driver->securityDriver, - vm, path) == -1) - goto endjob; - endjob: if ((ret == 0) && (flags & VIR_DUMP_CRASH)) { qemudShutdownVMDaemon(driver, vm, 0); @@ -6230,8 +6245,6 @@ endjob: } cleanup: - if (ret != 0) - unlink(path); if (vm) virDomainObjUnlock(vm); if (event) -- 1.7.3 -- Thanks, Hu Tao

`dump' watchdog action lets libvirtd to dump the guest when receives a watchdog event (which probably means a guest crash) Currently only qemu is supported. --- cfg.mk | 3 +- src/Makefile.am | 2 +- src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/qemu.conf | 5 ++ src/qemu/qemu_conf.c | 16 +++++++- src/qemu/qemu_conf.h | 5 ++ src/qemu/qemu_driver.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/threadpool.c | 3 + 9 files changed, 132 insertions(+), 3 deletions(-) diff --git a/cfg.mk b/cfg.mk index 5576ecb..2eb0f77 100644 --- a/cfg.mk +++ b/cfg.mk @@ -129,7 +129,8 @@ useless_free_options = \ --name=virStorageVolDefFree \ --name=xmlFree \ --name=xmlXPathFreeContext \ - --name=xmlXPathFreeObject + --name=xmlXPathFreeObject \ + --name=virWorkerPoolFree # The following template was generated by this command: # make ID && aid free|grep '^vi'|sed 's/ .*//;s/^/# /' diff --git a/src/Makefile.am b/src/Makefile.am index 5febd76..83ccd7c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -646,7 +646,7 @@ endif libvirt_driver_qemu_la_CFLAGS = $(NUMACTL_CFLAGS) \ -I@top_srcdir@/src/conf $(AM_CFLAGS) libvirt_driver_qemu_la_LDFLAGS = $(AM_LDFLAGS) -libvirt_driver_qemu_la_LIBADD = $(NUMACTL_LIBS) +libvirt_driver_qemu_la_LIBADD = $(NUMACTL_LIBS) ../src/libvirt_util.la if WITH_DRIVER_MODULES libvirt_driver_qemu_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_qemu_la_LDFLAGS += -module -avoid-version diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3f14cee..a6cb444 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -245,6 +245,7 @@ VIR_ENUM_IMPL(virDomainWatchdogAction, VIR_DOMAIN_WATCHDOG_ACTION_LAST, "shutdown", "poweroff", "pause", + "dump", "none") VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 899b19f..7f50b79 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -462,6 +462,7 @@ enum virDomainWatchdogAction { VIR_DOMAIN_WATCHDOG_ACTION_SHUTDOWN, VIR_DOMAIN_WATCHDOG_ACTION_POWEROFF, VIR_DOMAIN_WATCHDOG_ACTION_PAUSE, + VIR_DOMAIN_WATCHDOG_ACTION_DUMP, VIR_DOMAIN_WATCHDOG_ACTION_NONE, VIR_DOMAIN_WATCHDOG_ACTION_LAST diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index f4f965e..ba41f80 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -191,6 +191,11 @@ # save_image_format = "raw" # dump_image_format = "raw" +# When a domain is configured to be auto-dumped when libvirtd receives a +# watchdog event from qemu guest, libvirtd will save dump files in directory +# specified by auto_dump_path. Default value is /var/lib/libvirt/qemu/dump +# +# auto_dump_path = "/var/lib/libvirt/qemu/dump" # If provided by the host and a hugetlbfs mount point is configured, # a guest may request huge page backing. When this mount point is diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index b0343c6..6296378 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -386,6 +386,17 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, } } + p = virConfGetValue (conf, "auto_dump_path"); + CHECK_TYPE ("auto_dump_path", VIR_CONF_STRING); + if (p && p->str) { + VIR_FREE(driver->autoDumpPath); + if (!(driver->autoDumpPath = strdup(p->str))) { + virReportOOMError(); + virConfFree(conf); + return -1; + } + } + p = virConfGetValue (conf, "hugetlbfs_mount"); CHECK_TYPE ("hugetlbfs_mount", VIR_CONF_STRING); if (p && p->str) { @@ -5373,7 +5384,10 @@ int qemudBuildCommandLine(virConnectPtr conn, } ADD_ARG(optstr); - const char *action = virDomainWatchdogActionTypeToString(watchdog->action); + int act = watchdog->action; + if (act == VIR_DOMAIN_WATCHDOG_ACTION_DUMP) + act = VIR_DOMAIN_WATCHDOG_ACTION_PAUSE; + const char *action = virDomainWatchdogActionTypeToString(act); if (!action) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("invalid watchdog action")); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index aba64d6..0956e7b 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -41,6 +41,7 @@ # include "driver.h" # include "bitmap.h" # include "macvtap.h" +# include "threadpool.h" # define qemudDebug(fmt, ...) do {} while(0) @@ -107,6 +108,8 @@ enum qemud_cmd_flags { struct qemud_driver { virMutex lock; + virWorkerPoolPtr workerPool; + int privileged; uid_t user; @@ -174,6 +177,8 @@ struct qemud_driver { char *saveImageFormat; char *dumpImageFormat; + char *autoDumpPath; + pciDeviceList *activePciHostdevs; virBitmapPtr reservedVNCPorts; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cc6891e..d99733c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -85,6 +85,7 @@ #include "files.h" #include "fdstream.h" #include "configmake.h" +#include "threadpool.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -137,6 +138,16 @@ struct _qemuDomainObjPrivate { int persistentAddrs; }; +struct watchdogEvent +{ + virDomainObjPtr vm; + int action; +}; + +static enum qemud_save_formats getCompressionType(struct qemud_driver *driver); +static int doCoreDump(struct qemud_driver *driver, virDomainObjPtr vm, const char *path, enum qemud_save_formats compress); +static void processWatchdogEvent(void *data); + static int qemudShutdown(void); static void qemuDriverLock(struct qemud_driver *driver) @@ -1204,6 +1215,17 @@ qemuHandleDomainWatchdog(qemuMonitorPtr mon ATTRIBUTE_UNUSED, if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) VIR_WARN("Unable to save status on vm %s after IO error", vm->def->name); } + + if (vm->def->watchdog->action == VIR_DOMAIN_WATCHDOG_ACTION_DUMP) { + struct watchdogEvent *wdEvent; + if (VIR_ALLOC(wdEvent) == 0) { + wdEvent->action = VIR_DOMAIN_WATCHDOG_ACTION_DUMP; + wdEvent->vm = vm; + virWorkerPoolSendJob(driver->workerPool, wdEvent); + } else + virReportOOMError(); + } + virDomainObjUnlock(vm); if (watchdogEvent || lifecycleEvent) { @@ -1786,6 +1808,9 @@ qemudStartup(int privileged) { if (virAsprintf(&qemu_driver->snapshotDir, "%s/lib/libvirt/qemu/snapshot", LOCALSTATEDIR) == -1) goto out_of_memory; + if (virAsprintf(&qemu_driver->autoDumpPath, + "%s/lib/libvirt/qemu/dump", LOCALSTATEDIR) == -1) + goto out_of_memory; } else { uid_t uid = geteuid(); char *userdir = virGetUserDirectory(uid); @@ -1814,6 +1839,8 @@ qemudStartup(int privileged) { goto out_of_memory; if (virAsprintf(&qemu_driver->snapshotDir, "%s/qemu/snapshot", base) == -1) goto out_of_memory; + if (virAsprintf(&qemu_driver->autoDumpPath, "%s/qemu/dump", base) == -1) + goto out_of_memory; } if (virFileMakePath(qemu_driver->stateDir) != 0) { @@ -1846,6 +1873,12 @@ qemudStartup(int privileged) { qemu_driver->snapshotDir, virStrerror(errno, ebuf, sizeof ebuf)); goto error; } + if (virFileMakePath(qemu_driver->autoDumpPath) != 0) { + char ebuf[1024]; + VIR_ERROR(_("Failed to create dump dir '%s': %s"), + qemu_driver->autoDumpPath, virStrerror(errno, ebuf, sizeof ebuf)); + goto error; + } /* Configuration paths are either ~/.libvirt/qemu/... (session) or * /etc/libvirt/qemu/... (system). @@ -1971,6 +2004,10 @@ qemudStartup(int privileged) { qemudAutostartConfigs(qemu_driver); + qemu_driver->workerPool = virWorkerPoolNew(0, 1, processWatchdogEvent); + if (!qemu_driver->workerPool) + goto error; + if (conn) virConnectClose(conn); @@ -2078,6 +2115,7 @@ qemudShutdown(void) { VIR_FREE(qemu_driver->cacheDir); VIR_FREE(qemu_driver->saveDir); VIR_FREE(qemu_driver->snapshotDir); + VIR_FREE(qemu_driver->autoDumpPath); VIR_FREE(qemu_driver->vncTLSx509certdir); VIR_FREE(qemu_driver->vncListen); VIR_FREE(qemu_driver->vncPassword); @@ -2106,6 +2144,7 @@ qemudShutdown(void) { qemuDriverUnlock(qemu_driver); virMutexDestroy(&qemu_driver->lock); + virWorkerPoolFree(qemu_driver->workerPool); VIR_FREE(qemu_driver); return 0; @@ -4432,6 +4471,8 @@ retry: } } + + static virDrvOpenStatus qemudOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { @@ -6253,6 +6294,64 @@ cleanup: return ret; } +static void processWatchdogEvent(void *data) +{ + int ret; + struct watchdogEvent *wdEvent = data; + + switch (wdEvent->action) { + case VIR_DOMAIN_WATCHDOG_ACTION_DUMP: + { + char *dumpfile; + int i; + + qemuDomainObjPrivatePtr priv = wdEvent->vm->privateData; + + i = virAsprintf(&dumpfile, "%s/%s-%u", + qemu_driver->autoDumpPath, + wdEvent->vm->def->name, + (unsigned int)time(NULL)); + + qemuDriverLock(qemu_driver); + virDomainObjLock(wdEvent->vm); + + if (qemuDomainObjBeginJobWithDriver(qemu_driver, wdEvent->vm) < 0) + break; + + if (!virDomainObjIsActive(wdEvent->vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + break; + } + + ret = doCoreDump(qemu_driver, + wdEvent->vm, + dumpfile, + getCompressionType(qemu_driver)); + if (ret < 0) + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("Dump failed")); + + qemuDomainObjEnterMonitorWithDriver(qemu_driver, wdEvent->vm); + ret = qemuMonitorStartCPUs(priv->mon, NULL); + qemuDomainObjExitMonitorWithDriver(qemu_driver, wdEvent->vm); + + if (ret < 0) + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("Resuming after dump failed")); + + qemuDomainObjEndJob(wdEvent->vm); + + virDomainObjUnlock(wdEvent->vm); + qemuDriverUnlock(qemu_driver); + + VIR_FREE(dumpfile); + } + break; + } + + VIR_FREE(wdEvent); +} static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) { diff --git a/src/util/threadpool.c b/src/util/threadpool.c index 79f9fbb..b5034b3 100644 --- a/src/util/threadpool.c +++ b/src/util/threadpool.c @@ -118,6 +118,9 @@ error: void virWorkerPoolFree(virWorkerPoolPtr pool) { + if (!pool) + return; + virMutexLock(&pool->mutex); pool->quit = true; if (pool->nWorker > 0) { -- 1.7.3 -- Thanks, Hu Tao

--- daemon/libvirtd.c | 168 ++++++++--------------------------------------------- daemon/libvirtd.h | 4 + 2 files changed, 29 insertions(+), 143 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index caf51bf..f4987a3 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -67,6 +67,7 @@ #include "stream.h" #include "hooks.h" #include "virtaudit.h" +#include "threadpool.h" #ifdef HAVE_AVAHI # include "mdns.h" #endif @@ -248,7 +249,6 @@ static void sig_handler(int sig, siginfo_t * siginfo, static void qemudDispatchClientEvent(int watch, int fd, int events, void *opaque); static void qemudDispatchServerEvent(int watch, int fd, int events, void *opaque); -static int qemudStartWorker(struct qemud_server *server, struct qemud_worker *worker); void qemudClientMessageQueuePush(struct qemud_client_message **queue, @@ -1383,6 +1383,7 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket client->auth = sock->auth; client->addr = addr; client->addrstr = addrstr; + client->server = server; addrstr = NULL; for (i = 0 ; i < VIR_DOMAIN_EVENT_ID_LAST ; i++) { @@ -1458,19 +1459,6 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket server->clients[server->nclients++] = client; - if (server->nclients > server->nactiveworkers && - server->nactiveworkers < server->nworkers) { - for (i = 0 ; i < server->nworkers ; i++) { - if (!server->workers[i].hasThread) { - if (qemudStartWorker(server, &server->workers[i]) < 0) - return -1; - server->nactiveworkers++; - break; - } - } - } - - return 0; error: @@ -1534,100 +1522,27 @@ void qemudDispatchClientFailure(struct qemud_client *client) { VIR_FREE(client->addrstr); } - -/* Caller must hold server lock */ -static struct qemud_client *qemudPendingJob(struct qemud_server *server) +static void qemudWorker(void *data) { - int i; - for (i = 0 ; i < server->nclients ; i++) { - virMutexLock(&server->clients[i]->lock); - if (server->clients[i]->dx) { - /* Delibrately don't unlock client - caller wants the lock */ - return server->clients[i]; - } - virMutexUnlock(&server->clients[i]->lock); - } - return NULL; -} + struct qemud_client *client = data; + struct qemud_client_message *msg; -static void *qemudWorker(void *data) -{ - struct qemud_worker *worker = data; - struct qemud_server *server = worker->server; + virMutexLock(&client->lock); - while (1) { - struct qemud_client *client = NULL; - struct qemud_client_message *msg; + /* Remove our message from dispatch queue while we use it */ + msg = qemudClientMessageQueueServe(&client->dx); - virMutexLock(&server->lock); - while ((client = qemudPendingJob(server)) == NULL) { - if (worker->quitRequest || - virCondWait(&server->job, &server->lock) < 0) { - virMutexUnlock(&server->lock); - return NULL; - } - } - if (worker->quitRequest) { - virMutexUnlock(&client->lock); - virMutexUnlock(&server->lock); - return NULL; - } - worker->processingCall = 1; - virMutexUnlock(&server->lock); - - /* We own a locked client now... */ - client->refs++; - - /* Remove our message from dispatch queue while we use it */ - msg = qemudClientMessageQueueServe(&client->dx); - - /* This function drops the lock during dispatch, - * and re-acquires it before returning */ - if (remoteDispatchClientRequest (server, client, msg) < 0) { - VIR_FREE(msg); - qemudDispatchClientFailure(client); - client->refs--; - virMutexUnlock(&client->lock); - continue; - } - - client->refs--; - virMutexUnlock(&client->lock); - - virMutexLock(&server->lock); - worker->processingCall = 0; - virMutexUnlock(&server->lock); - } -} - -static int qemudStartWorker(struct qemud_server *server, - struct qemud_worker *worker) { - pthread_attr_t attr; - pthread_attr_init(&attr); - /* We want to join workers, so don't detach them */ - /*pthread_attr_setdetachstate(&attr, 1);*/ - - if (worker->hasThread) - return -1; - - worker->server = server; - worker->hasThread = 1; - worker->quitRequest = 0; - worker->processingCall = 0; - - if (pthread_create(&worker->thread, - &attr, - qemudWorker, - worker) != 0) { - worker->hasThread = 0; - worker->server = NULL; - return -1; + /* This function drops the lock during dispatch, + * and re-acquires it before returning */ + if (remoteDispatchClientRequest (client->server, client, msg) < 0) { + VIR_FREE(msg); + qemudDispatchClientFailure(client); } - return 0; + client->refs--; + virMutexUnlock(&client->lock); } - /* * Read data into buffer using wire decoding (plain or TLS) * @@ -1857,8 +1772,11 @@ readmore: } /* Move completed message to the end of the dispatch queue */ - if (msg) + if (msg) { + client->refs++; qemudClientMessageQueuePush(&client->dx, msg); + virWorkerPoolSendJob(server->workerPool, client); + } client->nrequests++; /* Possibly need to create another receive buffer */ @@ -1870,9 +1788,6 @@ readmore: client->rx->bufferLength = REMOTE_MESSAGE_HEADER_XDR_LEN; qemudUpdateClientEvent(client); - - /* Tell one of the workers to get on with it... */ - virCondSignal(&server->job); } } } @@ -2311,10 +2226,10 @@ static void *qemudRunLoop(void *opaque) { return NULL; } - for (i = 0 ; i < min_workers ; i++) { - if (qemudStartWorker(server, &server->workers[i]) < 0) - goto cleanup; - server->nactiveworkers++; + server->workerPool = virWorkerPoolNew(min_workers, max_workers, qemudWorker); + if (!server->workerPool) { + virMutexUnlock(&server->lock); + return NULL; } for (;!server->quitEventThread;) { @@ -2367,43 +2282,10 @@ static void *qemudRunLoop(void *opaque) { goto reprocess; } } - - /* If number of active workers exceeds both the min_workers - * threshold and the number of clients, then kill some - * off */ - for (i = 0 ; (i < server->nworkers && - server->nactiveworkers > server->nclients && - server->nactiveworkers > min_workers) ; i++) { - - if (server->workers[i].hasThread && - !server->workers[i].processingCall) { - server->workers[i].quitRequest = 1; - - virCondBroadcast(&server->job); - virMutexUnlock(&server->lock); - pthread_join(server->workers[i].thread, NULL); - virMutexLock(&server->lock); - server->workers[i].hasThread = 0; - server->nactiveworkers--; - } - } - } - -cleanup: - for (i = 0 ; i < server->nworkers ; i++) { - if (!server->workers[i].hasThread) - continue; - - server->workers[i].quitRequest = 1; - virCondBroadcast(&server->job); - - virMutexUnlock(&server->lock); - pthread_join(server->workers[i].thread, NULL); - virMutexLock(&server->lock); - server->workers[i].hasThread = 0; } - VIR_FREE(server->workers); + virWorkerPoolFree(server->workerPool); + server->workerPool = NULL; virMutexUnlock(&server->lock); return NULL; } diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index af20e56..9fa5edb 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -49,6 +49,7 @@ # include "logging.h" # include "threads.h" # include "network.h" +# include "threadpool.h" # if WITH_DTRACE # ifndef LIBVIRTD_PROBES_H @@ -192,6 +193,8 @@ struct qemud_client { int magic; + struct qemud_server *server; + int fd; int watch; unsigned int readonly :1; @@ -283,6 +286,7 @@ struct qemud_server { int privileged; + virWorkerPoolPtr workerPool; size_t nworkers; size_t nactiveworkers; struct qemud_worker *workers; -- 1.7.3 -- Thanks, Hu Tao

--- AUTHORS | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/AUTHORS b/AUTHORS index 16c755d..ca2414c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -136,6 +136,7 @@ Patches have also been contributed by: Osier Yang <jyang@redhat.com> Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Wen Congyang <wency@cn.fujitsu.com> + Hu Tao <hutao@cn.fujitsu.com> [....send patches to get your name here....] -- 1.7.3 -- Thanks, Hu Tao

--- src/Makefile.am | 1 + src/util/threadpool.c | 172 +++++++++++++++++++++++++++++++++++++++++++++++++ src/util/threadpool.h | 62 ++++++++++++++++++ 3 files changed, 235 insertions(+), 0 deletions(-) create mode 100644 src/util/threadpool.c create mode 100644 src/util/threadpool.h diff --git a/src/Makefile.am b/src/Makefile.am index a9a1986..5febd76 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -76,6 +76,7 @@ UTIL_SOURCES = \ util/uuid.c util/uuid.h \ util/util.c util/util.h \ util/xml.c util/xml.h \ + util/threadpool.c util/threadpool.h \ util/virtaudit.c util/virtaudit.h \ util/virterror.c util/virterror_internal.h diff --git a/src/util/threadpool.c b/src/util/threadpool.c new file mode 100644 index 0000000..79f9fbb --- /dev/null +++ b/src/util/threadpool.c @@ -0,0 +1,172 @@ +/* + * threadpool.c: a generic thread pool implementation + * + * Copyright (C) 2010 Hu Tao + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: + * Hu Tao <hutao@cn.fujitsu.com> + */ + +#include <config.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> + +#include "threadpool.h" +#include "memory.h" + +static void workerHandleJob(void *data) +{ + virDataPtr localData = NULL; + virWorkerPoolPtr pool = data; + + virMutexLock(&pool->mutex); + + while (1) { + while (!pool->quit && !pool->dataList) { + pool->nFreeWorker++; + virCondSignal(&pool->worker_cond); + if (virCondWait(&pool->cond, &pool->mutex) < 0) { + pool->nFreeWorker--; + goto out; + } + pool->nFreeWorker--; + + if (pool->nWorker > pool->nMaxWorker) + goto out; + } + + while ((localData = pool->dataList) != NULL) { + pool->dataList = pool->dataList->next; + localData->next = NULL; + + virMutexUnlock(&pool->mutex); + + (pool->func)(localData->data); + VIR_FREE(localData); + + virMutexLock(&pool->mutex); + } + + if (pool->quit) + break; + } + +out: + pool->nWorker--; + if (pool->nWorker == 0) + virCondSignal(&pool->quit_cond); + virMutexUnlock(&pool->mutex); +} + +virWorkerPoolPtr virWorkerPoolNew(int nWorker, int maxWorker, virWorkerFunc func) +{ + virWorkerPoolPtr pool; + virThread thread; + int i; + + if (nWorker < 0) + return NULL; + + if (nWorker > maxWorker) + return NULL; + + if (VIR_ALLOC(pool)) + return NULL; + + memset(pool, 0, sizeof(*pool)); + pool->func = func; + if (virMutexInit(&pool->mutex) < 0) + goto error; + if (virCondInit(&pool->cond) < 0) + goto error; + if (virCondInit(&pool->worker_cond) < 0) + goto error; + if (virCondInit(&pool->quit_cond) < 0) + goto error; + + for (i = 0; i < nWorker; i++) { + if (virThreadCreate(&thread, true, workerHandleJob, pool) < 0) { + pool->nWorker = i; + goto error; + } + } + + pool->nFreeWorker = 0; + pool->nWorker = nWorker; + pool->nMaxWorker = maxWorker; + + return pool; +error: + virWorkerPoolFree(pool); + return NULL; +} + +void virWorkerPoolFree(virWorkerPoolPtr pool) +{ + virMutexLock(&pool->mutex); + pool->quit = true; + if (pool->nWorker > 0) { + virCondBroadcast(&pool->cond); + virCondWait(&pool->quit_cond, &pool->mutex); + } + virMutexUnlock(&pool->mutex); + VIR_FREE(pool); +} + +int virWorkerPoolSendJob(virWorkerPoolPtr pool, void *data) +{ + virThread thread; + virDataPtr localData; + + if (VIR_ALLOC(localData)) + return -1; + + localData->data = data; + + virMutexLock(&pool->mutex); + if (pool->quit) { + virMutexUnlock(&pool->mutex); + VIR_FREE(localData); + return -1; + } + + localData->next = pool->dataList; + pool->dataList = localData; + + if (pool->nFreeWorker == 0 && pool->nWorker < pool->nMaxWorker) { + if (virThreadCreate(&thread, true, workerHandleJob, pool) == 0) + pool->nWorker++; + } + + virCondSignal(&pool->cond); + virMutexUnlock(&pool->mutex); + + return 0; +} + +int virWorkerPoolSetMaxWorker(virWorkerPoolPtr pool, int maxWorker) +{ + if (maxWorker < 0) + return -1; + + virMutexLock(&pool->mutex); + pool->nMaxWorker = maxWorker; + virMutexUnlock(&pool->mutex); + + return 0; +} diff --git a/src/util/threadpool.h b/src/util/threadpool.h new file mode 100644 index 0000000..f8039e6 --- /dev/null +++ b/src/util/threadpool.h @@ -0,0 +1,62 @@ +/* + * threadpool.h: a generic thread pool implementation + * + * Copyright (C) 2010 Hu Tao + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: + * Hu Tao <hutao@cn.fujitsu.com> + */ + +#ifndef __VIR_THREADPOOL_H__ +#define __VIR_THREADPOOL_H__ + +#include "threads.h" + +typedef void (*virWorkerFunc)(void *); + +struct _virData { + struct _virData *next; + + void *data; +}; +typedef struct _virData virData; +typedef virData *virDataPtr; + +struct _virWorkerPool { + size_t nWorker; + size_t nMaxWorker; + size_t nFreeWorker; + + bool quit; + + virWorkerFunc func; + virDataPtr dataList; + + virMutex mutex; + virCond cond; + virCond worker_cond; + virCond quit_cond; +}; +typedef struct _virWorkerPool virWorkerPool; +typedef virWorkerPool *virWorkerPoolPtr; + +virWorkerPoolPtr virWorkerPoolNew(int nWorker, int nMaxWorker, virWorkerFunc func) ATTRIBUTE_RETURN_CHECK; +void virWorkerPoolFree(virWorkerPoolPtr pool); +int virWorkerPoolSendJob(virWorkerPoolPtr pool, void *data) ATTRIBUTE_NONNULL(1); +int virWorkerPoolSetMaxWorker(virWorkerPoolPtr pool, int maxWorker) ATTRIBUTE_NONNULL(1); + +#endif -- 1.7.3 -- Thanks, Hu Tao

On 12/01/2010 06:04 PM, Hu Tao wrote:
This patch series adds a new watchdog action `dump' which lets libvirtd can do auto-dump when receiving a watchdog event from qemu guest.
In order to make the function work, there must be a watchdog device added to guest, and guest must have a watchdog daemon running, for example, /etc/init.d/watchdog start or auto-started on boot.
Changes:
v4:
- getCompressionType returns type of enum qemud_save_formats rather than int - use virThread api in thread pool - fix an error that qemuDomainObjBeginJobWithDriver() get lost in qemuDomainCoreDump()
You'll want to update to the latest libvirt.git, and rebase this series on top of that. In particular, you don't need to commit any po/*.po file changes ('git checkout po' is the easiest way to undo those changes if running 'make' caused them to be regenerated), and patches 3 and 7 have already been incorporated upstream, so they are no longer needed in your series.
Hu Tao (7): bug: local var referenced in another thread Add a threadpool implementation Fall back to QEMUD_SAVE_FORMAT_RAW if compression method fails. Add a new function doCoreDump Add a watchdog action `dump' Using threadpool API to manage qemud worker Add me to AUTHORS to make `make syntax-check' happy
AUTHORS | 1 + cfg.mk | 3 +- daemon/libvirtd.c | 168 +++++------------------------ daemon/libvirtd.h | 4 + po/af.po | 14 ++-- po/am.po | 14 ++--
Hmm; this diffstat appears to include uncommitted changes on your HEAD, since none of the 7 commits actually included po changes. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Wed, Dec 01, 2010 at 06:44:02PM -0700, Eric Blake wrote:
On 12/01/2010 06:04 PM, Hu Tao wrote:
This patch series adds a new watchdog action `dump' which lets libvirtd can do auto-dump when receiving a watchdog event from qemu guest.
In order to make the function work, there must be a watchdog device added to guest, and guest must have a watchdog daemon running, for example, /etc/init.d/watchdog start or auto-started on boot.
Changes:
v4:
- getCompressionType returns type of enum qemud_save_formats rather than int - use virThread api in thread pool - fix an error that qemuDomainObjBeginJobWithDriver() get lost in qemuDomainCoreDump()
You'll want to update to the latest libvirt.git, and rebase this series on top of that.
Oops! I forgot to do a pull&rebase again.
In particular, you don't need to commit any po/*.po file changes ('git checkout po' is the easiest way to undo those changes if running 'make' caused them to be regenerated), and patches 3 and 7 have already been incorporated upstream, so they are no longer needed in your series.
While I was sending patch 2 I saw those POes and realized stg messed my tree up and redid patch 2 before sending it.(But patch 0 was already sent so those POes were listed there)
Hu Tao (7): bug: local var referenced in another thread Add a threadpool implementation Fall back to QEMUD_SAVE_FORMAT_RAW if compression method fails. Add a new function doCoreDump Add a watchdog action `dump' Using threadpool API to manage qemud worker Add me to AUTHORS to make `make syntax-check' happy
AUTHORS | 1 + cfg.mk | 3 +- daemon/libvirtd.c | 168 +++++------------------------ daemon/libvirtd.h | 4 + po/af.po | 14 ++-- po/am.po | 14 ++--
Hmm; this diffstat appears to include uncommitted changes on your HEAD, since none of the 7 commits actually included po changes.
-- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Thu, Dec 02, 2010 at 09:04:22AM +0800, Hu Tao wrote: > This patch series adds a new watchdog action `dump' which lets libvirtd > can do auto-dump when receiving a watchdog event from qemu guest. > > In order to make the function work, there must be a watchdog device > added to guest, and guest must have a watchdog daemon running, for > example, /etc/init.d/watchdog start or auto-started on boot. > > Changes: > > v4: > > - getCompressionType returns type of enum qemud_save_formats rather > than int > - use virThread api in thread pool > - fix an error that qemuDomainObjBeginJobWithDriver() get lost in > qemuDomainCoreDump() and: - use thread pool in libvirtd (qemud worker)
participants (2)
-
Eric Blake
-
Hu Tao