On Tue, Jun 28, 2016 at 10:49:33AM +0200, Martin Kletzander wrote: > On Tue, Jun 28, 2016 at 10:12:01AM +0200, Martin Kletzander wrote: > > On Mon, Jun 27, 2016 at 02:28:39PM -0400, Cole Robinson wrote: > > > Similar to what virsh and virt-login-shell do > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1350315 > > > --- > > > I can't actually reproduce the bug, the backtrace is similar to > > > 97973ebb7 which added the same fix for virt-login-shell, and > > > that commit also mentions the randomness of reproducing... > > > > > > > I think we should try finding out what the cause for that is. It might > > be as simple as adding similar fix to yours and asking the user what > > error does he see with that fix in. The idea behind that is that > > scripts shouldn't need to call that initialization as that should be > > part of any first call they make to the library. > > > > > tools/virt-admin.c | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/tools/virt-admin.c b/tools/virt-admin.c > > > index 4acac65..7bff5c3 100644 > > > --- a/tools/virt-admin.c > > > +++ b/tools/virt-admin.c > > > @@ -1371,6 +1371,11 @@ main(int argc, char **argv) > > > return EXIT_FAILURE; > > > } > > > > > > + if (virInitialize() < 0) { > > > > If this is the right thing to do, it should be virAdmInitialize(). By > > using virInitialize() we're giving bad example to others as it only > > works in virt-admin thanks to internal.h being included, I guess. > > > > I tried this instead, what do you think about it? > > diff --git i/src/util/virerror.c w/src/util/virerror.c > index 1177570ef0d5..300b0b90252f 100644 > --- i/src/util/virerror.c > +++ w/src/util/virerror.c > @@ -37,7 +37,7 @@ > > VIR_LOG_INIT("util.error"); > > -virThreadLocal virLastErr; > +static virThreadLocal virLastErr; > > virErrorFunc virErrorHandler = NULL; /* global error handler */ > void *virUserData = NULL; /* associated data */ Has no functional effect. > diff --git i/src/util/virevent.c w/src/util/virevent.c > index e0fd35e41644..8f9d15e46454 100644 > --- i/src/util/virevent.c > +++ w/src/util/virevent.c > @@ -266,6 +266,9 @@ int virEventRegisterDefaultImpl(void) > { > VIR_DEBUG("registering default event implementation"); > > + if (virErrorInitialize() < 0) > + return -1; > + > virResetLastError(); > > if (virEventPollInit() < 0) { This is just hacking around fact that we didn't call virInitialize/virAdmInitialize(). > diff --git i/src/util/virthread.c w/src/util/virthread.c > index 6c495158f566..4b69451dd355 100644 > --- i/src/util/virthread.c > +++ w/src/util/virthread.c > @@ -308,7 +308,8 @@ int virThreadLocalInit(virThreadLocalPtr l, > virThreadLocalCleanup c) > { > int ret; > - if ((ret = pthread_key_create(&l->key, c)) != 0) { > + if (!l->initialized && > + (ret = pthread_key_create(&l->key, c)) != 0) { > errno = ret; > return -1; > } Has no functional effect, since !l->initialized will always be true. > diff --git i/src/util/virthread.h w/src/util/virthread.h > index e466d9bf0184..1ba8a0fe46bb 100644 > --- i/src/util/virthread.h > +++ w/src/util/virthread.h > @@ -54,6 +54,7 @@ typedef virThreadLocal *virThreadLocalPtr; > > struct virThreadLocal { > pthread_key_t key; > + bool initialized; > }; > > typedef struct virThread virThread; Regards, 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list