Re: [PATCH 03/24] maint: move debug statements first in public API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>>
>> Something tells me there's some "historical reason" for the VIR_DEBUG()
>> after virInitialize(). Something that perhaps some assumption that
>> virLogMessage() or what it calls may assume has been initialized properly...
> 
> Indeed - VIR_DEBUG has the potential for different behavior before and
> after virInitialize(); and if you are trying to log behavior of a
> client, getting the connection open details logged is important.
> 

>>
>> ACK - as long as you're satisfied that calling VIR_DEBUG before
>> virInitialize is ok
> 
> I'll respin this one to keep virInitialize() first.  The v2 series will
> be shorter, though, once I push the non-controversial patches from this
> review (thanks for tackling it, by the way).

Actually, after sleeping on it, I decided to just squash this in and push:

diff --git i/src/libvirt.c w/src/libvirt.c
index b197d96..e8ecbf9 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -348,6 +348,11 @@ static virOnceControl virGlobalOnce =
VIR_ONCE_CONTROL_INITIALIZER;
 static void
 virGlobalInit(void)
 {
+    /* It would be nice if we could trace the use of this call, to
+     * help diagnose in log files if a user calls something other than
+     * virConnectOpen first.  But we can't rely on VIR_DEBUG working
+     * until after initialization is complete, and since this is
+     * one-shot, we never get here again.  */
     if (virThreadInitialize() < 0 ||
         virErrorInitialize() < 0)
         goto error;
@@ -455,13 +460,18 @@ error:
  *
  * Initialize the library.
  *
- * This method is invoked automatically by any of the virConnectOpen API
- * calls. Since release 1.0.0, there is no need to call this method even
- * in a multithreaded application, since initialization is performed in
- * a thread safe manner.
+ * This method is invoked automatically by any of the virConnectOpen() API
+ * calls, and by virGetVersion(). Since release 1.0.0, there is no need to
+ * call this method even in a multithreaded application, since
+ * initialization is performed in a thread safe manner; but applications
+ * using an older version of the library should manually call this before
+ * setting up competing threads that attempt virConnectOpen in parallel.
  *
- * The only time it would be necessary to call virInitialize is if the
- * application did not invoke virConnectOpen as its first API call.
+ * The only other time it would be necessary to call virInitialize is
if the
+ * application did not invoke virConnectOpen as its first API call, such
+ * as when calling virEventRegisterImpl() before setting up connections,
+ * or when using virSetErrorFunc() to alter error reporting of the first
+ * connection attempt.
  *
  * Returns 0 in case of success, -1 in case of error
  */
@@ -904,7 +914,9 @@ virStateStop(void)
  * but due to the design of remote clients this is not reliable). To
  * get the version of the running hypervisor use the virConnectGetVersion
  * function instead. To get the libvirt library version used by a
- * connection use the virConnectGetLibVersion instead.
+ * connection use the virConnectGetLibVersion() instead.
+ *
+ * This function includes a call to virInitialize() when necessary.
  *
  * Returns -1 in case of failure, 0 otherwise, and values for @libVer and
  *       @typeVer have the format major * 1,000,000 + minor * 1,000 +
release.
@@ -913,10 +925,9 @@ int
 virGetVersion(unsigned long *libVer, const char *type ATTRIBUTE_UNUSED,
               unsigned long *typeVer)
 {
-    VIR_DEBUG("libVir=%p, type=%s, typeVer=%p", libVer, type, typeVer);
-
     if (virInitialize() < 0)
         goto error;
+    VIR_DEBUG("libVir=%p, type=%s, typeVer=%p", libVer, type, typeVer);

     if (libVer == NULL)
         goto error;
@@ -1367,10 +1378,9 @@ virConnectOpen(const char *name)
 {
     virConnectPtr ret = NULL;

-    VIR_DEBUG("name=%s", NULLSTR(name));
-
     if (virInitialize() < 0)
         goto error;
+    VIR_DEBUG("name=%s", NULLSTR(name));

     virResetLastError();
     ret = do_open(name, NULL, 0);
@@ -1404,10 +1414,9 @@ virConnectOpenReadOnly(const char *name)
 {
     virConnectPtr ret = NULL;

-    VIR_DEBUG("name=%s", NULLSTR(name));
-
     if (virInitialize() < 0)
         goto error;
+    VIR_DEBUG("name=%s", NULLSTR(name));

     virResetLastError();
     ret = do_open(name, NULL, VIR_CONNECT_RO);
@@ -1445,10 +1454,9 @@ virConnectOpenAuth(const char *name,
 {
     virConnectPtr ret = NULL;

-    VIR_DEBUG("name=%s, auth=%p, flags=%x", NULLSTR(name), auth, flags);
-
     if (virInitialize() < 0)
         goto error;
+    VIR_DEBUG("name=%s, auth=%p, flags=%x", NULLSTR(name), auth, flags);

     virResetLastError();
     ret = do_open(name, auth, flags);


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]