Re: [libvirt] [PATCH] build: avoid non-portable cast of pthread_t

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(a)redhat.com&gt; --- 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",

@@ -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",

@@ -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",

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 */

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 @@ -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,