POSIX says pthread_t is opaque. We can't guarantee if it is scaler or a pointer, nor what size it is; and BSD differs from Linux. We've also had reports of gcc complaining on attempts to cast it, if we use a cast to the wrong type (for example, pointers have to be cast to void* or intptr_t before being narrowed; while casting a function return of pthread_t to void* triggers another warning). Give up on casts, and use unions to get at decent bits instead. And rather than futz around with figuring which 32 bits of a potentially 64-bit pointer are most likely to be unique, convert the rest of the code base to use 64-bit values when using a debug id. Based on a report by Guido Günther against kFreeBSD, but with a fix that doesn't regress commit 4d970fd29 for FreeBSD. * src/util/virthreadpthread.c (virThreadSelfID, virThreadID): Use union to get at a decent bit representation of thread_t bits. * src/util/virthread.h (virThreadSelfID, virThreadID): Alter signature. * src/util/virthreadwin32.c (virThreadSelfID, virThreadID): Likewise. * src/qemu/qemu_domain.h (qemuDomainJobObj): Alter type of owner. * src/qemu/qemu_domain.c (qemuDomainObjTransferJob) (qemuDomainObjSetJobPhase, qemuDomainObjReleaseAsyncJob) (qemuDomainObjBeginNestedJob, qemuDomainObjBeginJobInternal): Fix clients. * src/util/virlog.c (virLogFormatString): Likewise. * src/util/vireventpoll.c (virEventPollInterruptLocked): Likewise. Signed-off-by: Eric Blake <eblake@xxxxxxxxxx> --- This one needs a review. I've tested this on Fedora 18 and FreeBSD 8.2; I also ran it through the mingw64 cross-compiler on Fedora (thanks to autobuild.sh), without grief. But I don't have a kFreeBSD system handy, which was the original source of the compilation complaint. src/qemu/qemu_domain.c | 12 ++++++------ src/qemu/qemu_domain.h | 4 ++-- src/util/vireventpoll.c | 4 ++-- src/util/virlog.c | 6 +++--- src/util/virthread.h | 6 +++--- src/util/virthreadpthread.c | 33 ++++++++++++++++++++++----------- src/util/virthreadwin32.c | 12 ++++++------ 7 files changed, 44 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 47edfba..bfc57e5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -187,7 +187,7 @@ qemuDomainObjTransferJob(virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; - VIR_DEBUG("Changing job owner from %d to %d", + VIR_DEBUG("Changing job owner from %lld to %lld", priv->job.owner, virThreadSelfID()); priv->job.owner = virThreadSelfID(); } @@ -846,7 +846,7 @@ qemuDomainObjSetJobPhase(virQEMUDriverPtr driver, int phase) { qemuDomainObjPrivatePtr priv = obj->privateData; - int me = virThreadSelfID(); + unsigned long long int me = virThreadSelfID(); if (!priv->job.asyncJob) return; @@ -856,7 +856,7 @@ qemuDomainObjSetJobPhase(virQEMUDriverPtr driver, qemuDomainAsyncJobPhaseToString(priv->job.asyncJob, phase)); if (priv->job.asyncOwner && me != priv->job.asyncOwner) { - VIR_WARN("'%s' async job is owned by thread %d", + VIR_WARN("'%s' async job is owned by thread %lld", qemuDomainAsyncJobTypeToString(priv->job.asyncJob), priv->job.asyncOwner); } @@ -898,7 +898,7 @@ qemuDomainObjReleaseAsyncJob(virDomainObjPtr obj) qemuDomainAsyncJobTypeToString(priv->job.asyncJob)); if (priv->job.asyncOwner != virThreadSelfID()) { - VIR_WARN("'%s' async job is owned by thread %d", + VIR_WARN("'%s' async job is owned by thread %lld", qemuDomainAsyncJobTypeToString(priv->job.asyncJob), priv->job.asyncOwner); } @@ -992,7 +992,7 @@ retry: error: VIR_WARN("Cannot start job (%s, %s) for domain %s;" - " current job is (%s, %s) owned by (%d, %d)", + " current job is (%s, %s) owned by (%lld, %lld)", qemuDomainJobTypeToString(job), qemuDomainAsyncJobTypeToString(asyncJob), obj->def->name, @@ -1056,7 +1056,7 @@ qemuDomainObjBeginNestedJob(virQEMUDriverPtr driver, } if (priv->job.asyncOwner != virThreadSelfID()) { - VIR_WARN("This thread doesn't seem to be the async job owner: %d", + VIR_WARN("This thread doesn't seem to be the async job owner: %lld", priv->job.asyncOwner); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index da04377..7746ba9 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -102,11 +102,11 @@ VIR_ENUM_DECL(qemuDomainAsyncJob) struct qemuDomainJobObj { virCond cond; /* Use to coordinate jobs */ enum qemuDomainJob active; /* Currently running job */ - int owner; /* Thread which set current job */ + unsigned long long int owner; /* Thread id which set current job */ virCond asyncCond; /* Use to coordinate with async jobs */ enum qemuDomainAsyncJob asyncJob; /* Currently active async job */ - int asyncOwner; /* Thread which set current async job */ + unsigned long long int asyncOwner; /* Thread which set current async job */ int phase; /* Job phase (mainly for migrations) */ unsigned long long mask; /* Jobs allowed during async job */ unsigned long long start; /* When the async job started */ diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c index 5c6d31b..19add73 100644 --- a/src/util/vireventpoll.c +++ b/src/util/vireventpoll.c @@ -1,7 +1,7 @@ /* * vireventpoll.c: Poll based event loop for monitoring file handles * - * Copyright (C) 2007, 2010-2012 Red Hat, Inc. + * Copyright (C) 2007, 2010-2013 Red Hat, Inc. * Copyright (C) 2007 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -708,7 +708,7 @@ static int virEventPollInterruptLocked(void) if (!eventLoop.running || virThreadIsSelf(&eventLoop.leader)) { - VIR_DEBUG("Skip interrupt, %d %d", eventLoop.running, + VIR_DEBUG("Skip interrupt, %d %lld", eventLoop.running, virThreadID(&eventLoop.leader)); return 0; } diff --git a/src/util/virlog.c b/src/util/virlog.c index bd47b38..b2e6458 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1,7 +1,7 @@ /* * virlog.c: internal logging and debugging * - * Copyright (C) 2008, 2010-2012 Red Hat, Inc. + * Copyright (C) 2008, 2010-2013 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 @@ -708,11 +708,11 @@ virLogFormatString(char **msg, * to just grep for it to find the right place. */ if ((funcname != NULL)) { - ret = virAsprintf(msg, "%d: %s : %s:%d : %s\n", + ret = virAsprintf(msg, "%lld: %s : %s:%d : %s\n", virThreadSelfID(), virLogPriorityString(priority), funcname, linenr, str); } else { - ret = virAsprintf(msg, "%d: %s : %s\n", + ret = virAsprintf(msg, "%lld: %s : %s\n", virThreadSelfID(), virLogPriorityString(priority), str); } diff --git a/src/util/virthread.h b/src/util/virthread.h index ca1306a..447fe84 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -1,7 +1,7 @@ /* * virthread.h: basic thread synchronization primitives * - * Copyright (C) 2009-2011 Red Hat, Inc. + * Copyright (C) 2009-2011, 2013 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 @@ -63,8 +63,8 @@ void virThreadCancel(virThreadPtr thread); * guaranteed to give unique values for distinct threads on all * architectures, nor are the two functions guaranteed to give the same * value for the same thread. */ -int virThreadSelfID(void); -int virThreadID(virThreadPtr thread); +unsigned long long int virThreadSelfID(void); +unsigned long long int virThreadID(virThreadPtr thread); /* Static initialization of mutexes is not possible, so we instead * provide for guaranteed one-time initialization via a callback diff --git a/src/util/virthreadpthread.c b/src/util/virthreadpthread.c index b42b333..20fa539 100644 --- a/src/util/virthreadpthread.c +++ b/src/util/virthreadpthread.c @@ -1,7 +1,7 @@ /* * virthreadpthread.c: basic thread synchronization primitives * - * Copyright (C) 2009-2011 Red Hat, Inc. + * Copyright (C) 2009-2011, 2013 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 @@ -210,25 +210,36 @@ bool virThreadIsSelf(virThreadPtr thread) return pthread_equal(pthread_self(), thread->thread) ? true : false; } -/* For debugging use only; this result is not guaranteed unique on BSD - * systems when pthread_t is a 64-bit pointer. */ -int virThreadSelfID(void) +/* For debugging use only; this result is not guaranteed unique if + * pthread_t is larger than a 64-bit pointer, nor does it always match + * the pthread_self() id on Linux. */ +unsigned long long int virThreadSelfID(void) { #if defined(HAVE_SYS_SYSCALL_H) && defined(SYS_gettid) pid_t tid; tid = syscall(SYS_gettid); - return (int)tid; + return tid; #else - return (int)(intptr_t)(void *)pthread_self(); + union { + unsigned long long int i; + pthread_t t; + } u; + u.t = pthread_self(); + return u.i; #endif } -/* For debugging use only; this result is not guaranteed unique on BSD - * systems when pthread_t is a 64-bit pointer, nor does it match the - * thread id of virThreadSelfID on Linux. */ -int virThreadID(virThreadPtr thread) +/* For debugging use only; this result is not guaranteed unique if + * pthread_t is larger than a 64-bit pointer, nor does it always match + * the thread id of virThreadSelfID on Linux. */ +unsigned long long int virThreadID(virThreadPtr thread) { - return (int)(uintptr_t)thread->thread; + union { + unsigned long long int i; + pthread_t t; + } u; + u.t = thread->thread; + return u.i; } void virThreadJoin(virThreadPtr thread) diff --git a/src/util/virthreadwin32.c b/src/util/virthreadwin32.c index 4543ad8..10837d2 100644 --- a/src/util/virthreadwin32.c +++ b/src/util/virthreadwin32.c @@ -1,7 +1,7 @@ /* * virthreadwin32.c: basic thread synchronization primitives * - * Copyright (C) 2009-2011 Red Hat, Inc. + * Copyright (C) 2009-2011, 2013 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 @@ -334,14 +334,14 @@ bool virThreadIsSelf(virThreadPtr thread) return self.thread == thread->thread ? true : false; } -/* For debugging use only; see comments in threads-pthread.c. */ -int virThreadSelfID(void) +/* For debugging use only; see comments in virthreadpthread.c. */ +unsigned long long int virThreadSelfID(void) { - return (int)GetCurrentThreadId(); + return GetCurrentThreadId(); } -/* For debugging use only; see comments in threads-pthread.c. */ -int virThreadID(virThreadPtr thread) +/* For debugging use only; see comments in virthreadpthread.c. */ +unsigned long long int virThreadID(virThreadPtr thread) { return (intptr_t)thread->thread; } -- 1.8.1.4 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list