Re: [libvirt] PATCH: 9/25: Make global error object thread local
"Daniel P. Berrange" <berrange(a)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.