[libvirt] [PATCH] virt-admin: Call virInitialize to fix startup crash

Martin Kletzander mkletzan at redhat.com
Tue Jun 28 08:49:33 UTC 2016


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 */
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) {
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;
     }
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;
--

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160628/9539273d/attachment-0001.sig>


More information about the libvir-list mailing list