[libvirt] PATCH: 9/25: Make global error object thread local

[Jim Meyering]

"Daniel P. Berrange" <berrange at redhat.com> wrote:
> The virGetLastError() and virConnGetLastError() methods are not
> even remotely thread safe, and the virCopyLastError/virConnCopyLastError
> methods don't help in this goal, being open to a race condition.
>
> This patch changes the internal impl of the global error object to
> store its virError instance in a thread local variable. All errors
> are now reported against the global error object. In the public
> API entry points, we explicitly reset the global error object to
> ensure no stale errors are hanging around. In all error paths we
> also set a generic error, if the internal driver forget to set an
> explicit error. Finally we also copy the global error to the per
> connection error object for back-compatability, though the global
> object remains non-threadsafe for application access.


ACK.
I looked through all of it, though confess I paid less
attention to detail as the scope of the repetition began to set in.
The code of many of those function bodies should be automatically generated.
Unfortunately, it's just a little too complicated to do with cpp...

What I mean, precisely is that there are 50-60 blocks that
fit this general mold:

+    if (conn->driver->domainSetMaxMemory) {
+        int ret;
+        ret = conn->driver->domainSetMaxMemory (domain, memory);
+        if (ret < 0)
+            goto error;
+        return ret;
+    }

Where all that varies is the method name (domainSetMaxMemory, here),
its argument list, and the type of "ret", which matches the type
of the containing function.  Oh, and of course the condition changes
with the type, and sometimes with the semantics of the method.

-----------------------------------------
However, I did notice one tiny problem:
There are many uses of TODO, like this:

  -    if (domain == NULL) {
  -        TODO
  -        return (-1);
  -    }
  +    virResetLastError();

Obviously not a compile-time problem, since it's defined to nothing,
but with no following semicolon, it would cause automatic indenting
tools to do the wrong thing.

Four uses remain after your patch series.