Each thread can use a thread local variable to keep the name of a job which is currently running in the job. The virThreadJobSetWorker API is supposed to be called once by any thread which is used as a worker, i.e., it is waiting in a pool, woken up to do a job, and returned back to the pool. The virThreadJobSet/virThreadJobClear APIs are to be called at the beginning/end of each job. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- include/libvirt/virterror.h | 1 + po/POTFILES.in | 1 + src/Makefile.am | 2 + src/libvirt_private.syms | 7 +++ src/util/virerror.c | 1 + src/util/virthreadjob.c | 126 ++++++++++++++++++++++++++++++++++++++++++++ src/util/virthreadjob.h | 33 ++++++++++++ 7 files changed, 171 insertions(+) create mode 100644 src/util/virthreadjob.c create mode 100644 src/util/virthreadjob.h diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 5dc99dc..9c5b069 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -125,6 +125,7 @@ typedef enum { VIR_FROM_FIREWALL = 59, /* Error from firewall */ VIR_FROM_POLKIT = 60, /* Error from polkit code */ + VIR_FROM_THREAD = 61, /* Error from thread utils */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_DOMAIN_LAST diff --git a/po/POTFILES.in b/po/POTFILES.in index ab6ae3b..dd06ab3 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -220,6 +220,7 @@ src/util/virstorageencryption.c src/util/virstoragefile.c src/util/virstring.c src/util/virsysinfo.c +src/util/virthreadjob.c src/util/virtime.c src/util/virtpm.c src/util/virtypedparam.c diff --git a/src/Makefile.am b/src/Makefile.am index 00b47e7..ec62ab4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -160,6 +160,7 @@ UTIL_SOURCES = \ util/virsysinfo.c util/virsysinfo.h \ util/virsystemd.c util/virsystemd.h \ util/virthread.c util/virthread.h \ + util/virthreadjob.c utils/virthreadjob.h \ util/virthreadpool.c util/virthreadpool.h \ util/virtime.h util/virtime.c \ util/virtpm.h util/virtpm.c \ @@ -2147,6 +2148,7 @@ libvirt_setuid_rpc_client_la_SOURCES = \ util/virstring.c \ util/virtime.c \ util/virthread.c \ + util/virthreadjob.c \ util/virtypedparam.c \ util/viruri.c \ util/virutil.c \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8141eba..f5756a7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2183,6 +2183,13 @@ virThreadSelf; virThreadSelfID; +# util/virthreadjob.h +virThreadJobClear; +virThreadJobGet; +virThreadJobSet; +virThreadJobSetWorker; + + # util/virthreadpool.h virThreadPoolFree; virThreadPoolGetMaxWorkers; diff --git a/src/util/virerror.c b/src/util/virerror.c index 9635c82..c4e84e7 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -132,6 +132,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST, "Firewall", "Polkit", /* 60 */ + "Thread jobs", ) diff --git a/src/util/virthreadjob.c b/src/util/virthreadjob.c new file mode 100644 index 0000000..2f483f9 --- /dev/null +++ b/src/util/virthreadjob.c @@ -0,0 +1,126 @@ +/* + * virthreadjob.c: code for tracking job associated with current thread + * + * Copyright (C) 2013-2015 Red Hat, Inc. + * + * 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, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Jiri Denemark <jdenemar(a)redhat.com&gt; + */ + +#include <config.h> + +#include "internal.h" +#include "virerror.h" +#include "virlog.h" +#include "virthread.h" +#include "virthreadjob.h" + +#define VIR_FROM_THIS VIR_FROM_THREAD +VIR_LOG_INIT("util.threadjob"); + +virThreadLocal virThreadJobWorker; +virThreadLocal virThreadJobName; + + +static int +virThreadJobOnceInit(void) +{ + if (virThreadLocalInit(&virThreadJobWorker, NULL) < 0 || + virThreadLocalInit(&virThreadJobName, NULL) < 0) + return -1; + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virThreadJob) + + +const char * +virThreadJobGet(void) +{ + const char *job; + + if (virThreadJobInitialize() < 0) + return NULL; + + job = virThreadLocalGet(&virThreadJobName); + if (!job) + job = virThreadLocalGet(&virThreadJobWorker); + + return job; +} + + +void +virThreadJobSetWorker(const char *worker) +{ + if (!worker || virThreadJobInitialize() < 0) + return; + + if (virThreadLocalSet(&virThreadJobWorker, (void *) worker) < 0) + virReportSystemError(errno, + _("cannot set worker name to %s"), + worker); + + VIR_DEBUG("Thread %llu is running worker %s", virThreadSelfID(), worker); +} + + +void +virThreadJobSet(const char *caller) +{ + const char *worker; + + if (!caller || virThreadJobInitialize() < 0) + return; + + if (virThreadLocalSet(&virThreadJobName, (void *) caller) < 0) + virReportSystemError(errno, + _("cannot set current job to %s"), + caller); + + if ((worker = virThreadLocalGet(&virThreadJobWorker))) { + VIR_DEBUG("Thread %llu (%s) is now running job %s", + virThreadSelfID(), worker, caller); + } else { + VIR_DEBUG("Thread %llu is now running job %s", + virThreadSelfID(), caller); + } +} + + +void +virThreadJobClear(int rv) +{ + const char *old; + const char *worker; + + if (virThreadJobInitialize() < 0) + return; + + if (!(old = virThreadLocalGet(&virThreadJobName))) + return; + + if (virThreadLocalSet(&virThreadJobName, NULL) < 0) + virReportSystemError(errno, "%s", _("cannot reset current job")); + + if ((worker = virThreadLocalGet(&virThreadJobWorker))) { + VIR_DEBUG("Thread %llu (%s) finished job %s with ret=%d", + virThreadSelfID(), worker, old, rv); + } else { + VIR_DEBUG("Thread %llu finished job %s with ret=%d", + virThreadSelfID(), old, rv); + } +} diff --git a/src/util/virthreadjob.h b/src/util/virthreadjob.h new file mode 100644 index 0000000..659f91d --- /dev/null +++ b/src/util/virthreadjob.h @@ -0,0 +1,33 @@ +/* + * virthreadjob.h: APIs for tracking job associated with current thread + * + * Copyright (C) 2013-2015 Red Hat, Inc. + * + * 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, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Jiri Denemark <jdenemar(a)redhat.com&gt; + */ + +#ifndef __VIR_THREAD_JOB_H__ +# define __VIR_THREAD_JOB_H__ + + +const char *virThreadJobGet(void); + +void virThreadJobSetWorker(const char *caller); +void virThreadJobSet(const char *caller); +void virThreadJobClear(int rv); + +#endif -- 2.3.3

Automatically assign a job to every thread created by virThreadCreate. The name of the virThreadFunc function passed to virThreadCreate is used as the job or worker name in case no name is explicitly passed. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/libvirt_private.syms | 2 +- src/util/virthread.c | 25 +++++++++++++++++++++---- src/util/virthread.h | 13 +++++++++---- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f5756a7..4980305 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2174,7 +2174,7 @@ virRWLockRead; virRWLockUnlock; virRWLockWrite; virThreadCancel; -virThreadCreate; +virThreadCreateFull; virThreadID; virThreadInitialize; virThreadIsSelf; diff --git a/src/util/virthread.c b/src/util/virthread.c index 7e841d1..c2a9e7f 100644 --- a/src/util/virthread.c +++ b/src/util/virthread.c @@ -30,6 +30,7 @@ #endif #include "viralloc.h" +#include "virthreadjob.h" /* Nothing special required for pthreads */ @@ -184,6 +185,8 @@ void virCondBroadcast(virCondPtr c) struct virThreadArgs { virThreadFunc func; + const char *funcName; + bool worker; void *opaque; }; @@ -194,14 +197,26 @@ static void *virThreadHelper(void *data) /* Free args early, rather than tying it up during the entire thread. */ VIR_FREE(args); + + if (local.worker) + virThreadJobSetWorker(local.funcName); + else + virThreadJobSet(local.funcName); + local.func(local.opaque); + + if (!local.worker) + virThreadJobClear(0); + return NULL; } -int virThreadCreate(virThreadPtr thread, - bool joinable, - virThreadFunc func, - void *opaque) +int virThreadCreateFull(virThreadPtr thread, + bool joinable, + virThreadFunc func, + const char *funcName, + bool worker, + void *opaque) { struct virThreadArgs *args; pthread_attr_t attr; @@ -216,6 +231,8 @@ int virThreadCreate(virThreadPtr thread, } args->func = func; + args->funcName = funcName; + args->worker = worker; args->opaque = opaque; if (!joinable) diff --git a/src/util/virthread.h b/src/util/virthread.h index 7146f0f..e466d9b 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -88,10 +88,15 @@ void virThreadOnExit(void); typedef void (*virThreadFunc)(void *opaque); -int virThreadCreate(virThreadPtr thread, - bool joinable, - virThreadFunc func, - void *opaque) ATTRIBUTE_RETURN_CHECK; +# define virThreadCreate(thread, joinable, func, opaque) \ + virThreadCreateFull(thread, joinable, func, #func, false, opaque) + +int virThreadCreateFull(virThreadPtr thread, + bool joinable, + virThreadFunc func, + const char *funcName, + bool worker, + void *opaque) ATTRIBUTE_RETURN_CHECK; void virThreadSelf(virThreadPtr thread); bool virThreadIsSelf(virThreadPtr thread); void virThreadJoin(virThreadPtr thread); -- 2.3.3

Since all APIs are also RPC calls, we automatically get all APIs covered with thread jobs. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- daemon/remote.c | 1 + src/locking/lock_daemon_dispatch.c | 1 + src/qemu/qemu_domain.c | 1 + src/rpc/gendispatch.pl | 6 +++++- 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index 62a4728..ea7ae94 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -51,6 +51,7 @@ #include "viraccessapicheck.h" #include "viraccessapicheckqemu.h" #include "virpolkit.h" +#include "virthreadjob.h" #define VIR_FROM_THIS VIR_FROM_RPC diff --git a/src/locking/lock_daemon_dispatch.c b/src/locking/lock_daemon_dispatch.c index 168a6af..a7cee9d 100644 --- a/src/locking/lock_daemon_dispatch.c +++ b/src/locking/lock_daemon_dispatch.c @@ -29,6 +29,7 @@ #include "lock_daemon.h" #include "lock_protocol.h" #include "virerror.h" +#include "virthreadjob.h" #define VIR_FROM_THIS VIR_FROM_RPC diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 949bf8b..559400c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -39,6 +39,7 @@ #include "virtime.h" #include "virstoragefile.h" #include "virstring.h" +#include "virthreadjob.h" #include "storage/storage_driver.h" diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 8b488eb..aa73d0c 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -428,8 +428,10 @@ elsif ($mode eq "server") { print " void *args$argann,\n"; print " void *ret$retann)\n"; print "{\n"; + print " int rv;\n"; + print " virThreadJobSet(\"$name\");\n"; print " VIR_DEBUG(\"server=%p client=%p msg=%p rerr=%p args=%p ret=%p\", server, client, msg, rerr, args, ret);\n"; - print " return $name(server, client, msg, rerr"; + print " rv = $name(server, client, msg, rerr"; if ($argtype ne "void") { print ", args"; } @@ -437,6 +439,8 @@ elsif ($mode eq "server") { print ", ret"; } print ");\n"; + print " virThreadJobClear(rv);\n"; + print " return rv;\n"; print "}\n"; # Finally we print out the dispatcher method body impl -- 2.3.3

Whenever we fail to acquire a job, we can report how long ago it was locked by another API. https://bugzilla.redhat.com/show_bug.cgi?id=853839 Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 20 ++++++++++++++++++-- src/qemu/qemu_domain.h | 2 ++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index dc55016..72b1b84 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -154,6 +154,7 @@ qemuDomainObjResetJob(qemuDomainObjPrivatePtr priv) job->active = QEMU_JOB_NONE; job->owner = 0; job->ownerAPI = NULL; + job->started = 0; } static void @@ -164,6 +165,7 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv) job->asyncJob = QEMU_ASYNC_JOB_NONE; job->asyncOwner = 0; job->asyncOwnerAPI = NULL; + job->asyncStarted = 0; job->phase = 0; job->mask = QEMU_JOB_DEFAULT_MASK; job->dump_memory_only = false; @@ -1326,6 +1328,8 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *blocker = NULL; int ret = -1; + unsigned long long duration = 0; + unsigned long long asyncDuration = 0; VIR_DEBUG("Starting %s: %s (async=%s vm=%p name=%s)", job == QEMU_JOB_ASYNC ? "async job" : "job", @@ -1366,6 +1370,8 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, qemuDomainObjResetJob(priv); + ignore_value(virTimeMillisNow(&now)); + if (job != QEMU_JOB_ASYNC) { VIR_DEBUG("Started job: %s (async=%s vm=%p name=%s)", qemuDomainJobTypeToString(job), @@ -1374,6 +1380,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, priv->job.active = job; priv->job.owner = virThreadSelfID(); priv->job.ownerAPI = virThreadJobGet(); + priv->job.started = now; } else { VIR_DEBUG("Started async job: %s (vm=%p name=%s)", qemuDomainAsyncJobTypeToString(asyncJob), @@ -1384,6 +1391,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, priv->job.asyncJob = asyncJob; priv->job.asyncOwner = virThreadSelfID(); priv->job.asyncOwnerAPI = virThreadJobGet(); + priv->job.asyncStarted = now; priv->job.current->started = now; } @@ -1394,15 +1402,23 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, return 0; error: + ignore_value(virTimeMillisNow(&now)); + if (priv->job.active && priv->job.started) + duration = now - priv->job.started; + if (priv->job.asyncJob && priv->job.asyncStarted) + asyncDuration = now - priv->job.asyncStarted; + VIR_WARN("Cannot start job (%s, %s) for domain %s; " - "current job is (%s, %s) owned by (%llu %s, %llu %s)", + "current job is (%s, %s) owned by (%llu %s, %llu %s) " + "for (%llus, %llus)", qemuDomainJobTypeToString(job), qemuDomainAsyncJobTypeToString(asyncJob), obj->def->name, qemuDomainJobTypeToString(priv->job.active), qemuDomainAsyncJobTypeToString(priv->job.asyncJob), priv->job.owner, NULLSTR(priv->job.ownerAPI), - priv->job.asyncOwner, NULLSTR(priv->job.asyncOwnerAPI)); + priv->job.asyncOwner, NULLSTR(priv->job.asyncOwnerAPI), + duration / 1000, asyncDuration / 1000); if (nested || qemuDomainNestedJobAllowed(priv, job)) blocker = priv->job.ownerAPI; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index a468982..9da80ca 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -118,11 +118,13 @@ struct qemuDomainJobObj { qemuDomainJob active; /* Currently running job */ unsigned long long owner; /* Thread id which set current job */ const char *ownerAPI; /* The API which owns the job */ + unsigned long long started; /* When the current job started */ virCond asyncCond; /* Use to coordinate with async jobs */ qemuDomainAsyncJob asyncJob; /* Currently active async job */ unsigned long long asyncOwner; /* Thread which set current async job */ const char *asyncOwnerAPI; /* The API which owns the async job */ + unsigned long long asyncStarted; /* When the current async job started */ int phase; /* Job phase (mainly for migrations) */ unsigned long long mask; /* Jobs allowed during async job */ bool dump_memory_only; /* use dump-guest-memory to do dump */ -- 2.3.3