[libvirt] [PATCH v6 0/4] 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: v6: - remove struct qemud_worker and qemud_server.job, qemud_server.nworkers, qemud_server.nactiveworkers, qemud_server.workers. v5: - qemu_driver is passed into threadpool as opaque parameter rather than visit the global qemu_driver in worker function - same situation as above of server in libvirtd.c - also list auto_dump_path in src/qemu/libvirtd_qemu.aug and src/qemu/test_libvirtd_qemu.aug - check return value of qemuDomainObjEndJob for safety v4: - updated threadpool to follow libvirt naming style, use appropriate internals APIs, and hide the struct definitions from the header (by Daniel) - fix an error that qemuDomainObjBeginJobWithDriver() get lost in qemuDomainCoreDump() - use thread pool in libvirtd (qemud worker) v3: - let default auto-dump dir be /var/lib/libvirt/qemu/dump Hu Tao (4): threadpool impl Add a new function doCoreDump Add a watchdog action `dump' Using threadpool API to manage qemud worker cfg.mk | 1 + daemon/libvirtd.c | 187 +++++--------------------------- daemon/libvirtd.h | 16 +--- src/Makefile.am | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 6 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 5 + src/qemu/qemu_conf.c | 16 +++- src/qemu/qemu_conf.h | 5 + src/qemu/qemu_driver.c | 228 ++++++++++++++++++++++++++++---------- src/qemu/test_libvirtd_qemu.aug | 2 + src/util/threadpool.c | 231 +++++++++++++++++++++++++++++++++++++++ src/util/threadpool.h | 48 ++++++++ 15 files changed, 517 insertions(+), 232 deletions(-) create mode 100644 src/util/threadpool.c create mode 100644 src/util/threadpool.h -- 1.7.3 -- Thanks, Hu Tao
* src/util/threadpool.c, src/util/threadpool.h: Thread pool implementation * src/Makefile.am: Build thread pool * src/libvirt_private.syms: Export public functions --- cfg.mk | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 6 + src/util/threadpool.c | 231 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/threadpool.h | 48 ++++++++++ 5 files changed, 287 insertions(+), 0 deletions(-) create mode 100644 src/util/threadpool.c create mode 100644 src/util/threadpool.h diff --git a/cfg.mk b/cfg.mk index 29de9d9..bda8c57 100644 --- a/cfg.mk +++ b/cfg.mk @@ -128,6 +128,7 @@ useless_free_options = \ --name=virStoragePoolObjFree \ --name=virStoragePoolSourceFree \ --name=virStorageVolDefFree \ + --name=virThreadPoolFree \ --name=xmlFree \ --name=xmlXPathFreeContext \ --name=xmlXPathFreeObject diff --git a/src/Makefile.am b/src/Makefile.am index 0923d60..196d8af 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -74,6 +74,7 @@ UTIL_SOURCES = \ util/threads.c util/threads.h \ util/threads-pthread.h \ util/threads-win32.h \ + util/threadpool.c util/threadpool.h \ util/uuid.c util/uuid.h \ util/util.c util/util.h \ util/xml.c util/xml.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e2def6c..a0a598b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -898,3 +898,9 @@ virXPathStringLimit; virXPathULong; virXPathULongHex; virXPathULongLong; + + +# threadpool.h +virThreadPoolNew; +virThreadPoolFree; +virThreadPoolSendJob; diff --git a/src/util/threadpool.c b/src/util/threadpool.c new file mode 100644 index 0000000..8217591 --- /dev/null +++ b/src/util/threadpool.c @@ -0,0 +1,231 @@ +/* + * threadpool.c: a generic thread pool implementation + * + * Copyright (C) 2010 Hu Tao + * Copyright (C) 2010 Daniel P. Berrange + * + * 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 + * + * Authors: + * Hu Tao <hutao@cn.fujitsu.com> + * Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> + +#include "threadpool.h" +#include "memory.h" +#include "threads.h" +#include "virterror_internal.h" +#include "ignore-value.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +typedef struct _virThreadPoolJob virThreadPoolJob; +typedef virThreadPoolJob *virThreadPoolJobPtr; + +struct _virThreadPoolJob { + virThreadPoolJobPtr next; + + void *data; +}; + +typedef struct _virThreadPoolJobList virThreadPoolJobList; +typedef virThreadPoolJobList *virThreadPoolJobListPtr; + +struct _virThreadPoolJobList { + virThreadPoolJobPtr head; + virThreadPoolJobPtr *tail; +}; + + +struct _virThreadPool { + bool quit; + + virThreadPoolJobFunc jobFunc; + void *jobOpaque; + virThreadPoolJobList jobList; + + virMutex mutex; + virCond cond; + virCond quit_cond; + + size_t maxWorkers; + size_t freeWorkers; + size_t nWorkers; + virThreadPtr workers; +}; + +static void virThreadPoolWorker(void *opaque) +{ + virThreadPoolPtr pool = opaque; + + virMutexLock(&pool->mutex); + + while (1) { + while (!pool->quit && + !pool->jobList.head) { + pool->freeWorkers++; + if (virCondWait(&pool->cond, &pool->mutex) < 0) { + pool->freeWorkers--; + goto out; + } + pool->freeWorkers--; + } + + if (pool->quit) + break; + + virThreadPoolJobPtr job = pool->jobList.head; + pool->jobList.head = pool->jobList.head->next; + job->next = NULL; + if (pool->jobList.tail == &job->next) + pool->jobList.tail = &pool->jobList.head; + + virMutexUnlock(&pool->mutex); + (pool->jobFunc)(job->data, pool->jobOpaque); + VIR_FREE(job); + virMutexLock(&pool->mutex); + } + +out: + pool->nWorkers--; + if (pool->nWorkers == 0) + virCondSignal(&pool->quit_cond); + virMutexUnlock(&pool->mutex); +} + +virThreadPoolPtr virThreadPoolNew(size_t minWorkers, + size_t maxWorkers, + virThreadPoolJobFunc func, + void *opaque) +{ + virThreadPoolPtr pool; + size_t i; + + if (minWorkers > maxWorkers) + minWorkers = maxWorkers; + + if (VIR_ALLOC(pool) < 0) { + virReportOOMError(); + return NULL; + } + + pool->jobList.head = NULL; + pool->jobList.tail = &pool->jobList.head; + + pool->jobFunc = func; + pool->jobOpaque = opaque; + + if (virMutexInit(&pool->mutex) < 0) + goto error; + if (virCondInit(&pool->cond) < 0) + goto error; + if (virCondInit(&pool->quit_cond) < 0) + goto error; + + if (VIR_ALLOC_N(pool->workers, minWorkers) < 0) + goto error; + + pool->maxWorkers = maxWorkers; + for (i = 0; i < minWorkers; i++) { + if (virThreadCreate(&pool->workers[i], + true, + virThreadPoolWorker, + pool) < 0) { + goto error; + } + pool->nWorkers++; + } + + return pool; + +error: + virThreadPoolFree(pool); + return NULL; + +} + +void virThreadPoolFree(virThreadPoolPtr pool) +{ + virThreadPoolJobPtr job; + + if (!pool) + return; + + virMutexLock(&pool->mutex); + pool->quit = true; + if (pool->nWorkers > 0) { + virCondBroadcast(&pool->cond); + ignore_value(virCondWait(&pool->quit_cond, &pool->mutex)); + } + + while ((job = pool->jobList.head)) { + pool->jobList.head = pool->jobList.head->next; + VIR_FREE(job); + } + + VIR_FREE(pool->workers); + virMutexUnlock(&pool->mutex); + virMutexDestroy(&pool->mutex); + ignore_value(virCondDestroy(&pool->quit_cond)); + ignore_value(virCondDestroy(&pool->cond)); + VIR_FREE(pool); +} + +int virThreadPoolSendJob(virThreadPoolPtr pool, + void *jobData) +{ + virThreadPoolJobPtr job; + + virMutexLock(&pool->mutex); + if (pool->quit) + goto error; + + if (pool->freeWorkers == 0 && + pool->nWorkers < pool->maxWorkers) { + if (VIR_EXPAND_N(pool->workers, pool->nWorkers, 1) < 0) { + virReportOOMError(); + goto error; + } + + if (virThreadCreate(&pool->workers[pool->nWorkers - 1], + true, + virThreadPoolWorker, + pool) < 0) { + pool->nWorkers--; + goto error; + } + } + + if (VIR_ALLOC(job) < 0) { + virReportOOMError(); + goto error; + } + + job->data = jobData; + job->next = NULL; + *pool->jobList.tail = job; + pool->jobList.tail = &(*pool->jobList.tail)->next; + + virCondSignal(&pool->cond); + virMutexUnlock(&pool->mutex); + + return 0; + +error: + virMutexUnlock(&pool->mutex); + return -1; +} diff --git a/src/util/threadpool.h b/src/util/threadpool.h new file mode 100644 index 0000000..adab68f --- /dev/null +++ b/src/util/threadpool.h @@ -0,0 +1,48 @@ +/* + * threadpool.h: a generic thread pool implementation + * + * Copyright (C) 2010 Hu Tao + * Copyright (C) 2010 Daniel P. Berrange + * + * 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> + * Daniel P. Berrange <berrange@redhat.com> + */ + +#ifndef __VIR_THREADPOOL_H__ +#define __VIR_THREADPOOL_H__ + +#include "internal.h" + +typedef struct _virThreadPool virThreadPool; +typedef virThreadPool *virThreadPoolPtr; + +typedef void (*virThreadPoolJobFunc)(void *jobdata, void *opaque); + +virThreadPoolPtr virThreadPoolNew(size_t minWorkers, + size_t maxWorkers, + virThreadPoolJobFunc func, + void *opaque) ATTRIBUTE_NONNULL(3); + +void virThreadPoolFree(virThreadPoolPtr pool); + +int virThreadPoolSendJob(virThreadPoolPtr pool, + void *jobdata) ATTRIBUTE_NONNULL(1) + ATTRIBUTE_NONNULL(2) + ATTRIBUTE_RETURN_CHECK; + +#endif -- 1.7.3 -- Thanks, Hu Tao
On Wed, Dec 08, 2010 at 02:19:06PM +0800, Hu Tao wrote:
* src/util/threadpool.c, src/util/threadpool.h: Thread pool implementation * src/Makefile.am: Build thread pool * src/libvirt_private.syms: Export public functions --- cfg.mk | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 6 + src/util/threadpool.c | 231 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/threadpool.h | 48 ++++++++++ 5 files changed, 287 insertions(+), 0 deletions(-) create mode 100644 src/util/threadpool.c create mode 100644 src/util/threadpool.h
ACK Daniel
On Thu, Dec 09, 2010 at 10:53:56AM +0000, Daniel P. Berrange wrote:
On Wed, Dec 08, 2010 at 02:19:06PM +0800, Hu Tao wrote:
* src/util/threadpool.c, src/util/threadpool.h: Thread pool implementation * src/Makefile.am: Build thread pool * src/libvirt_private.syms: Export public functions --- cfg.mk | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 6 + src/util/threadpool.c | 231 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/threadpool.h | 48 ++++++++++ 5 files changed, 287 insertions(+), 0 deletions(-) create mode 100644 src/util/threadpool.c create mode 100644 src/util/threadpool.h
ACK
Thanks:) -- Thanks, Hu Tao
On 12/07/2010 11:19 PM, Hu Tao wrote: I fixed your 'make syntax-check' problems: preprocessor_indentation cppi: src/util/threadpool.h: line 27: not properly indented cppi: src/util/threadpool.h: line 29: not properly indented maint.mk: incorrect preprocessor indentation
+++ b/src/libvirt_private.syms @@ -898,3 +898,9 @@ virXPathStringLimit; virXPathULong; virXPathULongHex; virXPathULongLong; + + +# threadpool.h +virThreadPoolNew; +virThreadPoolFree; +virThreadPoolSendJob;
then sorted this (#threadpool.h section before threads.h, and sort lines within the section), and pushed. Thanks for your persistence; the end result is nice! -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
This patch prepares for the next patch. --- src/qemu/qemu_driver.c | 132 +++++++++++++++++++++++++++--------------------- 1 files changed, 74 insertions(+), 58 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f5164e1..bf101fc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6007,6 +6007,78 @@ 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) { @@ -6041,13 +6113,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); @@ -6069,26 +6138,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); @@ -6112,43 +6161,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); @@ -6181,8 +6199,6 @@ endjob: } cleanup: - if (ret != 0) - unlink(path); if (vm) virDomainObjUnlock(vm); if (event) -- 1.7.3 -- Thanks, Hu Tao
On Wed, Dec 08, 2010 at 02:19:12PM +0800, Hu Tao wrote:
This patch prepares for the next patch. --- src/qemu/qemu_driver.c | 132 +++++++++++++++++++++++++++--------------------- 1 files changed, 74 insertions(+), 58 deletions(-)
ACK Daniel
On 12/09/2010 03:54 AM, Daniel P. Berrange wrote:
On Wed, Dec 08, 2010 at 02:19:12PM +0800, Hu Tao wrote:
This patch prepares for the next patch. --- src/qemu/qemu_driver.c | 132 +++++++++++++++++++++++++++--------------------- 1 files changed, 74 insertions(+), 58 deletions(-)
ACK
Pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
`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. --- src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 5 ++ src/qemu/qemu_conf.c | 16 ++++++- src/qemu/qemu_conf.h | 5 ++ src/qemu/qemu_driver.c | 96 +++++++++++++++++++++++++++++++++++++++ src/qemu/test_libvirtd_qemu.aug | 2 + 8 files changed, 126 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9c54a59..109f6fc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -244,6 +244,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/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 78852f3..2f37015 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -37,6 +37,7 @@ module Libvirtd_qemu = | str_array_entry "cgroup_device_acl" | str_entry "save_image_format" | str_entry "dump_image_format" + | str_entry "auto_dump_path" | str_entry "hugetlbfs_mount" | bool_entry "relaxed_acs_check" | bool_entry "vnc_allow_host_audio" 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 d81e6cc..9c7dd49 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) { @@ -5270,7 +5281,10 @@ qemudBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, optstr); VIR_FREE(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 a1556cb..53f8185 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -43,6 +43,7 @@ # include "bitmap.h" # include "macvtap.h" # include "command.h" +# include "threadpool.h" # define qemudDebug(fmt, ...) do {} while(0) @@ -109,6 +110,8 @@ enum qemud_cmd_flags { struct qemud_driver { virMutex lock; + virThreadPoolPtr workerPool; + int privileged; uid_t user; @@ -176,6 +179,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 bf101fc..1a01807 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,14 @@ struct _qemuDomainObjPrivate { int persistentAddrs; }; +struct watchdogEvent +{ + virDomainObjPtr vm; + int action; +}; + +static void processWatchdogEvent(void *data, void *opaque); + static int qemudShutdown(void); static void qemuDriverLock(struct qemud_driver *driver) @@ -1204,6 +1213,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; + ignore_value(virThreadPoolSendJob(driver->workerPool, wdEvent)); + } else + virReportOOMError(); + } + virDomainObjUnlock(vm); if (watchdogEvent || lifecycleEvent) { @@ -1786,6 +1806,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 +1837,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 +1871,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 +2002,10 @@ qemudStartup(int privileged) { qemudAutostartConfigs(qemu_driver); + qemu_driver->workerPool = virThreadPoolNew(0, 1, processWatchdogEvent, qemu_driver); + if (!qemu_driver->workerPool) + goto error; + if (conn) virConnectClose(conn); @@ -2077,6 +2112,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); @@ -2112,6 +2148,7 @@ qemudShutdown(void) { qemuDriverUnlock(qemu_driver); virMutexDestroy(&qemu_driver->lock); + virThreadPoolFree(qemu_driver->workerPool); VIR_FREE(qemu_driver); return 0; @@ -6207,6 +6244,65 @@ cleanup: return ret; } +static void processWatchdogEvent(void *data, void *opaque) +{ + int ret; + struct watchdogEvent *wdEvent = data; + struct qemud_driver *driver = opaque; + + switch (wdEvent->action) { + case VIR_DOMAIN_WATCHDOG_ACTION_DUMP: + { + char *dumpfile; + int i; + + qemuDomainObjPrivatePtr priv = wdEvent->vm->privateData; + + i = virAsprintf(&dumpfile, "%s/%s-%u", + driver->autoDumpPath, + wdEvent->vm->def->name, + (unsigned int)time(NULL)); + + qemuDriverLock(driver); + virDomainObjLock(wdEvent->vm); + + if (qemuDomainObjBeginJobWithDriver(driver, wdEvent->vm) < 0) + break; + + if (!virDomainObjIsActive(wdEvent->vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + break; + } + + ret = doCoreDump(driver, + wdEvent->vm, + dumpfile, + getCompressionType(driver)); + if (ret < 0) + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("Dump failed")); + + qemuDomainObjEnterMonitorWithDriver(driver, wdEvent->vm); + ret = qemuMonitorStartCPUs(priv->mon, NULL); + qemuDomainObjExitMonitorWithDriver(driver, wdEvent->vm); + + if (ret < 0) + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("Resuming after dump failed")); + + if (qemuDomainObjEndJob(wdEvent->vm) > 0) + virDomainObjUnlock(wdEvent->vm); + + qemuDriverUnlock(driver); + + VIR_FREE(dumpfile); + } + break; + } + + VIR_FREE(wdEvent); +} static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) { diff --git a/src/qemu/test_libvirtd_qemu.aug b/src/qemu/test_libvirtd_qemu.aug index d3ae58d..ce5e69b 100644 --- a/src/qemu/test_libvirtd_qemu.aug +++ b/src/qemu/test_libvirtd_qemu.aug @@ -96,6 +96,8 @@ save_image_format = \"gzip\" dump_image_format = \"gzip\" +auto_dump_path = \"/var/lib/libvirt/qemu/dump\" + hugetlbfs_mount = \"/dev/hugepages\" set_process_name = 1 -- 1.7.3 -- Thanks, Hu Tao
On Wed, Dec 08, 2010 at 02:19:17PM +0800, Hu Tao wrote:
`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. --- src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 5 ++ src/qemu/qemu_conf.c | 16 ++++++- src/qemu/qemu_conf.h | 5 ++ src/qemu/qemu_driver.c | 96 +++++++++++++++++++++++++++++++++++++++ src/qemu/test_libvirtd_qemu.aug | 2 + 8 files changed, 126 insertions(+), 1 deletions(-)
ACK Daniel
On 09/12/2010, at 9:55 PM, Daniel P. Berrange wrote:
On Wed, Dec 08, 2010 at 02:19:17PM +0800, Hu Tao wrote:
`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. --- src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 5 ++ src/qemu/qemu_conf.c | 16 ++++++- src/qemu/qemu_conf.h | 5 ++ src/qemu/qemu_driver.c | 96 +++++++++++++++++++++++++++++++++++++++ src/qemu/test_libvirtd_qemu.aug | 2 + 8 files changed, 126 insertions(+), 1 deletions(-)
Doesn't seem to be any updates to the documentation with the patch, explaining what it does and how to use it?
On Fri, Dec 10, 2010 at 01:17:20AM +1100, Justin Clift wrote:
On 09/12/2010, at 9:55 PM, Daniel P. Berrange wrote:
On Wed, Dec 08, 2010 at 02:19:17PM +0800, Hu Tao wrote:
`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. --- src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 5 ++ src/qemu/qemu_conf.c | 16 ++++++- src/qemu/qemu_conf.h | 5 ++ src/qemu/qemu_driver.c | 96 +++++++++++++++++++++++++++++++++++++++ src/qemu/test_libvirtd_qemu.aug | 2 + 8 files changed, 126 insertions(+), 1 deletions(-)
Doesn't seem to be any updates to the documentation with the patch, explaining what it does and how to use it?
Thank you for pointing it out! -- Thanks, Hu Tao
On 12/07/2010 11:19 PM, Hu Tao wrote:
`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. --- src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 5 ++ src/qemu/qemu_conf.c | 16 ++++++- src/qemu/qemu_conf.h | 5 ++ src/qemu/qemu_driver.c | 96 +++++++++++++++++++++++++++++++++++++++ src/qemu/test_libvirtd_qemu.aug | 2 + 8 files changed, 126 insertions(+), 1 deletions(-)
Needed a bit of conflict resolution given other patches that have gone in since this was posted, as well as a tweak to pass 'make check' (test_libvirtd_qemu.aug was incomplete), but I've now pushed this patch since it had an ACK. As Daniel said, I'm omitting patch 4, and looking forward to his larger rewrite (which I did promise to review: https://www.redhat.com/archives/libvir-list/2010-December/msg00073.html). However, as Justin pointed out, the new dump option still needs documentation and testing: - mention 'dump' in docs/formatdomain.html.in - update to docs/schemas/domain.rng to list dump as a valid option - add an .xml file somewhere under tests/ that exercises the new XML option -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Thu, Dec 09, 2010 at 12:00:48PM -0700, Eric Blake wrote:
On 12/07/2010 11:19 PM, Hu Tao wrote:
`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. --- src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 5 ++ src/qemu/qemu_conf.c | 16 ++++++- src/qemu/qemu_conf.h | 5 ++ src/qemu/qemu_driver.c | 96 +++++++++++++++++++++++++++++++++++++++ src/qemu/test_libvirtd_qemu.aug | 2 + 8 files changed, 126 insertions(+), 1 deletions(-)
Needed a bit of conflict resolution given other patches that have gone in since this was posted, as well as a tweak to pass 'make check' (test_libvirtd_qemu.aug was incomplete), but I've now pushed this patch since it had an ACK. As Daniel said, I'm omitting patch 4, and looking forward to his larger rewrite (which I did promise to review: https://www.redhat.com/archives/libvir-list/2010-December/msg00073.html).
However, as Justin pointed out, the new dump option still needs documentation and testing: - mention 'dump' in docs/formatdomain.html.in - update to docs/schemas/domain.rng to list dump as a valid option - add an .xml file somewhere under tests/ that exercises the new XML option
OK, will update it. -- Thanks, Hu Tao
The xml watchdog dump option is converted to qemu watchdog pause arg but it is not reasonable to convert it back from qemu watchdog pause arg since there already is a xml watchdog pause option, so a test for the dump option to convert it from arg to xml is not added. --- I assume the dump option will appear in 0.8.7 and mark it since 0.8.7. docs/formatdomain.html.in | 8 +++++- docs/schemas/domain.rng | 1 + .../qemuxml2argv-watchdog-dump.args | 1 + .../qemuxml2argv-watchdog-dump.xml | 26 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 5 files changed, 36 insertions(+), 1 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-watchdog-dump.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-watchdog-dump.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 8db8b52..305bcb1 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1687,14 +1687,20 @@ qemu-kvm -net nic,model=? /dev/null <li>'poweroff' — forcefully power off the guest</li> <li>'pause' — pause the guest</li> <li>'none' — do nothing</li> + <li>'dump' — automatic dump the guest + <span class="since">Since 0.8.7</span></li> </ul> <p> - Note that the 'shutdown' action requires that the guest + Note 1: the 'shutdown' action requires that the guest is responsive to ACPI signals. In the sort of situations where the watchdog has expired, guests are usually unable to respond to ACPI signals. Therefore using 'shutdown' is not recommended. </p> + <p> + Note 2: the directory to save dump files can be configured + by <code>auto_dump_path</code> in file /etc/libvirt/qemu.conf. + </p> </dd> </dl> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 811d559..650ed7d 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1472,6 +1472,7 @@ <value>poweroff</value> <value>pause</value> <value>none</value> + <value>dump</value> </choice> </attribute> </optional> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-watchdog-dump.args b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-dump.args new file mode 100644 index 0000000..50b26f8 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-dump.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -watchdog ib700 -watchdog-action pause diff --git a/tests/qemuxml2argvdata/qemuxml2argv-watchdog-dump.xml b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-dump.xml new file mode 100644 index 0000000..4314ec4 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-watchdog-dump.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <watchdog model='ib700' action='dump'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 662f7bb..8328d65 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -399,6 +399,7 @@ mymain(int argc, char **argv) DO_TEST("watchdog", 0, false); DO_TEST("watchdog-device", QEMUD_CMD_FLAG_DEVICE | QEMUD_CMD_FLAG_NODEFCONFIG, false); + DO_TEST("watchdog-dump", 0, false); DO_TEST("balloon-device", QEMUD_CMD_FLAG_DEVICE | QEMUD_CMD_FLAG_NODEFCONFIG, false); DO_TEST("balloon-device-auto", QEMUD_CMD_FLAG_DEVICE | -- 1.7.3 -- Thanks, Hu Tao
On 12/09/2010 10:52 PM, Hu Tao wrote:
The xml watchdog dump option is converted to qemu watchdog pause arg but it is not reasonable to convert it back from qemu watchdog pause arg since there already is a xml watchdog pause option, so a test for the dump option to convert it from arg to xml is not added.
--- I assume the dump option will appear in 0.8.7 and mark it since 0.8.7.
Reasonable assumption; and if we go with 0.9.0 instead, there's several other places that can all be adjusted at once.
+++ b/docs/formatdomain.html.in @@ -1687,14 +1687,20 @@ qemu-kvm -net nic,model=? /dev/null <li>'poweroff' — forcefully power off the guest</li> <li>'pause' — pause the guest</li> <li>'none' — do nothing</li> + <li>'dump' — automatic dump the guest + <span class="since">Since 0.8.7</span></li>
s/automatic/automatically/ ACK with that fixed, so I pushed it. Thanks again for the quick response! -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
--- daemon/libvirtd.c | 187 ++++++++--------------------------------------------- daemon/libvirtd.h | 16 +---- 2 files changed, 30 insertions(+), 173 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 791b3dc..229c0cc 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, @@ -842,18 +842,10 @@ static struct qemud_server *qemudInitialize(void) { VIR_FREE(server); return NULL; } - if (virCondInit(&server->job) < 0) { - VIR_ERROR0(_("cannot initialize condition variable")); - virMutexDestroy(&server->lock); - VIR_FREE(server); - return NULL; - } if (virEventInit() < 0) { VIR_ERROR0(_("Failed to initialize event system")); virMutexDestroy(&server->lock); - if (virCondDestroy(&server->job) < 0) - {} VIR_FREE(server); return NULL; } @@ -1458,19 +1450,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 +1513,28 @@ void qemudDispatchClientFailure(struct qemud_client *client) { VIR_FREE(client->addrstr); } - -/* Caller must hold server lock */ -static struct qemud_client *qemudPendingJob(struct qemud_server *server) -{ - 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; -} - -static void *qemudWorker(void *data) +static void qemudWorker(void *data, void *opaque) { - struct qemud_worker *worker = data; - struct qemud_server *server = worker->server; - - while (1) { - struct qemud_client *client = NULL; - struct qemud_client_message *msg; - - 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);*/ + struct qemud_server *server = opaque; + struct qemud_client *client = data; + struct qemud_client_message *msg; - if (worker->hasThread) - return -1; + virMutexLock(&client->lock); - worker->server = server; - worker->hasThread = 1; - worker->quitRequest = 0; - worker->processingCall = 0; + /* Remove our message from dispatch queue while we use it */ + msg = qemudClientMessageQueueServe(&client->dx); - 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 (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 +1764,11 @@ readmore: } /* Move completed message to the end of the dispatch queue */ - if (msg) + if (msg) { + client->refs++; qemudClientMessageQueuePush(&client->dx, msg); + ignore_value(virThreadPoolSendJob(server->workerPool, client)); + } client->nrequests++; /* Possibly need to create another receive buffer */ @@ -1870,9 +1780,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); } } } @@ -2305,18 +2212,16 @@ static void *qemudRunLoop(void *opaque) { if (min_workers > max_workers) max_workers = min_workers; - server->nworkers = max_workers; - if (VIR_ALLOC_N(server->workers, server->nworkers) < 0) { - VIR_ERROR0(_("Failed to allocate workers")); + server->workerPool = virThreadPoolNew(min_workers, + max_workers, + qemudWorker, + server); + if (!server->workerPool) { + VIR_ERROR0(_("Failed to create thread pool")); + virMutexUnlock(&server->lock); return NULL; } - for (i = 0 ; i < min_workers ; i++) { - if (qemudStartWorker(server, &server->workers[i]) < 0) - goto cleanup; - server->nactiveworkers++; - } - for (;!server->quitEventThread;) { /* A shutdown timeout is specified, so check * if any drivers have active state, if not @@ -2367,47 +2272,14 @@ 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); for (i = 0; i < server->nclients; i++) qemudFreeClient(server->clients[i]); server->nclients = 0; VIR_SHRINK_N(server->clients, server->nclients_max, server->nclients_max); + virThreadPoolFree(server->workerPool); + server->workerPool = NULL; virMutexUnlock(&server->lock); return NULL; } @@ -2475,9 +2347,6 @@ static void qemudCleanup(struct qemud_server *server) { virStateCleanup(); - if (virCondDestroy(&server->job) < 0) { - ; - } virMutexDestroy(&server->lock); VIR_FREE(server); diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index af20e56..e4ee63b 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 @@ -266,26 +267,13 @@ struct qemud_socket { struct qemud_socket *next; }; -struct qemud_worker { - pthread_t thread; - unsigned int hasThread :1; - unsigned int processingCall :1; - unsigned int quitRequest :1; - - /* back-pointer to our server */ - struct qemud_server *server; -}; - /* Main server state */ struct qemud_server { virMutex lock; - virCond job; int privileged; - size_t nworkers; - size_t nactiveworkers; - struct qemud_worker *workers; + virThreadPoolPtr workerPool; size_t nsockets; struct qemud_socket *sockets; size_t nclients; -- 1.7.3 -- Thanks, Hu Tao
On Wed, Dec 08, 2010 at 02:19:23PM +0800, Hu Tao wrote:
--- daemon/libvirtd.c | 187 ++++++++--------------------------------------------- daemon/libvirtd.h | 16 +---- 2 files changed, 30 insertions(+), 173 deletions(-)
While there is nothing wrong with this patch, I'd prefer if we didn't apply this one. I've got a huge patch series pending which basically deletes all the thread & RPC code from the libvirtd daemon, and replaces it with calls to a new set of libvirt internal RPC APIs. These new APIs of course make use of your thread pool implementation, so the net result will be the same. Regards, Daniel
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Hu Tao -
Justin Clift