On Thu, May 02, 2013 at 03:58:23PM -0600, Eric Blake wrote:
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@redhat.com> ---
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",
s/lld/llu/ since you declared it unsigned
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();
s/int//; 'unsigned long long' is verbose enough already without adding a redundent 'int' suffix on it too. Same throughout this patch.
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",
s/lld/llu/
@@ -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",
s/lld/llu/
@@ -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)",
s/lld/llu/g
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",
s/lld/llu/
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 */
s/int//;
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 */
s/int//;
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,
s/lld/llu/ etc, etc. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|