On Fri, Apr 17, 2015 at 11:16:49AM +0100, Daniel P. Berrange wrote:
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.
Yes, I wanted to dispatch errors on connections and just haven't realized it's not needed. Dropping two patches and changing four lines fixed this and it Just Works.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list