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

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

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

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