Re: [libvirt] [PATCH 02/15] util: Add virabstracts file for keeping abstract classes

On Thu, Apr 16, 2015 at 04:46:37PM +0200, Martin Kletzander wrote: > The first class in this file is going to be an abstract connection class > that holds a per-connection error inside. virConnect will be the first > child class inheriting from this one. > > This is a separate file because virerror.c is going to depend on it and > putting it into datatypes along with other connect classes would create > a dependency on datatypes from the util library. So I can understand why you are doing this - you'll have the admin connection also inherit from this later, but.... > +struct _virAbstractConnect { > + virObjectLockable parent; > + > + /* > + * Object lock must be acquired before accessing/changing any of > + * members following this point, or changing the ref count of any > + * virDomain/virNetwork object associated with this connection. > + */ > + > + /* Per-connection error. */ > + virError err; /* the last error */ > + virErrorFunc handler; /* associated handlet */ > + void *userData; /* the user data */ These fields are really legacy stuff that we no longer encourage the use of. These dated from before the time that we have thread-local error objects, and they cannot ever be safely accessed when using the virConnect in a multi-threaded context. So, if we're creating a new admin connection object, I'd really suggest we don't want to have these connection level error objects. Just mandate use of the thread-local errors for the admin connection IIUC, removing these error objects, would really kill the need for this virAbstractConnect class, as the admin connection could just inherit from virObjectLockable.