• Subject: [libvirt] [PATCH 0/9] virClassNew rework
  • Author: Michal Privoznik
  • Date: April 13, 2018, 2:47 p.m.
  • Patches: 9 / 9
Changeset
cfg.mk                                  |   8 ++
src/access/viraccessmanager.c           |   6 +-
src/bhyve/bhyve_conf.c                  |   6 +-
src/conf/capabilities.c                 |  10 +--
src/conf/domain_capabilities.c          |  12 +--
src/conf/domain_conf.c                  |  22 ++----
src/conf/domain_event.c                 | 130 +++++++++++---------------------
src/conf/network_event.c                |  12 +--
src/conf/node_device_event.c            |  18 ++---
src/conf/object_event.c                 |  12 +--
src/conf/secret_event.c                 |  18 ++---
src/conf/storage_event.c                |  18 ++---
src/conf/virdomainobjlist.c             |   6 +-
src/conf/virinterfaceobj.c              |  12 +--
src/conf/virnetworkobj.c                |  12 +--
src/conf/virnodedeviceobj.c             |  14 ++--
src/conf/virsecretobj.c                 |  12 +--
src/conf/virstorageobj.c                |  24 ++----
src/datatypes.c                         |   8 +-
src/datatypes.h                         |  30 ++++----
src/interface/interface_backend_netcf.c |   6 +-
src/libvirt-admin.c                     |   8 +-
src/libvirt-domain-snapshot.c           |   2 +-
src/libvirt-domain.c                    |   2 +-
src/libvirt-host.c                      |   2 +-
src/libvirt-interface.c                 |   2 +-
src/libvirt-network.c                   |   2 +-
src/libvirt-nodedev.c                   |   8 +-
src/libvirt-nwfilter.c                  |   2 +-
src/libvirt-secret.c                    |   2 +-
src/libvirt-storage.c                   |   4 +-
src/libvirt-stream.c                    |   2 +-
src/libxl/libxl_conf.c                  |   6 +-
src/libxl/libxl_domain.c                |   6 +-
src/libxl/libxl_migration.c             |   6 +-
src/logging/log_handler.c               |   6 +-
src/lxc/lxc_conf.c                      |   6 +-
src/lxc/lxc_monitor.c                   |   6 +-
src/node_device/node_device_driver.c    |   4 +-
src/node_device/node_device_udev.c      |   6 +-
src/qemu/qemu_agent.c                   |   6 +-
src/qemu/qemu_capabilities.c            |   8 +-
src/qemu/qemu_conf.c                    |   6 +-
src/qemu/qemu_domain.c                  |  36 +++------
src/qemu/qemu_monitor.c                 |   6 +-
src/rpc/virkeepalive.c                  |   6 +-
src/rpc/virnetclient.c                  |   6 +-
src/rpc/virnetclientprogram.c           |   8 +-
src/rpc/virnetclientstream.c            |   6 +-
src/rpc/virnetdaemon.c                  |   6 +-
src/rpc/virnetlibsshsession.c           |   6 +-
src/rpc/virnetsaslcontext.c             |  18 +++--
src/rpc/virnetserver.c                  |   6 +-
src/rpc/virnetserverclient.c            |   6 +-
src/rpc/virnetserverprogram.c           |   8 +-
src/rpc/virnetserverservice.c           |   8 +-
src/rpc/virnetsocket.c                  |   6 +-
src/rpc/virnetsshsession.c              |   6 +-
src/rpc/virnettlscontext.c              |  12 +--
src/security/security_manager.c         |   6 +-
src/test/test_driver.c                  |   6 +-
src/util/virclosecallbacks.c            |   6 +-
src/util/virdnsmasq.c                   |   9 +--
src/util/virfdstream.c                  |  10 +--
src/util/virfilecache.c                 |   8 +-
src/util/virhash.c                      |   6 +-
src/util/virhostdev.c                   |   6 +-
src/util/viridentity.c                  |   6 +-
src/util/virmacmap.c                    |   8 +-
src/util/virmacmap.h                    |   2 +-
src/util/virmdev.c                      |   6 +-
src/util/virobject.c                    |  43 ++++++-----
src/util/virobject.h                    |   7 ++
src/util/virpci.c                       |   6 +-
src/util/virportallocator.c             |   6 +-
src/util/virresctrl.c                   |  12 +--
src/util/virscsi.c                      |   6 +-
src/util/virscsivhost.c                 |   6 +-
src/util/virusb.c                       |   6 +-
src/vbox/vbox_common.c                  |   6 +-
src/vz/vz_driver.c                      |   6 +-
tests/virfilecachetest.c                |   8 +-
82 files changed, 322 insertions(+), 517 deletions(-)
Git apply log
Switched to a new branch 'cover.1523630758.git.mprivozn@redhat.com'
Applying: domain_event: s/MetadataCange/MetadataChange/g
Using index info to reconstruct a base tree...
M	src/conf/domain_event.c
Falling back to patching base and 3-way merge...
No changes -- Patch already applied.
Applying: util: Make structs follow our naming convention
Applying: datatypes: Rename @parent to @parentName in virNodeDevice
Applying: src: Unify virObject member name
Applying: src: Unify dispose function names
Using index info to reconstruct a base tree...
M	src/conf/capabilities.c
M	src/conf/domain_conf.c
Falling back to patching base and 3-way merge...
No changes -- Patch already applied.
Applying: Introduce virNetSASLContextDispose
Using index info to reconstruct a base tree...
M	src/rpc/virnetsaslcontext.c
Falling back to patching base and 3-way merge...
Auto-merging src/rpc/virnetsaslcontext.c
CONFLICT (content): Merge conflict in src/rpc/virnetsaslcontext.c
error: Failed to merge in the changes.
Patch failed at 0001 Introduce virNetSASLContextDispose
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Failed to apply patch:
[libvirt] [PATCH 6/9] Introduce virNetSASLContextDispose
Test passed: syntax-check

loading

[libvirt] [PATCH 0/9] virClassNew rework
Posted by Michal Privoznik, 14 weeks ago
This is inspired by commit of f574e2e5214fb9. Also, the more the code is
unified the easier it is to maintain.

Michal Privoznik (9):
  domain_event: s/MetadataCange/MetadataChange/g
  util: Make structs follow our naming convention
  datatypes: Rename @parent to @parentName in virNodeDevice
  src: Unify virObject member name
  src: Unify dispose function names
  Introduce virNetSASLContextDispose
  virobject: Introduce VIR_CLASS_NEW() macro
  virobject: Check if @parent is the first member in class
  cfg.mk: Introduce syntax-check rule to prefer VIR_CLASS_NEW

 cfg.mk                                  |   8 ++
 src/access/viraccessmanager.c           |   6 +-
 src/bhyve/bhyve_conf.c                  |   6 +-
 src/conf/capabilities.c                 |  10 +--
 src/conf/domain_capabilities.c          |  12 +--
 src/conf/domain_conf.c                  |  22 ++----
 src/conf/domain_event.c                 | 130 +++++++++++---------------------
 src/conf/network_event.c                |  12 +--
 src/conf/node_device_event.c            |  18 ++---
 src/conf/object_event.c                 |  12 +--
 src/conf/secret_event.c                 |  18 ++---
 src/conf/storage_event.c                |  18 ++---
 src/conf/virdomainobjlist.c             |   6 +-
 src/conf/virinterfaceobj.c              |  12 +--
 src/conf/virnetworkobj.c                |  12 +--
 src/conf/virnodedeviceobj.c             |  14 ++--
 src/conf/virsecretobj.c                 |  12 +--
 src/conf/virstorageobj.c                |  24 ++----
 src/datatypes.c                         |   8 +-
 src/datatypes.h                         |  30 ++++----
 src/interface/interface_backend_netcf.c |   6 +-
 src/libvirt-admin.c                     |   8 +-
 src/libvirt-domain-snapshot.c           |   2 +-
 src/libvirt-domain.c                    |   2 +-
 src/libvirt-host.c                      |   2 +-
 src/libvirt-interface.c                 |   2 +-
 src/libvirt-network.c                   |   2 +-
 src/libvirt-nodedev.c                   |   8 +-
 src/libvirt-nwfilter.c                  |   2 +-
 src/libvirt-secret.c                    |   2 +-
 src/libvirt-storage.c                   |   4 +-
 src/libvirt-stream.c                    |   2 +-
 src/libxl/libxl_conf.c                  |   6 +-
 src/libxl/libxl_domain.c                |   6 +-
 src/libxl/libxl_migration.c             |   6 +-
 src/logging/log_handler.c               |   6 +-
 src/lxc/lxc_conf.c                      |   6 +-
 src/lxc/lxc_monitor.c                   |   6 +-
 src/node_device/node_device_driver.c    |   4 +-
 src/node_device/node_device_udev.c      |   6 +-
 src/qemu/qemu_agent.c                   |   6 +-
 src/qemu/qemu_capabilities.c            |   8 +-
 src/qemu/qemu_conf.c                    |   6 +-
 src/qemu/qemu_domain.c                  |  36 +++------
 src/qemu/qemu_monitor.c                 |   6 +-
 src/rpc/virkeepalive.c                  |   6 +-
 src/rpc/virnetclient.c                  |   6 +-
 src/rpc/virnetclientprogram.c           |   8 +-
 src/rpc/virnetclientstream.c            |   6 +-
 src/rpc/virnetdaemon.c                  |   6 +-
 src/rpc/virnetlibsshsession.c           |   6 +-
 src/rpc/virnetsaslcontext.c             |  18 +++--
 src/rpc/virnetserver.c                  |   6 +-
 src/rpc/virnetserverclient.c            |   6 +-
 src/rpc/virnetserverprogram.c           |   8 +-
 src/rpc/virnetserverservice.c           |   8 +-
 src/rpc/virnetsocket.c                  |   6 +-
 src/rpc/virnetsshsession.c              |   6 +-
 src/rpc/virnettlscontext.c              |  12 +--
 src/security/security_manager.c         |   6 +-
 src/test/test_driver.c                  |   6 +-
 src/util/virclosecallbacks.c            |   6 +-
 src/util/virdnsmasq.c                   |   9 +--
 src/util/virfdstream.c                  |  10 +--
 src/util/virfilecache.c                 |   8 +-
 src/util/virhash.c                      |   6 +-
 src/util/virhostdev.c                   |   6 +-
 src/util/viridentity.c                  |   6 +-
 src/util/virmacmap.c                    |   8 +-
 src/util/virmacmap.h                    |   2 +-
 src/util/virmdev.c                      |   6 +-
 src/util/virobject.c                    |  43 ++++++-----
 src/util/virobject.h                    |   7 ++
 src/util/virpci.c                       |   6 +-
 src/util/virportallocator.c             |   6 +-
 src/util/virresctrl.c                   |  12 +--
 src/util/virscsi.c                      |   6 +-
 src/util/virscsivhost.c                 |   6 +-
 src/util/virusb.c                       |   6 +-
 src/vbox/vbox_common.c                  |   6 +-
 src/vz/vz_driver.c                      |   6 +-
 tests/virfilecachetest.c                |   8 +-
 82 files changed, 322 insertions(+), 517 deletions(-)

-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/9] domain_event: s/MetadataCange/MetadataChange/g
Posted by Michal Privoznik, 14 weeks ago
There's a typo in struct name.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/conf/domain_event.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 7baccd5b57..fdb48a1eaa 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -270,13 +270,13 @@ struct _virDomainEventDeviceRemovalFailed {
 typedef struct _virDomainEventDeviceRemovalFailed virDomainEventDeviceRemovalFailed;
 typedef virDomainEventDeviceRemovalFailed *virDomainEventDeviceRemovalFailedPtr;
 
-struct _virDomainEventMetadataCange {
+struct _virDomainEventMetadataChange {
     virDomainEvent parent;
 
     int type;
     char *nsuri;
 };
-typedef struct _virDomainEventMetadataCange virDomainEventMetadataChange;
+typedef struct _virDomainEventMetadataChange virDomainEventMetadataChange;
 typedef virDomainEventMetadataChange *virDomainEventMetadataChangePtr;
 
 struct _virDomainEventBlockThreshold {
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/9] domain_event: s/MetadataCange/MetadataChange/g
Posted by Erik Skultety, 13 weeks ago
On Fri, Apr 13, 2018 at 04:47:08PM +0200, Michal Privoznik wrote:
> There's a typo in struct name.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/conf/domain_event.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/9] util: Make structs follow our naming convention
Posted by Michal Privoznik, 14 weeks ago
There are two structs virMacMap and virFDStreamData that don't
have the underscore prefix. Put it there so that they follow the
rest of the code.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/virfdstream.c | 4 ++--
 src/util/virmacmap.c   | 2 +-
 src/util/virmacmap.h   | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index be40379a92..e2d3f365cd 100644
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -75,9 +75,9 @@ struct _virFDStreamMsg {
 
 
 /* Tunnelled migration stream support */
-typedef struct virFDStreamData virFDStreamData;
+typedef struct _virFDStreamData virFDStreamData;
 typedef virFDStreamData *virFDStreamDataPtr;
-struct virFDStreamData {
+struct _virFDStreamData {
     virObjectLockable parent;
 
     int fd;
diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c
index 42890ba2ff..d3be3066cc 100644
--- a/src/util/virmacmap.c
+++ b/src/util/virmacmap.c
@@ -43,7 +43,7 @@ VIR_LOG_INIT("util.virmacmap");
  */
 #define VIR_MAC_MAP_FILE_SIZE_MAX (32 * 1024 * 1024)
 
-struct virMacMap {
+struct _virMacMap {
     virObjectLockable parent;
 
     virHashTablePtr macs;
diff --git a/src/util/virmacmap.h b/src/util/virmacmap.h
index e6f754e247..93295c9107 100644
--- a/src/util/virmacmap.h
+++ b/src/util/virmacmap.h
@@ -24,7 +24,7 @@
 #ifndef __VIR_MACMAP_H__
 # define __VIR_MACMAP_H__
 
-typedef struct virMacMap virMacMap;
+typedef struct _virMacMap virMacMap;
 typedef virMacMap *virMacMapPtr;
 
 char *
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/9] util: Make structs follow our naming convention
Posted by Erik Skultety, 13 weeks ago
On Fri, Apr 13, 2018 at 04:47:09PM +0200, Michal Privoznik wrote:
> There are two structs virMacMap and virFDStreamData that don't
> have the underscore prefix. Put it there so that they follow the
> rest of the code.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/util/virfdstream.c | 4 ++--
>  src/util/virmacmap.c   | 2 +-
>  src/util/virmacmap.h   | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)

these suffer from the same problem:
daemonAdmClientPrivate
virDomainQemuMonitorEventData
xentoollog_logger_libvirt (no convention here at all, so I'd skip this one)
virLXCMeminfo
qemuBlockNodeNameBackingChainData
daemonClientStream
virNetMessageHeader
virNetMessageError
virCgroup
virNetlinkCallbackData
virPerf
virPerfEvent
virPerfEventAttr
virRotatingFileWriterEntry
virRotatingFileReaderEntry
virRotatingFileWriter
virRotatingFileReader
virMutex
virRWLock
virCond
virThreadLocal
virThread
virTypedParameterRemoteValue (the second typedef is completely wrong):
    typedef struct _virTypedParameterRemoteValue virTypedParameterRemoteValue;
    typedef struct virTypedParameterRemoteValue *virTypedParameterRemoteValuePtr;
virOnceControl
vbox - basically all of the structs :P
vzDomObj

Honestly, given the number of places where this should be fixed, I'm not sure
whether we should really go with the patch, but at the same time, I can imagine
us having this unified once and for all.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/9] util: Make structs follow our naming convention
Posted by Michal Privoznik, 13 weeks ago
On 04/16/2018 09:55 AM, Erik Skultety wrote:
> On Fri, Apr 13, 2018 at 04:47:09PM +0200, Michal Privoznik wrote:
>> There are two structs virMacMap and virFDStreamData that don't
>> have the underscore prefix. Put it there so that they follow the
>> rest of the code.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/util/virfdstream.c | 4 ++--
>>  src/util/virmacmap.c   | 2 +-
>>  src/util/virmacmap.h   | 2 +-
>>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> these suffer from the same problem:
> daemonAdmClientPrivate
> virDomainQemuMonitorEventData
> xentoollog_logger_libvirt (no convention here at all, so I'd skip this one)
> virLXCMeminfo
> qemuBlockNodeNameBackingChainData
> daemonClientStream
> virNetMessageHeader
> virNetMessageError
> virCgroup
> virNetlinkCallbackData
> virPerf
> virPerfEvent
> virPerfEventAttr
> virRotatingFileWriterEntry
> virRotatingFileReaderEntry
> virRotatingFileWriter
> virRotatingFileReader
> virMutex
> virRWLock
> virCond
> virThreadLocal
> virThread
> virTypedParameterRemoteValue (the second typedef is completely wrong):
>     typedef struct _virTypedParameterRemoteValue virTypedParameterRemoteValue;
>     typedef struct virTypedParameterRemoteValue *virTypedParameterRemoteValuePtr;
> virOnceControl
> vbox - basically all of the structs :P
> vzDomObj
> 
> Honestly, given the number of places where this should be fixed, I'm not sure
> whether we should really go with the patch, but at the same time, I can imagine
> us having this unified once and for all.

Actually, I don't need this patch. So I'm dropping it.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/9] datatypes: Rename @parent to @parentName in virNodeDevice
Posted by Michal Privoznik, 14 weeks ago
In next patches this name will be needed for a different memeber.
Also, it makes sense to rename the variable because it does not
contain reference to parent device, just its name.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/conf/virnodedeviceobj.c          | 2 +-
 src/datatypes.c                      | 2 +-
 src/datatypes.h                      | 2 +-
 src/libvirt-nodedev.c                | 6 +++---
 src/node_device/node_device_driver.c | 4 ++--
 src/test/test_driver.c               | 6 +++---
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index ad0f27ee47..9d2996046f 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -870,7 +870,7 @@ virNodeDeviceObjListExportCallback(void *payload,
         virNodeDeviceMatch(obj, data->flags)) {
         if (data->devices) {
             if (!(device = virGetNodeDevice(data->conn, def->name)) ||
-                VIR_STRDUP(device->parent, def->parent) < 0) {
+                VIR_STRDUP(device->parentName, def->parent) < 0) {
                 virObjectUnref(device);
                 data->error = true;
                 goto cleanup;
diff --git a/src/datatypes.c b/src/datatypes.c
index f7eef24ba8..0c3c66a9ce 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -653,7 +653,7 @@ virNodeDeviceDispose(void *obj)
     VIR_DEBUG("release dev %p %s", dev, dev->name);
 
     VIR_FREE(dev->name);
-    VIR_FREE(dev->parent);
+    VIR_FREE(dev->parentName);
 
     virObjectUnref(dev->conn);
 }
diff --git a/src/datatypes.h b/src/datatypes.h
index 1a8ea01ba3..66733b075c 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -618,7 +618,7 @@ struct _virNodeDevice {
     virObject object;
     virConnectPtr conn;                 /* pointer back to the connection */
     char *name;                         /* device name (unique on node) */
-    char *parent;                       /* parent device name */
+    char *parentName;                   /* parent device name */
 };
 
 /**
diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index 563ce889b9..8ced3cea0e 100644
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -346,16 +346,16 @@ virNodeDeviceGetParent(virNodeDevicePtr dev)
 
     virCheckNodeDeviceReturn(dev, NULL);
 
-    if (!dev->parent) {
+    if (!dev->parentName) {
         if (dev->conn->nodeDeviceDriver && dev->conn->nodeDeviceDriver->nodeDeviceGetParent) {
-            dev->parent = dev->conn->nodeDeviceDriver->nodeDeviceGetParent(dev);
+            dev->parentName = dev->conn->nodeDeviceDriver->nodeDeviceGetParent(dev);
         } else {
             virReportUnsupportedError();
             virDispatchError(dev->conn);
             return NULL;
         }
     }
-    return dev->parent;
+    return dev->parentName;
 }
 
 
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 61afa1f8eb..d04a31747a 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -256,7 +256,7 @@ nodeDeviceLookupByName(virConnectPtr conn,
         goto cleanup;
 
     if ((device = virGetNodeDevice(conn, name))) {
-        if (VIR_STRDUP(device->parent, def->parent) < 0) {
+        if (VIR_STRDUP(device->parentName, def->parent) < 0) {
             virObjectUnref(device);
             device = NULL;
         }
@@ -290,7 +290,7 @@ nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn,
         goto cleanup;
 
     if ((device = virGetNodeDevice(conn, def->name))) {
-        if (VIR_STRDUP(device->parent, def->parent) < 0) {
+        if (VIR_STRDUP(device->parentName, def->parent) < 0) {
             virObjectUnref(device);
             device = NULL;
         }
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index eec7a82924..f5b5e3ee8d 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5416,7 +5416,7 @@ testNodeDeviceLookupByName(virConnectPtr conn, const char *name)
     def = virNodeDeviceObjGetDef(obj);
 
     if ((ret = virGetNodeDevice(conn, name))) {
-        if (VIR_STRDUP(ret->parent, def->parent) < 0) {
+        if (VIR_STRDUP(ret->parentName, def->parent) < 0) {
             virObjectUnref(ret);
             ret = NULL;
         }
@@ -5641,8 +5641,8 @@ testNodeDeviceCreateXML(virConnectPtr conn,
     if (!(dev = virGetNodeDevice(conn, objdef->name)))
         goto cleanup;
 
-    VIR_FREE(dev->parent);
-    if (VIR_STRDUP(dev->parent, def->parent) < 0)
+    VIR_FREE(dev->parentName);
+    if (VIR_STRDUP(dev->parentName, def->parent) < 0)
         goto cleanup;
 
     ret = dev;
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/9] datatypes: Rename @parent to @parentName in virNodeDevice
Posted by Erik Skultety, 13 weeks ago
On Fri, Apr 13, 2018 at 04:47:10PM +0200, Michal Privoznik wrote:
> In next patches this name will be needed for a different memeber.
> Also, it makes sense to rename the variable because it does not
> contain reference to parent device, just its name.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/conf/virnodedeviceobj.c          | 2 +-
>  src/datatypes.c                      | 2 +-
>  src/datatypes.h                      | 2 +-
>  src/libvirt-nodedev.c                | 6 +++---
>  src/node_device/node_device_driver.c | 4 ++--
>  src/test/test_driver.c               | 6 +++---
>  6 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
> index ad0f27ee47..9d2996046f 100644
> --- a/src/conf/virnodedeviceobj.c
> +++ b/src/conf/virnodedeviceobj.c
> @@ -870,7 +870,7 @@ virNodeDeviceObjListExportCallback(void *payload,
>          virNodeDeviceMatch(obj, data->flags)) {
>          if (data->devices) {
>              if (!(device = virGetNodeDevice(data->conn, def->name)) ||
> -                VIR_STRDUP(device->parent, def->parent) < 0) {
> +                VIR_STRDUP(device->parentName, def->parent) < 0) {
>                  virObjectUnref(device);
>                  data->error = true;
>                  goto cleanup;
> diff --git a/src/datatypes.c b/src/datatypes.c
> index f7eef24ba8..0c3c66a9ce 100644
> --- a/src/datatypes.c
> +++ b/src/datatypes.c
> @@ -653,7 +653,7 @@ virNodeDeviceDispose(void *obj)
>      VIR_DEBUG("release dev %p %s", dev, dev->name);
>
>      VIR_FREE(dev->name);
> -    VIR_FREE(dev->parent);
> +    VIR_FREE(dev->parentName);
>
>      virObjectUnref(dev->conn);
>  }
> diff --git a/src/datatypes.h b/src/datatypes.h
> index 1a8ea01ba3..66733b075c 100644
> --- a/src/datatypes.h
> +++ b/src/datatypes.h
> @@ -618,7 +618,7 @@ struct _virNodeDevice {
>      virObject object;
>      virConnectPtr conn;                 /* pointer back to the connection */
>      char *name;                         /* device name (unique on node) */
> -    char *parent;                       /* parent device name */
> +    char *parentName;                   /* parent device name */
>  };
>
>  /**
> diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
> index 563ce889b9..8ced3cea0e 100644
> --- a/src/libvirt-nodedev.c
> +++ b/src/libvirt-nodedev.c
> @@ -346,16 +346,16 @@ virNodeDeviceGetParent(virNodeDevicePtr dev)
>
>      virCheckNodeDeviceReturn(dev, NULL);
>
> -    if (!dev->parent) {
> +    if (!dev->parentName) {
>          if (dev->conn->nodeDeviceDriver && dev->conn->nodeDeviceDriver->nodeDeviceGetParent) {
> -            dev->parent = dev->conn->nodeDeviceDriver->nodeDeviceGetParent(dev);
> +            dev->parentName = dev->conn->nodeDeviceDriver->nodeDeviceGetParent(dev);

Since you're adjusting the struct member name, you could go as far as fixing
the *GetParent accessor's name too.

With that:
Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/9] datatypes: Rename @parent to @parentName in virNodeDevice
Posted by Michal Privoznik, 13 weeks ago
On 04/16/2018 09:26 AM, Erik Skultety wrote:
> On Fri, Apr 13, 2018 at 04:47:10PM +0200, Michal Privoznik wrote:
>> In next patches this name will be needed for a different memeber.
>> Also, it makes sense to rename the variable because it does not
>> contain reference to parent device, just its name.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/conf/virnodedeviceobj.c          | 2 +-
>>  src/datatypes.c                      | 2 +-
>>  src/datatypes.h                      | 2 +-
>>  src/libvirt-nodedev.c                | 6 +++---
>>  src/node_device/node_device_driver.c | 4 ++--
>>  src/test/test_driver.c               | 6 +++---
>>  6 files changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
>> index ad0f27ee47..9d2996046f 100644
>> --- a/src/conf/virnodedeviceobj.c
>> +++ b/src/conf/virnodedeviceobj.c
>> @@ -870,7 +870,7 @@ virNodeDeviceObjListExportCallback(void *payload,
>>          virNodeDeviceMatch(obj, data->flags)) {
>>          if (data->devices) {
>>              if (!(device = virGetNodeDevice(data->conn, def->name)) ||
>> -                VIR_STRDUP(device->parent, def->parent) < 0) {
>> +                VIR_STRDUP(device->parentName, def->parent) < 0) {
>>                  virObjectUnref(device);
>>                  data->error = true;
>>                  goto cleanup;
>> diff --git a/src/datatypes.c b/src/datatypes.c
>> index f7eef24ba8..0c3c66a9ce 100644
>> --- a/src/datatypes.c
>> +++ b/src/datatypes.c
>> @@ -653,7 +653,7 @@ virNodeDeviceDispose(void *obj)
>>      VIR_DEBUG("release dev %p %s", dev, dev->name);
>>
>>      VIR_FREE(dev->name);
>> -    VIR_FREE(dev->parent);
>> +    VIR_FREE(dev->parentName);
>>
>>      virObjectUnref(dev->conn);
>>  }
>> diff --git a/src/datatypes.h b/src/datatypes.h
>> index 1a8ea01ba3..66733b075c 100644
>> --- a/src/datatypes.h
>> +++ b/src/datatypes.h
>> @@ -618,7 +618,7 @@ struct _virNodeDevice {
>>      virObject object;
>>      virConnectPtr conn;                 /* pointer back to the connection */
>>      char *name;                         /* device name (unique on node) */
>> -    char *parent;                       /* parent device name */
>> +    char *parentName;                   /* parent device name */
>>  };
>>
>>  /**
>> diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
>> index 563ce889b9..8ced3cea0e 100644
>> --- a/src/libvirt-nodedev.c
>> +++ b/src/libvirt-nodedev.c
>> @@ -346,16 +346,16 @@ virNodeDeviceGetParent(virNodeDevicePtr dev)
>>
>>      virCheckNodeDeviceReturn(dev, NULL);
>>
>> -    if (!dev->parent) {
>> +    if (!dev->parentName) {
>>          if (dev->conn->nodeDeviceDriver && dev->conn->nodeDeviceDriver->nodeDeviceGetParent) {
>> -            dev->parent = dev->conn->nodeDeviceDriver->nodeDeviceGetParent(dev);
>> +            dev->parentName = dev->conn->nodeDeviceDriver->nodeDeviceGetParent(dev);
> 
> Since you're adjusting the struct member name, you could go as far as fixing
> the *GetParent accessor's name too.

I can't. That is a public API and as such it cannot change.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/9] datatypes: Rename @parent to @parentName in virNodeDevice
Posted by John Ferlan, 13 weeks ago

On 04/13/2018 10:47 AM, Michal Privoznik wrote:
> In next patches this name will be needed for a different memeber.

s/memeber/member

> Also, it makes sense to rename the variable because it does not
> contain reference to parent device, just its name.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/conf/virnodedeviceobj.c          | 2 +-
>  src/datatypes.c                      | 2 +-
>  src/datatypes.h                      | 2 +-
>  src/libvirt-nodedev.c                | 6 +++---
>  src/node_device/node_device_driver.c | 4 ++--
>  src/test/test_driver.c               | 6 +++---
>  6 files changed, 11 insertions(+), 11 deletions(-)
> 

[...]

FWIW: Not sure about changing the accessor's name too - that would mean
the API name doesn't match (virNodeDeviceGetParent which could lead to
other problems with automatic code generation).

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/9] datatypes: Rename @parent to @parentName in virNodeDevice
Posted by Erik Skultety, 13 weeks ago
On Mon, Apr 16, 2018 at 07:53:42AM -0400, John Ferlan wrote:
>
>
> On 04/13/2018 10:47 AM, Michal Privoznik wrote:
> > In next patches this name will be needed for a different memeber.
>
> s/memeber/member
>
> > Also, it makes sense to rename the variable because it does not
> > contain reference to parent device, just its name.
> >
> > Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> > ---
> >  src/conf/virnodedeviceobj.c          | 2 +-
> >  src/datatypes.c                      | 2 +-
> >  src/datatypes.h                      | 2 +-
> >  src/libvirt-nodedev.c                | 6 +++---
> >  src/node_device/node_device_driver.c | 4 ++--
> >  src/test/test_driver.c               | 6 +++---
> >  6 files changed, 11 insertions(+), 11 deletions(-)
> >
>
> [...]
>
> FWIW: Not sure about changing the accessor's name too - that would mean
> the API name doesn't match (virNodeDeviceGetParent which could lead to
> other problems with automatic code generation).

Oh, is that so?! I missed this fact, sorry for the noise then. Nevertheless,
it's sad we've such a mistake.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/9] datatypes: Rename @parent to @parentName in virNodeDevice
Posted by Michal Privoznik, 13 weeks ago
On 04/16/2018 01:53 PM, John Ferlan wrote:
> 
> 
> On 04/13/2018 10:47 AM, Michal Privoznik wrote:
>> In next patches this name will be needed for a different memeber.
> 
> s/memeber/member
> 
>> Also, it makes sense to rename the variable because it does not
>> contain reference to parent device, just its name.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/conf/virnodedeviceobj.c          | 2 +-
>>  src/datatypes.c                      | 2 +-
>>  src/datatypes.h                      | 2 +-
>>  src/libvirt-nodedev.c                | 6 +++---
>>  src/node_device/node_device_driver.c | 4 ++--
>>  src/test/test_driver.c               | 6 +++---
>>  6 files changed, 11 insertions(+), 11 deletions(-)
>>
> 
> [...]
> 
> FWIW: Not sure about changing the accessor's name too - that would mean
> the API name doesn't match (virNodeDeviceGetParent which could lead to
> other problems with automatic code generation).

Not sure what do you mean by code generation. But that reminds me I
forgot to rename variable on RPC protocol level :-)

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/9] src: Unify virObject member name
Posted by Michal Privoznik, 14 weeks ago
Whenever we declare a new object the first member of the struct
has to be virObject (or any other member of that family). Now, up
until now we did not care about the name of the struct member.
But lets unify it so that we can do some checks at compile time
later.

The unified name is 'parent'.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/datatypes.h               | 28 ++++++++++++++--------------
 src/libvirt-admin.c           |  2 +-
 src/libvirt-domain-snapshot.c |  2 +-
 src/libvirt-domain.c          |  2 +-
 src/libvirt-host.c            |  2 +-
 src/libvirt-interface.c       |  2 +-
 src/libvirt-network.c         |  2 +-
 src/libvirt-nodedev.c         |  2 +-
 src/libvirt-nwfilter.c        |  2 +-
 src/libvirt-secret.c          |  2 +-
 src/libvirt-storage.c         |  4 ++--
 src/libvirt-stream.c          |  2 +-
 src/qemu/qemu_capabilities.c  |  2 +-
 src/rpc/virnetclientprogram.c |  2 +-
 src/rpc/virnetserverprogram.c |  2 +-
 src/rpc/virnetserverservice.c |  2 +-
 src/util/virdnsmasq.c         |  2 +-
 src/util/virfilecache.c       |  2 +-
 tests/virfilecachetest.c      |  2 +-
 19 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/src/datatypes.h b/src/datatypes.h
index 66733b075c..192c86be80 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -453,7 +453,7 @@ struct _virAdmConnectCloseCallbackData {
  * Internal structure associated to a connection
  */
 struct _virConnect {
-    virObjectLockable object;
+    virObjectLockable parent;
 
     /* All the variables from here, until declared otherwise in one of
      * the following comments, are setup at time of connection open
@@ -496,7 +496,7 @@ struct _virConnect {
  * Internal structure associated to an admin connection
  */
 struct _virAdmConnect {
-    virObjectLockable object;
+    virObjectLockable parent;
     virURIPtr uri;
 
     void *privateData;
@@ -512,7 +512,7 @@ struct _virAdmConnect {
  * Internal structure associated to a daemon server
  */
 struct _virAdmServer {
-    virObject object;
+    virObject parent;
     virAdmConnectPtr conn;          /* pointer back to the admin connection */
     char *name;                     /* the server external name */
 };
@@ -523,7 +523,7 @@ struct _virAdmServer {
  * Internal structure associated to a client connected to daemon
  */
 struct _virAdmClient {
-    virObject object;
+    virObject parent;
     virAdmServerPtr srv;            /* pointer to the server client is
                                      * connected to, which also holds a
                                      * reference back to the admin connection
@@ -539,7 +539,7 @@ struct _virAdmClient {
 * Internal structure associated to a domain
 */
 struct _virDomain {
-    virObject object;
+    virObject parent;
     virConnectPtr conn;                  /* pointer back to the connection */
     char *name;                          /* the domain external name */
     int id;                              /* the domain ID */
@@ -552,7 +552,7 @@ struct _virDomain {
 * Internal structure associated to a domain
 */
 struct _virNetwork {
-    virObject object;
+    virObject parent;
     virConnectPtr conn;                  /* pointer back to the connection */
     char *name;                          /* the network external name */
     unsigned char uuid[VIR_UUID_BUFLEN]; /* the network unique identifier */
@@ -564,7 +564,7 @@ struct _virNetwork {
 * Internal structure associated to a physical host interface
 */
 struct _virInterface {
-    virObject object;
+    virObject parent;
     virConnectPtr conn;                  /* pointer back to the connection */
     char *name;                          /* the network external name */
     char *mac;                           /* the interface MAC address */
@@ -576,7 +576,7 @@ struct _virInterface {
 * Internal structure associated to a storage pool
 */
 struct _virStoragePool {
-    virObject object;
+    virObject parent;
     virConnectPtr conn;                  /* pointer back to the connection */
     char *name;                          /* the storage pool external name */
     unsigned char uuid[VIR_UUID_BUFLEN]; /* the storage pool unique identifier */
@@ -595,7 +595,7 @@ struct _virStoragePool {
 * Internal structure associated to a storage volume
 */
 struct _virStorageVol {
-    virObject object;
+    virObject parent;
     virConnectPtr conn;                  /* pointer back to the connection */
     char *pool;                          /* Pool name of owner */
     char *name;                          /* the storage vol external name */
@@ -615,7 +615,7 @@ struct _virStorageVol {
  * Internal structure associated with a node device
  */
 struct _virNodeDevice {
-    virObject object;
+    virObject parent;
     virConnectPtr conn;                 /* pointer back to the connection */
     char *name;                         /* device name (unique on node) */
     char *parentName;                   /* parent device name */
@@ -627,7 +627,7 @@ struct _virNodeDevice {
  * Internal structure associated with a secret
  */
 struct _virSecret {
-    virObject object;
+    virObject parent;
     virConnectPtr conn;                  /* pointer back to the connection */
     unsigned char uuid[VIR_UUID_BUFLEN]; /* the secret unique identifier */
     int usageType;                       /* the type of usage */
@@ -644,7 +644,7 @@ typedef int (*virStreamFinishFunc)(virStreamPtr, void *opaque);
  * Internal structure associated with an input stream
  */
 struct _virStream {
-    virObject object;
+    virObject parent;
     virConnectPtr conn;
     unsigned int flags;
 
@@ -658,7 +658,7 @@ struct _virStream {
  * Internal structure associated with a domain snapshot
  */
 struct _virDomainSnapshot {
-    virObject object;
+    virObject parent;
     char *name;
     virDomainPtr domain;
 };
@@ -669,7 +669,7 @@ struct _virDomainSnapshot {
 * Internal structure associated to a network filter
 */
 struct _virNWFilter {
-    virObject object;
+    virObject parent;
     virConnectPtr conn;                  /* pointer back to the connection */
     char *name;                          /* the network filter external name */
     unsigned char uuid[VIR_UUID_BUFLEN]; /* the network filter unique identifier */
diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
index de595a9f7f..7f695311b4 100644
--- a/src/libvirt-admin.c
+++ b/src/libvirt-admin.c
@@ -340,7 +340,7 @@ int
 virAdmConnectRef(virAdmConnectPtr conn)
 {
     VIR_DEBUG("conn=%p refs=%d", conn,
-              conn ? conn->object.parent.u.s.refs : 0);
+              conn ? conn->parent.parent.u.s.refs : 0);
 
     virResetLastError();
     virCheckAdmConnectReturn(conn, -1);
diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c
index e44a1d1f38..100326a5e7 100644
--- a/src/libvirt-domain-snapshot.c
+++ b/src/libvirt-domain-snapshot.c
@@ -1159,7 +1159,7 @@ int
 virDomainSnapshotRef(virDomainSnapshotPtr snapshot)
 {
     VIR_DEBUG("snapshot=%p, refs=%d", snapshot,
-              snapshot ? snapshot->object.u.s.refs : 0);
+              snapshot ? snapshot->parent.u.s.refs : 0);
 
     virResetLastError();
 
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 63d2ae23d7..2d86e48979 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -589,7 +589,7 @@ virDomainFree(virDomainPtr domain)
 int
 virDomainRef(virDomainPtr domain)
 {
-    VIR_DOMAIN_DEBUG(domain, "refs=%d", domain ? domain->object.u.s.refs : 0);
+    VIR_DOMAIN_DEBUG(domain, "refs=%d", domain ? domain->parent.u.s.refs : 0);
 
     virResetLastError();
 
diff --git a/src/libvirt-host.c b/src/libvirt-host.c
index 7ff7407a08..76087badd8 100644
--- a/src/libvirt-host.c
+++ b/src/libvirt-host.c
@@ -51,7 +51,7 @@ VIR_LOG_INIT("libvirt.host");
 int
 virConnectRef(virConnectPtr conn)
 {
-    VIR_DEBUG("conn=%p refs=%d", conn, conn ? conn->object.parent.u.s.refs : 0);
+    VIR_DEBUG("conn=%p refs=%d", conn, conn ? conn->parent.parent.u.s.refs : 0);
 
     virResetLastError();
 
diff --git a/src/libvirt-interface.c b/src/libvirt-interface.c
index bb79101abe..69415293ed 100644
--- a/src/libvirt-interface.c
+++ b/src/libvirt-interface.c
@@ -642,7 +642,7 @@ virInterfaceDestroy(virInterfacePtr iface, unsigned int flags)
 int
 virInterfaceRef(virInterfacePtr iface)
 {
-    VIR_DEBUG("iface=%p refs=%d", iface, iface ? iface->object.u.s.refs : 0);
+    VIR_DEBUG("iface=%p refs=%d", iface, iface ? iface->parent.u.s.refs : 0);
 
     virResetLastError();
 
diff --git a/src/libvirt-network.c b/src/libvirt-network.c
index da33543001..9f9e0ddaf8 100644
--- a/src/libvirt-network.c
+++ b/src/libvirt-network.c
@@ -679,7 +679,7 @@ int
 virNetworkRef(virNetworkPtr network)
 {
     VIR_DEBUG("network=%p refs=%d", network,
-              network ? network->object.u.s.refs : 0);
+              network ? network->parent.u.s.refs : 0);
 
     virResetLastError();
 
diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index 8ced3cea0e..10050b193b 100644
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -477,7 +477,7 @@ virNodeDeviceFree(virNodeDevicePtr dev)
 int
 virNodeDeviceRef(virNodeDevicePtr dev)
 {
-    VIR_DEBUG("dev=%p refs=%d", dev, dev ? dev->object.u.s.refs : 0);
+    VIR_DEBUG("dev=%p refs=%d", dev, dev ? dev->parent.u.s.refs : 0);
 
     virResetLastError();
 
diff --git a/src/libvirt-nwfilter.c b/src/libvirt-nwfilter.c
index 43e2b164e8..948c30deef 100644
--- a/src/libvirt-nwfilter.c
+++ b/src/libvirt-nwfilter.c
@@ -504,7 +504,7 @@ int
 virNWFilterRef(virNWFilterPtr nwfilter)
 {
     VIR_DEBUG("nwfilter=%p refs=%d", nwfilter,
-              nwfilter ? nwfilter->object.u.s.refs : 0);
+              nwfilter ? nwfilter->parent.u.s.refs : 0);
 
     virResetLastError();
 
diff --git a/src/libvirt-secret.c b/src/libvirt-secret.c
index 1e1cd4e65b..711c4fc580 100644
--- a/src/libvirt-secret.c
+++ b/src/libvirt-secret.c
@@ -659,7 +659,7 @@ int
 virSecretRef(virSecretPtr secret)
 {
     VIR_DEBUG("secret=%p refs=%d", secret,
-              secret ? secret->object.u.s.refs : 0);
+              secret ? secret->parent.u.s.refs : 0);
 
     virResetLastError();
 
diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
index 3845a5d55e..1879c6bd60 100644
--- a/src/libvirt-storage.c
+++ b/src/libvirt-storage.c
@@ -868,7 +868,7 @@ virStoragePoolFree(virStoragePoolPtr pool)
 int
 virStoragePoolRef(virStoragePoolPtr pool)
 {
-    VIR_DEBUG("pool=%p refs=%d", pool, pool ? pool->object.u.s.refs : 0);
+    VIR_DEBUG("pool=%p refs=%d", pool, pool ? pool->parent.u.s.refs : 0);
 
     virResetLastError();
 
@@ -1904,7 +1904,7 @@ virStorageVolFree(virStorageVolPtr vol)
 int
 virStorageVolRef(virStorageVolPtr vol)
 {
-    VIR_DEBUG("vol=%p refs=%d", vol, vol ? vol->object.u.s.refs : 0);
+    VIR_DEBUG("vol=%p refs=%d", vol, vol ? vol->parent.u.s.refs : 0);
 
     virResetLastError();
 
diff --git a/src/libvirt-stream.c b/src/libvirt-stream.c
index 3204b7b177..ef83696bcd 100644
--- a/src/libvirt-stream.c
+++ b/src/libvirt-stream.c
@@ -86,7 +86,7 @@ int
 virStreamRef(virStreamPtr stream)
 {
     VIR_DEBUG("stream=%p refs=%d", stream,
-              stream ? stream->object.u.s.refs : 0);
+              stream ? stream->parent.u.s.refs : 0);
 
     virResetLastError();
 
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c8488f875d..b5347b6cad 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -506,7 +506,7 @@ struct _virQEMUCapsHostCPUData {
  * And don't forget to update virQEMUCapsNewCopy.
  */
 struct _virQEMUCaps {
-    virObject object;
+    virObject parent;
 
     bool usedQMP;
 
diff --git a/src/rpc/virnetclientprogram.c b/src/rpc/virnetclientprogram.c
index 505b40fc4b..d1e54a0892 100644
--- a/src/rpc/virnetclientprogram.c
+++ b/src/rpc/virnetclientprogram.c
@@ -40,7 +40,7 @@
 VIR_LOG_INIT("rpc.netclientprogram");
 
 struct _virNetClientProgram {
-    virObject object;
+    virObject parent;
 
     unsigned program;
     unsigned version;
diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c
index 75b0052cdb..f28274fc22 100644
--- a/src/rpc/virnetserverprogram.c
+++ b/src/rpc/virnetserverprogram.c
@@ -37,7 +37,7 @@
 VIR_LOG_INIT("rpc.netserverprogram");
 
 struct _virNetServerProgram {
-    virObject object;
+    virObject parent;
 
     unsigned program;
     unsigned version;
diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index 4e5426ffea..d2350b2b91 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -34,7 +34,7 @@
 #define VIR_FROM_THIS VIR_FROM_RPC
 
 struct _virNetServerService {
-    virObject object;
+    virObject parent;
 
     size_t nsocks;
     virNetSocketPtr *socks;
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index 1b78c1fadc..db5c51b6a2 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -620,7 +620,7 @@ dnsmasqReload(pid_t pid ATTRIBUTE_UNUSED)
  *
  */
 struct _dnsmasqCaps {
-    virObject object;
+    virObject parent;
     char *binaryPath;
     bool noRefresh;
     time_t mtime;
diff --git a/src/util/virfilecache.c b/src/util/virfilecache.c
index 2577d711bc..e24eb5a68c 100644
--- a/src/util/virfilecache.c
+++ b/src/util/virfilecache.c
@@ -45,7 +45,7 @@ VIR_LOG_INIT("util.filecache")
 
 
 struct _virFileCache {
-    virObjectLockable object;
+    virObjectLockable parent;
 
     virHashTablePtr table;
 
diff --git a/tests/virfilecachetest.c b/tests/virfilecachetest.c
index ae7d08d257..d4a9d598ef 100644
--- a/tests/virfilecachetest.c
+++ b/tests/virfilecachetest.c
@@ -28,7 +28,7 @@
 
 
 struct _testFileCacheObj {
-    virObject object;
+    virObject parent;
     char *data;
 };
 typedef struct _testFileCacheObj testFileCacheObj;
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/9] src: Unify virObject member name
Posted by Erik Skultety, 13 weeks ago
On Fri, Apr 13, 2018 at 04:47:11PM +0200, Michal Privoznik wrote:
> Whenever we declare a new object the first member of the struct
> has to be virObject (or any other member of that family). Now, up
> until now we did not care about the name of the struct member.
> But lets unify it so that we can do some checks at compile time
> later.
>
> The unified name is 'parent'.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/9] src: Unify dispose function names
Posted by Michal Privoznik, 14 weeks ago
If a function is disposing virSomething it should be called
virSomethingDispose(). There are two offenders:
virCapabilitiesDispose(virCapsPtr) and
virDomainXMLOptionClassDispose(virDomainXMLOptionPtr).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/conf/capabilities.c | 6 +++---
 src/conf/domain_conf.c  | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index edf9f54f77..33b9194041 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -58,14 +58,14 @@ VIR_ENUM_IMPL(virCapsHostPMTarget, VIR_NODE_SUSPEND_TARGET_LAST,
               "suspend_mem", "suspend_disk", "suspend_hybrid");
 
 static virClassPtr virCapsClass;
-static void virCapabilitiesDispose(void *obj);
+static void virCapsDispose(void *obj);
 
 static int virCapabilitiesOnceInit(void)
 {
     if (!(virCapsClass = virClassNew(virClassForObject(),
                                      "virCaps",
                                      sizeof(virCaps),
-                                     virCapabilitiesDispose)))
+                                     virCapsDispose)))
         return -1;
 
     return 0;
@@ -215,7 +215,7 @@ virCapabilitiesClearSecModel(virCapsHostSecModelPtr secmodel)
 }
 
 static void
-virCapabilitiesDispose(void *object)
+virCapsDispose(void *object)
 {
     virCapsPtr caps = object;
     size_t i;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d23182f18a..4dad8e3b20 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -934,7 +934,7 @@ VIR_ENUM_IMPL(virDomainShmemModel, VIR_DOMAIN_SHMEM_MODEL_LAST,
 static virClassPtr virDomainObjClass;
 static virClassPtr virDomainXMLOptionClass;
 static void virDomainObjDispose(void *obj);
-static void virDomainXMLOptionClassDispose(void *obj);
+static void virDomainXMLOptionDispose(void *obj);
 
 static int virDomainObjOnceInit(void)
 {
@@ -947,7 +947,7 @@ static int virDomainObjOnceInit(void)
     if (!(virDomainXMLOptionClass = virClassNew(virClassForObject(),
                                                 "virDomainXMLOption",
                                                 sizeof(virDomainXMLOption),
-                                                virDomainXMLOptionClassDispose)))
+                                                virDomainXMLOptionDispose)))
         return -1;
 
     return 0;
@@ -957,7 +957,7 @@ VIR_ONCE_GLOBAL_INIT(virDomainObj)
 
 
 static void
-virDomainXMLOptionClassDispose(void *obj)
+virDomainXMLOptionDispose(void *obj)
 {
     virDomainXMLOptionPtr xmlopt = obj;
 
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/9] src: Unify dispose function names
Posted by Erik Skultety, 13 weeks ago
On Fri, Apr 13, 2018 at 04:47:12PM +0200, Michal Privoznik wrote:
> If a function is disposing virSomething it should be called
> virSomethingDispose(). There are two offenders:
> virCapabilitiesDispose(virCapsPtr) and
> virDomainXMLOptionClassDispose(virDomainXMLOptionPtr).
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---

An idea for a related patch: there are a few violators that put
"virSomethingClass" is the name of the class, while the general trend is to
omit "Class" from the name (it also doesn't make that much sense to have it
there).

Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/9] Introduce virNetSASLContextDispose
Posted by Michal Privoznik, 14 weeks ago
Strictly speaking this is not needed right now. However, next
commits will require dispose function to exist.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/rpc/virnetsaslcontext.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/rpc/virnetsaslcontext.c b/src/rpc/virnetsaslcontext.c
index a7b891feb6..f1f8bdc855 100644
--- a/src/rpc/virnetsaslcontext.c
+++ b/src/rpc/virnetsaslcontext.c
@@ -52,6 +52,7 @@ struct _virNetSASLSession {
 
 static virClassPtr virNetSASLContextClass;
 static virClassPtr virNetSASLSessionClass;
+static void virNetSASLContextDispose(void *obj);
 static void virNetSASLSessionDispose(void *obj);
 
 static int virNetSASLContextOnceInit(void)
@@ -59,7 +60,7 @@ static int virNetSASLContextOnceInit(void)
     if (!(virNetSASLContextClass = virClassNew(virClassForObjectLockable(),
                                                "virNetSASLContext",
                                                sizeof(virNetSASLContext),
-                                               NULL)))
+                                               virNetSASLContextDispose)))
         return -1;
 
     if (!(virNetSASLSessionClass = virClassNew(virClassForObjectLockable(),
@@ -686,6 +687,11 @@ ssize_t virNetSASLSessionDecode(virNetSASLSessionPtr sasl,
     return ret;
 }
 
+void virNetSASLContextDispose(void *obj ATTRIBUTE_UNUSED)
+{
+    /* nada */
+}
+
 void virNetSASLSessionDispose(void *obj)
 {
     virNetSASLSessionPtr sasl = obj;
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/9] Introduce virNetSASLContextDispose
Posted by Erik Skultety, 13 weeks ago
On Fri, Apr 13, 2018 at 04:47:13PM +0200, Michal Privoznik wrote:
> Strictly speaking this is not needed right now. However, next

The first sentence only makes sense in the context of the patch series review,
that will not be the case for someone looking at it in the future - plain
"Future commits rely on the presence of this callback" should suffice.

Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/9] Introduce virNetSASLContextDispose
Posted by Erik Skultety, 13 weeks ago
On Fri, Apr 13, 2018 at 04:47:13PM +0200, Michal Privoznik wrote:
> Strictly speaking this is not needed right now. However, next
> commits will require dispose function to exist.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---

...

>  src/rpc/virnetsaslcontext.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> +void virNetSASLContextDispose(void *obj ATTRIBUTE_UNUSED)
> +{
> +    /* nada */

One more thing, either leave the body empty, or replace the commentary with a
plain "return" call.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/9] virobject: Introduce VIR_CLASS_NEW() macro
Posted by Michal Privoznik, 14 weeks ago
So far we are repeating the following lines over and over:

  virClassNew(virClassForObject(),
              "virSomeObject",
              sizeof(virSomeObject),
              virSomeObjectDispose);

While this works, it is impossible to do some checking. Firstly,
the class name (the 2nd argument) doesn't match the name in the
code in all cases (the 3rd argument). Secondly, the current style
is needlessly verbose. This commit turns example into following:

  VIR_CLASS_NEW(virClassForObject(),
                virSomeObject);

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/access/viraccessmanager.c           |   6 +-
 src/bhyve/bhyve_conf.c                  |   6 +-
 src/conf/capabilities.c                 |   6 +-
 src/conf/domain_capabilities.c          |  12 +--
 src/conf/domain_conf.c                  |  18 ++---
 src/conf/domain_event.c                 | 126 +++++++++++---------------------
 src/conf/network_event.c                |  12 +--
 src/conf/node_device_event.c            |  18 ++---
 src/conf/object_event.c                 |  12 +--
 src/conf/secret_event.c                 |  18 ++---
 src/conf/storage_event.c                |  18 ++---
 src/conf/virdomainobjlist.c             |   6 +-
 src/conf/virinterfaceobj.c              |  12 +--
 src/conf/virnetworkobj.c                |  12 +--
 src/conf/virnodedeviceobj.c             |  12 +--
 src/conf/virsecretobj.c                 |  12 +--
 src/conf/virstorageobj.c                |  24 ++----
 src/datatypes.c                         |   6 +-
 src/interface/interface_backend_netcf.c |   6 +-
 src/libvirt-admin.c                     |   6 +-
 src/libxl/libxl_conf.c                  |   6 +-
 src/libxl/libxl_domain.c                |   6 +-
 src/libxl/libxl_migration.c             |   6 +-
 src/logging/log_handler.c               |   6 +-
 src/lxc/lxc_conf.c                      |   6 +-
 src/lxc/lxc_monitor.c                   |   6 +-
 src/node_device/node_device_udev.c      |   6 +-
 src/qemu/qemu_agent.c                   |   6 +-
 src/qemu/qemu_capabilities.c            |   6 +-
 src/qemu/qemu_conf.c                    |   6 +-
 src/qemu/qemu_domain.c                  |  36 +++------
 src/qemu/qemu_monitor.c                 |   6 +-
 src/rpc/virkeepalive.c                  |   6 +-
 src/rpc/virnetclient.c                  |   6 +-
 src/rpc/virnetclientprogram.c           |   6 +-
 src/rpc/virnetclientstream.c            |   6 +-
 src/rpc/virnetdaemon.c                  |   6 +-
 src/rpc/virnetlibsshsession.c           |   6 +-
 src/rpc/virnetsaslcontext.c             |  12 +--
 src/rpc/virnetserver.c                  |   6 +-
 src/rpc/virnetserverclient.c            |   6 +-
 src/rpc/virnetserverprogram.c           |   6 +-
 src/rpc/virnetserverservice.c           |   6 +-
 src/rpc/virnetsocket.c                  |   6 +-
 src/rpc/virnetsshsession.c              |   6 +-
 src/rpc/virnettlscontext.c              |  12 +--
 src/security/security_manager.c         |   6 +-
 src/util/virclosecallbacks.c            |   6 +-
 src/util/virdnsmasq.c                   |   7 +-
 src/util/virfdstream.c                  |   6 +-
 src/util/virfilecache.c                 |   6 +-
 src/util/virhash.c                      |   6 +-
 src/util/virhostdev.c                   |   6 +-
 src/util/viridentity.c                  |   6 +-
 src/util/virmacmap.c                    |   6 +-
 src/util/virmdev.c                      |   6 +-
 src/util/virobject.c                    |  12 +--
 src/util/virobject.h                    |   4 +
 src/util/virpci.c                       |   6 +-
 src/util/virportallocator.c             |   6 +-
 src/util/virresctrl.c                   |  12 +--
 src/util/virscsi.c                      |   6 +-
 src/util/virscsivhost.c                 |   6 +-
 src/util/virusb.c                       |   6 +-
 src/vbox/vbox_common.c                  |   6 +-
 src/vz/vz_driver.c                      |   6 +-
 tests/virfilecachetest.c                |   6 +-
 67 files changed, 230 insertions(+), 453 deletions(-)

diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c
index c268ec57f7..2940692598 100644
--- a/src/access/viraccessmanager.c
+++ b/src/access/viraccessmanager.c
@@ -54,10 +54,8 @@ static void virAccessManagerDispose(void *obj);
 
 static int virAccessManagerOnceInit(void)
 {
-    if (!(virAccessManagerClass = virClassNew(virClassForObjectLockable(),
-                                              "virAccessManagerClass",
-                                              sizeof(virAccessManager),
-                                              virAccessManagerDispose)))
+    if (!(virAccessManagerClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                virAccessManager)))
         return -1;
 
     return 0;
diff --git a/src/bhyve/bhyve_conf.c b/src/bhyve/bhyve_conf.c
index b0b40c5754..027311ad37 100644
--- a/src/bhyve/bhyve_conf.c
+++ b/src/bhyve/bhyve_conf.c
@@ -36,10 +36,8 @@ static void virBhyveDriverConfigDispose(void *obj);
 
 static int virBhyveConfigOnceInit(void)
 {
-     if (!(virBhyveDriverConfigClass = virClassNew(virClassForObject(),
-                                                   "virBhyveDriverConfig",
-                                                   sizeof(virBhyveDriverConfig),
-                                                   virBhyveDriverConfigDispose)))
+     if (!(virBhyveDriverConfigClass = VIR_CLASS_NEW(virClassForObject(),
+                                                     virBhyveDriverConfig)))
          return -1;
 
      return 0;
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 33b9194041..239d409388 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -62,10 +62,8 @@ static void virCapsDispose(void *obj);
 
 static int virCapabilitiesOnceInit(void)
 {
-    if (!(virCapsClass = virClassNew(virClassForObject(),
-                                     "virCaps",
-                                     sizeof(virCaps),
-                                     virCapsDispose)))
+    if (!(virCapsClass = VIR_CLASS_NEW(virClassForObject(),
+                                       virCaps)))
         return -1;
 
     return 0;
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index f7d9be50f8..aad93392ea 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -40,16 +40,12 @@ static void virDomainCapsCPUModelsDispose(void *obj);
 
 static int virDomainCapsOnceInit(void)
 {
-    if (!(virDomainCapsClass = virClassNew(virClassForObjectLockable(),
-                                           "virDomainCapsClass",
-                                           sizeof(virDomainCaps),
-                                           virDomainCapsDispose)))
+    if (!(virDomainCapsClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                             virDomainCaps)))
         return -1;
 
-    virDomainCapsCPUModelsClass = virClassNew(virClassForObject(),
-                                              "virDomainCapsCPUModelsClass",
-                                              sizeof(virDomainCapsCPUModels),
-                                              virDomainCapsCPUModelsDispose);
+    virDomainCapsCPUModelsClass = VIR_CLASS_NEW(virClassForObject(),
+                                                virDomainCapsCPUModels);
     if (!virDomainCapsCPUModelsClass)
         return -1;
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4dad8e3b20..364f41f384 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -938,16 +938,12 @@ static void virDomainXMLOptionDispose(void *obj);
 
 static int virDomainObjOnceInit(void)
 {
-    if (!(virDomainObjClass = virClassNew(virClassForObjectLockable(),
-                                          "virDomainObj",
-                                          sizeof(virDomainObj),
-                                          virDomainObjDispose)))
+    if (!(virDomainObjClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                            virDomainObj)))
         return -1;
 
-    if (!(virDomainXMLOptionClass = virClassNew(virClassForObject(),
-                                                "virDomainXMLOption",
-                                                sizeof(virDomainXMLOption),
-                                                virDomainXMLOptionDispose)))
+    if (!(virDomainXMLOptionClass = VIR_CLASS_NEW(virClassForObject(),
+                                                  virDomainXMLOption)))
         return -1;
 
     return 0;
@@ -12223,10 +12219,8 @@ static virClassPtr virDomainChrSourceDefClass;
 static int
 virDomainChrSourceDefOnceInit(void)
 {
-    virDomainChrSourceDefClass = virClassNew(virClassForObject(),
-                                             "virDomainChrSourceDef",
-                                             sizeof(virDomainChrSourceDef),
-                                             virDomainChrSourceDefDispose);
+    virDomainChrSourceDefClass = VIR_CLASS_NEW(virClassForObject(),
+                                               virDomainChrSourceDef);
     if (!virDomainChrSourceDefClass)
         return -1;
     else
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index fdb48a1eaa..2272844085 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -296,130 +296,88 @@ static int
 virDomainEventsOnceInit(void)
 {
     if (!(virDomainEventClass =
-          virClassNew(virClassForObjectEvent(),
-                      "virDomainEvent",
-                      sizeof(virDomainEvent),
-                      virDomainEventDispose)))
+          VIR_CLASS_NEW(virClassForObjectEvent(),
+                        virDomainEvent)))
         return -1;
     if (!(virDomainEventLifecycleClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventLifecycle",
-                      sizeof(virDomainEventLifecycle),
-                      virDomainEventLifecycleDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventLifecycle)))
         return -1;
     if (!(virDomainEventRTCChangeClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventRTCChange",
-                      sizeof(virDomainEventRTCChange),
-                      virDomainEventRTCChangeDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventRTCChange)))
         return -1;
     if (!(virDomainEventWatchdogClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventWatchdog",
-                      sizeof(virDomainEventWatchdog),
-                      virDomainEventWatchdogDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventWatchdog)))
         return -1;
     if (!(virDomainEventIOErrorClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventIOError",
-                      sizeof(virDomainEventIOError),
-                      virDomainEventIOErrorDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventIOError)))
         return -1;
     if (!(virDomainEventGraphicsClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventGraphics",
-                      sizeof(virDomainEventGraphics),
-                      virDomainEventGraphicsDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventGraphics)))
         return -1;
     if (!(virDomainEventBlockJobClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventBlockJob",
-                      sizeof(virDomainEventBlockJob),
-                      virDomainEventBlockJobDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventBlockJob)))
         return -1;
     if (!(virDomainEventDiskChangeClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventDiskChange",
-                      sizeof(virDomainEventDiskChange),
-                      virDomainEventDiskChangeDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventDiskChange)))
         return -1;
     if (!(virDomainEventTrayChangeClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventTrayChange",
-                      sizeof(virDomainEventTrayChange),
-                      virDomainEventTrayChangeDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventTrayChange)))
         return -1;
     if (!(virDomainEventBalloonChangeClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventBalloonChange",
-                      sizeof(virDomainEventBalloonChange),
-                      virDomainEventBalloonChangeDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventBalloonChange)))
         return -1;
     if (!(virDomainEventDeviceRemovedClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventDeviceRemoved",
-                      sizeof(virDomainEventDeviceRemoved),
-                      virDomainEventDeviceRemovedDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventDeviceRemoved)))
         return -1;
     if (!(virDomainEventDeviceAddedClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventDeviceAdded",
-                      sizeof(virDomainEventDeviceAdded),
-                      virDomainEventDeviceAddedDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventDeviceAdded)))
         return -1;
     if (!(virDomainEventPMClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventPM",
-                      sizeof(virDomainEventPM),
-                      virDomainEventPMDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventPM)))
         return -1;
     if (!(virDomainQemuMonitorEventClass =
-          virClassNew(virClassForObjectEvent(),
-                      "virDomainQemuMonitorEvent",
-                      sizeof(virDomainQemuMonitorEvent),
-                      virDomainQemuMonitorEventDispose)))
+          VIR_CLASS_NEW(virClassForObjectEvent(),
+                        virDomainQemuMonitorEvent)))
         return -1;
     if (!(virDomainEventTunableClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventTunable",
-                      sizeof(virDomainEventTunable),
-                      virDomainEventTunableDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventTunable)))
         return -1;
     if (!(virDomainEventAgentLifecycleClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventAgentLifecycle",
-                      sizeof(virDomainEventAgentLifecycle),
-                      virDomainEventAgentLifecycleDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventAgentLifecycle)))
         return -1;
     if (!(virDomainEventMigrationIterationClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventMigrationIteration",
-                      sizeof(virDomainEventMigrationIteration),
-                      virDomainEventMigrationIterationDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventMigrationIteration)))
         return -1;
     if (!(virDomainEventJobCompletedClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventJobCompleted",
-                      sizeof(virDomainEventJobCompleted),
-                      virDomainEventJobCompletedDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventJobCompleted)))
         return -1;
     if (!(virDomainEventDeviceRemovalFailedClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventDeviceRemovalFailed",
-                      sizeof(virDomainEventDeviceRemovalFailed),
-                      virDomainEventDeviceRemovalFailedDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventDeviceRemovalFailed)))
         return -1;
     if (!(virDomainEventMetadataChangeClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventMetadataChange",
-                      sizeof(virDomainEventMetadataChange),
-                      virDomainEventMetadataChangeDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventMetadataChange)))
         return -1;
     if (!(virDomainEventBlockThresholdClass =
-          virClassNew(virDomainEventClass,
-                      "virDomainEventBlockThreshold",
-                      sizeof(virDomainEventBlockThreshold),
-                      virDomainEventBlockThresholdDispose)))
+          VIR_CLASS_NEW(virDomainEventClass,
+                        virDomainEventBlockThreshold)))
         return -1;
     return 0;
 }
diff --git a/src/conf/network_event.c b/src/conf/network_event.c
index e0d1a3d5ca..2a94aceb6e 100644
--- a/src/conf/network_event.c
+++ b/src/conf/network_event.c
@@ -58,16 +58,12 @@ static int
 virNetworkEventsOnceInit(void)
 {
     if (!(virNetworkEventClass =
-          virClassNew(virClassForObjectEvent(),
-                      "virNetworkEvent",
-                      sizeof(virNetworkEvent),
-                      virNetworkEventDispose)))
+          VIR_CLASS_NEW(virClassForObjectEvent(),
+                        virNetworkEvent)))
         return -1;
     if (!(virNetworkEventLifecycleClass =
-          virClassNew(virNetworkEventClass,
-                      "virNetworkEventLifecycle",
-                      sizeof(virNetworkEventLifecycle),
-                      virNetworkEventLifecycleDispose)))
+          VIR_CLASS_NEW(virNetworkEventClass,
+                        virNetworkEventLifecycle)))
         return -1;
     return 0;
 }
diff --git a/src/conf/node_device_event.c b/src/conf/node_device_event.c
index 312ef512d1..0fbb4da49d 100644
--- a/src/conf/node_device_event.c
+++ b/src/conf/node_device_event.c
@@ -67,22 +67,16 @@ static int
 virNodeDeviceEventsOnceInit(void)
 {
     if (!(virNodeDeviceEventClass =
-          virClassNew(virClassForObjectEvent(),
-                      "virNodeDeviceEvent",
-                      sizeof(virNodeDeviceEvent),
-                      virNodeDeviceEventDispose)))
+          VIR_CLASS_NEW(virClassForObjectEvent(),
+                        virNodeDeviceEvent)))
         return -1;
     if (!(virNodeDeviceEventLifecycleClass =
-          virClassNew(virNodeDeviceEventClass,
-                      "virNodeDeviceEventLifecycle",
-                      sizeof(virNodeDeviceEventLifecycle),
-                      virNodeDeviceEventLifecycleDispose)))
+          VIR_CLASS_NEW(virNodeDeviceEventClass,
+                        virNodeDeviceEventLifecycle)))
         return -1;
     if (!(virNodeDeviceEventUpdateClass =
-          virClassNew(virNodeDeviceEventClass,
-                      "virNodeDeviceEventUpdate",
-                      sizeof(virNodeDeviceEventUpdate),
-                      virNodeDeviceEventUpdateDispose)))
+          VIR_CLASS_NEW(virNodeDeviceEventClass,
+                        virNodeDeviceEventUpdate)))
         return -1;
     return 0;
 }
diff --git a/src/conf/object_event.c b/src/conf/object_event.c
index e8116b880c..5a3dccc70b 100644
--- a/src/conf/object_event.c
+++ b/src/conf/object_event.c
@@ -93,17 +93,13 @@ static int
 virObjectEventOnceInit(void)
 {
     if (!(virObjectEventStateClass =
-          virClassNew(virClassForObjectLockable(),
-                      "virObjectEventState",
-                      sizeof(virObjectEventState),
-                      virObjectEventStateDispose)))
+          VIR_CLASS_NEW(virClassForObjectLockable(),
+                        virObjectEventState)))
         return -1;
 
     if (!(virObjectEventClass =
-          virClassNew(virClassForObject(),
-                      "virObjectEvent",
-                      sizeof(virObjectEvent),
-                      virObjectEventDispose)))
+          VIR_CLASS_NEW(virClassForObject(),
+                        virObjectEvent)))
         return -1;
 
     return 0;
diff --git a/src/conf/secret_event.c b/src/conf/secret_event.c
index c130909282..4b9898a9d1 100644
--- a/src/conf/secret_event.c
+++ b/src/conf/secret_event.c
@@ -66,22 +66,16 @@ static int
 virSecretEventsOnceInit(void)
 {
     if (!(virSecretEventClass =
-          virClassNew(virClassForObjectEvent(),
-                      "virSecretEvent",
-                      sizeof(virSecretEvent),
-                      virSecretEventDispose)))
+          VIR_CLASS_NEW(virClassForObjectEvent(),
+                        virSecretEvent)))
         return -1;
     if (!(virSecretEventLifecycleClass =
-          virClassNew(virSecretEventClass,
-                      "virSecretEventLifecycle",
-                      sizeof(virSecretEventLifecycle),
-                      virSecretEventLifecycleDispose)))
+          VIR_CLASS_NEW(virSecretEventClass,
+                        virSecretEventLifecycle)))
         return -1;
     if (!(virSecretEventValueChangedClass =
-          virClassNew(virSecretEventClass,
-                      "virSecretEventValueChanged",
-                      sizeof(virSecretEventValueChanged),
-                      virSecretEventValueChangedDispose)))
+          VIR_CLASS_NEW(virSecretEventClass,
+                        virSecretEventValueChanged)))
         return -1;
     return 0;
 }
diff --git a/src/conf/storage_event.c b/src/conf/storage_event.c
index f9b796878a..83aa165787 100644
--- a/src/conf/storage_event.c
+++ b/src/conf/storage_event.c
@@ -67,22 +67,16 @@ static int
 virStoragePoolEventsOnceInit(void)
 {
     if (!(virStoragePoolEventClass =
-          virClassNew(virClassForObjectEvent(),
-                      "virStoragePoolEvent",
-                      sizeof(virStoragePoolEvent),
-                      virStoragePoolEventDispose)))
+          VIR_CLASS_NEW(virClassForObjectEvent(),
+                        virStoragePoolEvent)))
         return -1;
     if (!(virStoragePoolEventLifecycleClass =
-          virClassNew(virStoragePoolEventClass,
-                      "virStoragePoolEventLifecycle",
-                      sizeof(virStoragePoolEventLifecycle),
-                      virStoragePoolEventLifecycleDispose)))
+          VIR_CLASS_NEW(virStoragePoolEventClass,
+                        virStoragePoolEventLifecycle)))
         return -1;
     if (!(virStoragePoolEventRefreshClass =
-          virClassNew(virStoragePoolEventClass,
-                      "virStoragePoolEventRefresh",
-                      sizeof(virStoragePoolEventRefresh),
-                      virStoragePoolEventRefreshDispose)))
+          VIR_CLASS_NEW(virStoragePoolEventClass,
+                        virStoragePoolEventRefresh)))
         return -1;
     return 0;
 }
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index 7022abe094..5df4921187 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -56,10 +56,8 @@ struct _virDomainObjList {
 
 static int virDomainObjListOnceInit(void)
 {
-    if (!(virDomainObjListClass = virClassNew(virClassForObjectRWLockable(),
-                                              "virDomainObjList",
-                                              sizeof(virDomainObjList),
-                                              virDomainObjListDispose)))
+    if (!(virDomainObjListClass = VIR_CLASS_NEW(virClassForObjectRWLockable(),
+                                                virDomainObjList)))
         return -1;
 
     return 0;
diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index f90c0bd9c4..8c572fbb23 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -58,16 +58,12 @@ static void virInterfaceObjListDispose(void *obj);
 static int
 virInterfaceObjOnceInit(void)
 {
-    if (!(virInterfaceObjClass = virClassNew(virClassForObjectLockable(),
-                                             "virInterfaceObj",
-                                             sizeof(virInterfaceObj),
-                                             virInterfaceObjDispose)))
+    if (!(virInterfaceObjClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                               virInterfaceObj)))
         return -1;
 
-    if (!(virInterfaceObjListClass = virClassNew(virClassForObjectRWLockable(),
-                                                 "virInterfaceObjList",
-                                                 sizeof(virInterfaceObjList),
-                                                 virInterfaceObjListDispose)))
+    if (!(virInterfaceObjListClass = VIR_CLASS_NEW(virClassForObjectRWLockable(),
+                                                   virInterfaceObjList)))
         return -1;
 
     return 0;
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 8cd1b62c1c..38522445e1 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -74,16 +74,12 @@ static void virNetworkObjListDispose(void *obj);
 static int
 virNetworkObjOnceInit(void)
 {
-    if (!(virNetworkObjClass = virClassNew(virClassForObjectLockable(),
-                                           "virNetworkObj",
-                                           sizeof(virNetworkObj),
-                                           virNetworkObjDispose)))
+    if (!(virNetworkObjClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                             virNetworkObj)))
         return -1;
 
-    if (!(virNetworkObjListClass = virClassNew(virClassForObjectRWLockable(),
-                                               "virNetworkObjList",
-                                               sizeof(virNetworkObjList),
-                                               virNetworkObjListDispose)))
+    if (!(virNetworkObjListClass = VIR_CLASS_NEW(virClassForObjectRWLockable(),
+                                                 virNetworkObjList)))
         return -1;
     return 0;
 }
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index 9d2996046f..09402d82f1 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -58,16 +58,12 @@ static bool virNodeDeviceObjHasCap(const virNodeDeviceObj *obj, int type);
 static int
 virNodeDeviceObjOnceInit(void)
 {
-    if (!(virNodeDeviceObjClass = virClassNew(virClassForObjectLockable(),
-                                              "virNodeDeviceObj",
-                                              sizeof(virNodeDeviceObj),
-                                              virNodeDeviceObjDispose)))
+    if (!(virNodeDeviceObjClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                virNodeDeviceObj)))
         return -1;
 
-    if (!(virNodeDeviceObjListClass = virClassNew(virClassForObjectRWLockable(),
-                                                  "virNodeDeviceObjList",
-                                                  sizeof(virNodeDeviceObjList),
-                                                  virNodeDeviceObjListDispose)))
+    if (!(virNodeDeviceObjListClass = VIR_CLASS_NEW(virClassForObjectRWLockable(),
+                                                    virNodeDeviceObjList)))
         return -1;
 
     return 0;
diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 47e0b28968..fee31ff366 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -68,16 +68,12 @@ struct virSecretSearchData {
 static int
 virSecretObjOnceInit(void)
 {
-    if (!(virSecretObjClass = virClassNew(virClassForObjectLockable(),
-                                          "virSecretObj",
-                                          sizeof(virSecretObj),
-                                          virSecretObjDispose)))
+    if (!(virSecretObjClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                            virSecretObj)))
         return -1;
 
-    if (!(virSecretObjListClass = virClassNew(virClassForObjectRWLockable(),
-                                              "virSecretObjList",
-                                              sizeof(virSecretObjList),
-                                              virSecretObjListDispose)))
+    if (!(virSecretObjListClass = VIR_CLASS_NEW(virClassForObjectRWLockable(),
+                                                virSecretObjList)))
         return -1;
 
     return 0;
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 799b8c9fa3..7ed187e30f 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -110,16 +110,12 @@ struct _virStoragePoolObjList {
 static int
 virStorageVolObjOnceInit(void)
 {
-    if (!(virStorageVolObjClass = virClassNew(virClassForObjectLockable(),
-                                              "virStorageVolObj",
-                                              sizeof(virStorageVolObj),
-                                              virStorageVolObjDispose)))
+    if (!(virStorageVolObjClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                virStorageVolObj)))
         return -1;
 
-    if (!(virStorageVolObjListClass = virClassNew(virClassForObjectRWLockable(),
-                                                  "virStorageVolObjList",
-                                                  sizeof(virStorageVolObjList),
-                                                  virStorageVolObjListDispose)))
+    if (!(virStorageVolObjListClass = VIR_CLASS_NEW(virClassForObjectRWLockable(),
+                                                    virStorageVolObjList)))
         return -1;
 
     return 0;
@@ -207,16 +203,12 @@ virStorageVolObjListDispose(void *opaque)
 static int
 virStoragePoolObjOnceInit(void)
 {
-    if (!(virStoragePoolObjClass = virClassNew(virClassForObjectLockable(),
-                                               "virStoragePoolObj",
-                                               sizeof(virStoragePoolObj),
-                                               virStoragePoolObjDispose)))
+    if (!(virStoragePoolObjClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                 virStoragePoolObj)))
         return -1;
 
-    if (!(virStoragePoolObjListClass = virClassNew(virClassForObjectRWLockable(),
-                                                   "virStoragePoolObjList",
-                                                   sizeof(virStoragePoolObjList),
-                                                   virStoragePoolObjListDispose)))
+    if (!(virStoragePoolObjListClass = VIR_CLASS_NEW(virClassForObjectRWLockable(),
+                                                     virStoragePoolObjList)))
         return -1;
 
     return 0;
diff --git a/src/datatypes.c b/src/datatypes.c
index 0c3c66a9ce..3016e45076 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -74,10 +74,8 @@ static int
 virDataTypesOnceInit(void)
 {
 #define DECLARE_CLASS_COMMON(basename, parent) \
-    if (!(basename ## Class = virClassNew(parent, \
-                                          #basename, \
-                                          sizeof(basename), \
-                                          basename ## Dispose))) \
+    if (!(basename ## Class = VIR_CLASS_NEW(parent, \
+                                            basename))) \
         return -1;
 #define DECLARE_CLASS(basename) \
     DECLARE_CLASS_COMMON(basename, virClassForObject())
diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c
index cc2402febb..8acccf8940 100644
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
@@ -55,10 +55,8 @@ static void virNetcfDriverStateDispose(void *obj);
 static int
 virNetcfDriverStateOnceInit(void)
 {
-    if (!(virNetcfDriverStateClass = virClassNew(virClassForObjectLockable(),
-                                       "virNetcfDriverState",
-                                       sizeof(virNetcfDriverState),
-                                       virNetcfDriverStateDispose)))
+    if (!(virNetcfDriverStateClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                   virNetcfDriverStat)))
         return -1;
     return 0;
 }
diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
index 7f695311b4..cb2ec9d633 100644
--- a/src/libvirt-admin.c
+++ b/src/libvirt-admin.c
@@ -67,10 +67,8 @@ virAdmGlobalInit(void)
     if (!bindtextdomain(PACKAGE, LOCALEDIR))
         goto error;
 
-    if (!(remoteAdminPrivClass = virClassNew(virClassForObjectLockable(),
-                                             "remoteAdminPriv",
-                                             sizeof(remoteAdminPriv),
-                                             remoteAdminPrivDispose)))
+    if (!(remoteAdminPrivClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                               remoteAdminPriv)))
         goto error;
 
     return;
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index df1cece82f..b398cbee67 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -62,10 +62,8 @@ static void libxlDriverConfigDispose(void *obj);
 
 static int libxlConfigOnceInit(void)
 {
-    if (!(libxlDriverConfigClass = virClassNew(virClassForObject(),
-                                               "libxlDriverConfig",
-                                               sizeof(libxlDriverConfig),
-                                               libxlDriverConfigDispose)))
+    if (!(libxlDriverConfigClass = VIR_CLASS_NEW(virClassForObject(),
+                                                 libxlDriverConfig)))
         return -1;
 
     return 0;
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index ef9a902671..0882a9fc98 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -58,10 +58,8 @@ libxlDomainObjPrivateDispose(void *obj);
 static int
 libxlDomainObjPrivateOnceInit(void)
 {
-    if (!(libxlDomainObjPrivateClass = virClassNew(virClassForObjectLockable(),
-                                                   "libxlDomainObjPrivate",
-                                                   sizeof(libxlDomainObjPrivate),
-                                                   libxlDomainObjPrivateDispose)))
+    if (!(libxlDomainObjPrivateClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                     libxlDomainObjPrivate)))
         return -1;
 
     return 0;
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 5c0fd4b052..93e72bf815 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -244,10 +244,8 @@ libxlMigrationDstArgsDispose(void *obj)
 static int
 libxlMigrationDstArgsOnceInit(void)
 {
-    if (!(libxlMigrationDstArgsClass = virClassNew(virClassForObject(),
-                                                   "libxlMigrationDstArgs",
-                                                   sizeof(libxlMigrationDstArgs),
-                                                   libxlMigrationDstArgsDispose)))
+    if (!(libxlMigrationDstArgsClass = VIR_CLASS_NEW(virClassForObject(),
+                                                     libxlMigrationDstArgs)))
         return -1;
 
     return 0;
diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c
index cd0ba6ec24..7d275d4a80 100644
--- a/src/logging/log_handler.c
+++ b/src/logging/log_handler.c
@@ -76,10 +76,8 @@ static void virLogHandlerDispose(void *obj);
 static int
 virLogHandlerOnceInit(void)
 {
-    if (!(virLogHandlerClass = virClassNew(virClassForObjectLockable(),
-                                          "virLogHandler",
-                                          sizeof(virLogHandler),
-                                          virLogHandlerDispose)))
+    if (!(virLogHandlerClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                             virLogHandler)))
         return -1;
 
     return 0;
diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
index 92a82a4768..f623685be8 100644
--- a/src/lxc/lxc_conf.c
+++ b/src/lxc/lxc_conf.c
@@ -48,10 +48,8 @@ static void virLXCDriverConfigDispose(void *obj);
 
 static int virLXCConfigOnceInit(void)
 {
-    if (!(virLXCDriverConfigClass = virClassNew(virClassForObject(),
-                                                 "virLXCDriverConfig",
-                                                 sizeof(virLXCDriverConfig),
-                                                 virLXCDriverConfigDispose)))
+    if (!(virLXCDriverConfigClass = VIR_CLASS_NEW(virClassForObject(),
+                                                  virLXCDriverConfig)))
         return -1;
 
     return 0;
diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c
index 9cab6c2035..0c6daf5882 100644
--- a/src/lxc/lxc_monitor.c
+++ b/src/lxc/lxc_monitor.c
@@ -51,10 +51,8 @@ static void virLXCMonitorDispose(void *obj);
 
 static int virLXCMonitorOnceInit(void)
 {
-    if (!(virLXCMonitorClass = virClassNew(virClassForObjectLockable(),
-                                           "virLXCMonitor",
-                                           sizeof(virLXCMonitor),
-                                           virLXCMonitorDispose)))
+    if (!(virLXCMonitorClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                             virLXCMonitor)))
         return -1;
 
     return 0;
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index de01816402..e87b5886b8 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -93,10 +93,8 @@ udevEventDataDispose(void *obj)
 static int
 udevEventDataOnceInit(void)
 {
-    if (!(udevEventDataClass = virClassNew(virClassForObjectLockable(),
-                                           "udevEventData",
-                                           sizeof(udevEventData),
-                                           udevEventDataDispose)))
+    if (!(udevEventDataClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                             udevEventData)))
         return -1;
 
     return 0;
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 85af53d194..3e03997f07 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -138,10 +138,8 @@ static void qemuAgentDispose(void *obj);
 
 static int qemuAgentOnceInit(void)
 {
-    if (!(qemuAgentClass = virClassNew(virClassForObjectLockable(),
-                                       "qemuAgent",
-                                       sizeof(qemuAgent),
-                                       qemuAgentDispose)))
+    if (!(qemuAgentClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                         qemuAgent)))
         return -1;
 
     return 0;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b5347b6cad..656dac18bf 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -548,10 +548,8 @@ static void virQEMUCapsDispose(void *obj);
 
 static int virQEMUCapsOnceInit(void)
 {
-    if (!(virQEMUCapsClass = virClassNew(virClassForObject(),
-                                         "virQEMUCaps",
-                                         sizeof(virQEMUCaps),
-                                         virQEMUCapsDispose)))
+    if (!(virQEMUCapsClass = VIR_CLASS_NEW(virClassForObject(),
+                                           virQEMUCaps)))
         return -1;
 
     return 0;
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 36cf3a281c..5e05fedb9e 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -80,10 +80,8 @@ static void virQEMUDriverConfigDispose(void *obj);
 
 static int virQEMUConfigOnceInit(void)
 {
-    virQEMUDriverConfigClass = virClassNew(virClassForObject(),
-                                           "virQEMUDriverConfig",
-                                           sizeof(virQEMUDriverConfig),
-                                           virQEMUDriverConfigDispose);
+    virQEMUDriverConfigClass = VIR_CLASS_NEW(virClassForObject(),
+                                             virQEMUDriverConfig);
 
     if (!virQEMUDriverConfigClass)
         return -1;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7000de6a91..133506dd18 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -130,16 +130,12 @@ static void qemuDomainSaveCookieDispose(void *obj);
 static int
 qemuDomainOnceInit(void)
 {
-    if (!(qemuDomainLogContextClass = virClassNew(virClassForObject(),
-                                                 "qemuDomainLogContext",
-                                                 sizeof(qemuDomainLogContext),
-                                                 qemuDomainLogContextDispose)))
+    if (!(qemuDomainLogContextClass = VIR_CLASS_NEW(virClassForObject(),
+                                                    qemuDomainLogContext)))
         return -1;
 
-    if (!(qemuDomainSaveCookieClass = virClassNew(virClassForObject(),
-                                                  "qemuDomainSaveCookie",
-                                                  sizeof(qemuDomainSaveCookie),
-                                                  qemuDomainSaveCookieDispose)))
+    if (!(qemuDomainSaveCookieClass = VIR_CLASS_NEW(virClassForObject(),
+                                                    qemuDomainSaveCookie)))
         return -1;
 
     return 0;
@@ -988,10 +984,8 @@ static void qemuDomainDiskPrivateDispose(void *obj);
 static int
 qemuDomainDiskPrivateOnceInit(void)
 {
-    qemuDomainDiskPrivateClass = virClassNew(virClassForObject(),
-                                             "qemuDomainDiskPrivate",
-                                             sizeof(qemuDomainDiskPrivate),
-                                             qemuDomainDiskPrivateDispose);
+    qemuDomainDiskPrivateClass = VIR_CLASS_NEW(virClassForObject(),
+                                               qemuDomainDiskPrivate);
     if (!qemuDomainDiskPrivateClass)
         return -1;
     else
@@ -1028,10 +1022,8 @@ static void qemuDomainStorageSourcePrivateDispose(void *obj);
 static int
 qemuDomainStorageSourcePrivateOnceInit(void)
 {
-    qemuDomainStorageSourcePrivateClass = virClassNew(virClassForObject(),
-                                                      "qemuDomainStorageSourcePrivate",
-                                                      sizeof(qemuDomainStorageSourcePrivate),
-                                                      qemuDomainStorageSourcePrivateDispose);
+    qemuDomainStorageSourcePrivateClass = VIR_CLASS_NEW(virClassForObject(),
+                                                        qemuDomainStorageSourcePrivate);
     if (!qemuDomainStorageSourcePrivateClass)
         return -1;
     else
@@ -1071,10 +1063,8 @@ static void qemuDomainVcpuPrivateDispose(void *obj);
 static int
 qemuDomainVcpuPrivateOnceInit(void)
 {
-    qemuDomainVcpuPrivateClass = virClassNew(virClassForObject(),
-                                             "qemuDomainVcpuPrivate",
-                                             sizeof(qemuDomainVcpuPrivate),
-                                             qemuDomainVcpuPrivateDispose);
+    qemuDomainVcpuPrivateClass = VIR_CLASS_NEW(virClassForObject(),
+                                               qemuDomainVcpuPrivate);
     if (!qemuDomainVcpuPrivateClass)
         return -1;
     else
@@ -1116,10 +1106,8 @@ static int
 qemuDomainChrSourcePrivateOnceInit(void)
 {
     qemuDomainChrSourcePrivateClass =
-        virClassNew(virClassForObject(),
-                    "qemuDomainChrSourcePrivate",
-                    sizeof(qemuDomainChrSourcePrivate),
-                    qemuDomainChrSourcePrivateDispose);
+        VIR_CLASS_NEW(virClassForObject(),
+                      qemuDomainChrSourcePrivate);
     if (!qemuDomainChrSourcePrivateClass)
         return -1;
     else
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 7b647525b3..a8b2ba91b5 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -165,10 +165,8 @@ static void qemuMonitorDispose(void *obj);
 
 static int qemuMonitorOnceInit(void)
 {
-    if (!(qemuMonitorClass = virClassNew(virClassForObjectLockable(),
-                                         "qemuMonitor",
-                                         sizeof(qemuMonitor),
-                                         qemuMonitorDispose)))
+    if (!(qemuMonitorClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                           qemuMonitor)))
         return -1;
 
     return 0;
diff --git a/src/rpc/virkeepalive.c b/src/rpc/virkeepalive.c
index 4f666fd09b..aa49e6c7f4 100644
--- a/src/rpc/virkeepalive.c
+++ b/src/rpc/virkeepalive.c
@@ -58,10 +58,8 @@ static void virKeepAliveDispose(void *obj);
 
 static int virKeepAliveOnceInit(void)
 {
-    if (!(virKeepAliveClass = virClassNew(virClassForObjectLockable(),
-                                          "virKeepAlive",
-                                          sizeof(virKeepAlive),
-                                          virKeepAliveDispose)))
+    if (!(virKeepAliveClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                            virKeepAlive)))
         return -1;
 
     return 0;
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 0c8d58c32c..5fa304b24f 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -120,10 +120,8 @@ static void virNetClientDispose(void *obj);
 
 static int virNetClientOnceInit(void)
 {
-    if (!(virNetClientClass = virClassNew(virClassForObjectLockable(),
-                                          "virNetClient",
-                                          sizeof(virNetClient),
-                                          virNetClientDispose)))
+    if (!(virNetClientClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                            virNetClient)))
         return -1;
 
     return 0;
diff --git a/src/rpc/virnetclientprogram.c b/src/rpc/virnetclientprogram.c
index d1e54a0892..04f5dea7ad 100644
--- a/src/rpc/virnetclientprogram.c
+++ b/src/rpc/virnetclientprogram.c
@@ -54,10 +54,8 @@ static void virNetClientProgramDispose(void *obj);
 
 static int virNetClientProgramOnceInit(void)
 {
-    if (!(virNetClientProgramClass = virClassNew(virClassForObject(),
-                                                 "virNetClientProgram",
-                                                 sizeof(virNetClientProgram),
-                                                 virNetClientProgramDispose)))
+    if (!(virNetClientProgramClass = VIR_CLASS_NEW(virClassForObject(),
+                                                   virNetClientProgram)))
         return -1;
 
     return 0;
diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c
index 2f037db0dc..963634b5af 100644
--- a/src/rpc/virnetclientstream.c
+++ b/src/rpc/virnetclientstream.c
@@ -71,10 +71,8 @@ static void virNetClientStreamDispose(void *obj);
 
 static int virNetClientStreamOnceInit(void)
 {
-    if (!(virNetClientStreamClass = virClassNew(virClassForObjectLockable(),
-                                                "virNetClientStream",
-                                                sizeof(virNetClientStream),
-                                                virNetClientStreamDispose)))
+    if (!(virNetClientStreamClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                  virNetClientStream)))
         return -1;
 
     return 0;
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index 6f00bfd9d1..8658878b0a 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -110,10 +110,8 @@ virNetDaemonDispose(void *obj)
 static int
 virNetDaemonOnceInit(void)
 {
-    if (!(virNetDaemonClass = virClassNew(virClassForObjectLockable(),
-                                          "virNetDaemon",
-                                          sizeof(virNetDaemon),
-                                          virNetDaemonDispose)))
+    if (!(virNetDaemonClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                            virNetDaemon)))
         return -1;
 
     return 0;
diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c
index 25f93cec97..5875c258df 100644
--- a/src/rpc/virnetlibsshsession.c
+++ b/src/rpc/virnetlibsshsession.c
@@ -161,10 +161,8 @@ virNetLibsshSessionOnceInit(void)
 {
     const char *dbgLevelStr;
 
-    if (!(virNetLibsshSessionClass = virClassNew(virClassForObjectLockable(),
-                                                 "virNetLibsshSession",
-                                                 sizeof(virNetLibsshSession),
-                                                 virNetLibsshSessionDispose)))
+    if (!(virNetLibsshSessionClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                   virNetLibsshSession)))
         return -1;
 
     if (ssh_init() < 0) {
diff --git a/src/rpc/virnetsaslcontext.c b/src/rpc/virnetsaslcontext.c
index f1f8bdc855..56fa57ff7e 100644
--- a/src/rpc/virnetsaslcontext.c
+++ b/src/rpc/virnetsaslcontext.c
@@ -57,16 +57,12 @@ static void virNetSASLSessionDispose(void *obj);
 
 static int virNetSASLContextOnceInit(void)
 {
-    if (!(virNetSASLContextClass = virClassNew(virClassForObjectLockable(),
-                                               "virNetSASLContext",
-                                               sizeof(virNetSASLContext),
-                                               virNetSASLContextDispose)))
+    if (!(virNetSASLContextClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                 virNetSASLContext)))
         return -1;
 
-    if (!(virNetSASLSessionClass = virClassNew(virClassForObjectLockable(),
-                                               "virNetSASLSession",
-                                               sizeof(virNetSASLSession),
-                                               virNetSASLSessionDispose)))
+    if (!(virNetSASLSessionClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                 virNetSASLSession)))
         return -1;
 
     return 0;
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 3ce21a8f53..de0692f3cf 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -93,10 +93,8 @@ static inline size_t virNetServerTrackCompletedAuthLocked(virNetServerPtr srv);
 
 static int virNetServerOnceInit(void)
 {
-    if (!(virNetServerClass = virClassNew(virClassForObjectLockable(),
-                                          "virNetServer",
-                                          sizeof(virNetServer),
-                                          virNetServerDispose)))
+    if (!(virNetServerClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                            virNetServer)))
         return -1;
 
     return 0;
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index d5f0cf448f..2ef590641c 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -129,10 +129,8 @@ static void virNetServerClientDispose(void *obj);
 
 static int virNetServerClientOnceInit(void)
 {
-    if (!(virNetServerClientClass = virClassNew(virClassForObjectLockable(),
-                                                "virNetServerClient",
-                                                sizeof(virNetServerClient),
-                                                virNetServerClientDispose)))
+    if (!(virNetServerClientClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                  virNetServerClient)))
         return -1;
 
     return 0;
diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c
index f28274fc22..75cd01b299 100644
--- a/src/rpc/virnetserverprogram.c
+++ b/src/rpc/virnetserverprogram.c
@@ -51,10 +51,8 @@ static void virNetServerProgramDispose(void *obj);
 
 static int virNetServerProgramOnceInit(void)
 {
-    if (!(virNetServerProgramClass = virClassNew(virClassForObject(),
-                                                 "virNetServerProgram",
-                                                 sizeof(virNetServerProgram),
-                                                 virNetServerProgramDispose)))
+    if (!(virNetServerProgramClass = VIR_CLASS_NEW(virClassForObject(),
+                                                   virNetServerProgram)))
         return -1;
 
     return 0;
diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index d2350b2b91..11a1d164fd 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -57,10 +57,8 @@ static void virNetServerServiceDispose(void *obj);
 
 static int virNetServerServiceOnceInit(void)
 {
-    if (!(virNetServerServiceClass = virClassNew(virClassForObject(),
-                                                 "virNetServerService",
-                                                 sizeof(virNetServerService),
-                                                 virNetServerServiceDispose)))
+    if (!(virNetServerServiceClass = VIR_CLASS_NEW(virClassForObject(),
+                                                   virNetServerService)))
         return -1;
 
     return 0;
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index f362a09555..f605db3930 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -124,10 +124,8 @@ static void virNetSocketDispose(void *obj);
 
 static int virNetSocketOnceInit(void)
 {
-    if (!(virNetSocketClass = virClassNew(virClassForObjectLockable(),
-                                          "virNetSocket",
-                                          sizeof(virNetSocket),
-                                          virNetSocketDispose)))
+    if (!(virNetSocketClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                            virNetSocket)))
         return -1;
 
     return 0;
diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c
index e742175654..a5f905b88d 100644
--- a/src/rpc/virnetsshsession.c
+++ b/src/rpc/virnetsshsession.c
@@ -167,10 +167,8 @@ static virClassPtr virNetSSHSessionClass;
 static int
 virNetSSHSessionOnceInit(void)
 {
-    if (!(virNetSSHSessionClass = virClassNew(virClassForObjectLockable(),
-                                              "virNetSSHSession",
-                                              sizeof(virNetSSHSession),
-                                              virNetSSHSessionDispose)))
+    if (!(virNetSSHSessionClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                virNetSSHSession)))
         return -1;
 
     return 0;
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 5699eb8f24..a419c5f831 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -90,16 +90,12 @@ static void virNetTLSSessionDispose(void *obj);
 
 static int virNetTLSContextOnceInit(void)
 {
-    if (!(virNetTLSContextClass = virClassNew(virClassForObjectLockable(),
-                                              "virNetTLSContext",
-                                              sizeof(virNetTLSContext),
-                                              virNetTLSContextDispose)))
+    if (!(virNetTLSContextClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                virNetTLSContext)))
         return -1;
 
-    if (!(virNetTLSSessionClass = virClassNew(virClassForObjectLockable(),
-                                              "virNetTLSSession",
-                                              sizeof(virNetTLSSession),
-                                              virNetTLSSessionDispose)))
+    if (!(virNetTLSSessionClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                virNetTLSSession)))
         return -1;
 
     return 0;
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index fdeea4d533..422de15060 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -59,10 +59,8 @@ void virSecurityManagerDispose(void *obj)
 static int
 virSecurityManagerOnceInit(void)
 {
-    if (!(virSecurityManagerClass = virClassNew(virClassForObjectLockable(),
-                                                "virSecurityManagerClass",
-                                                sizeof(virSecurityManager),
-                                                virSecurityManagerDispose)))
+    if (!(virSecurityManagerClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                  virSecurityManager)))
         return -1;
 
     return 0;
diff --git a/src/util/virclosecallbacks.c b/src/util/virclosecallbacks.c
index 49dac65892..11d8fd8cbd 100644
--- a/src/util/virclosecallbacks.c
+++ b/src/util/virclosecallbacks.c
@@ -53,10 +53,8 @@ static void virCloseCallbacksDispose(void *obj);
 
 static int virCloseCallbacksOnceInit(void)
 {
-    virCloseCallbacksClass = virClassNew(virClassForObjectLockable(),
-                                         "virCloseCallbacks",
-                                         sizeof(virCloseCallbacks),
-                                         virCloseCallbacksDispose);
+    virCloseCallbacksClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                           virCloseCallbacks);
 
     if (!virCloseCallbacksClass)
         return -1;
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index db5c51b6a2..3af3785a23 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -641,12 +641,9 @@ dnsmasqCapsDispose(void *obj)
 
 static int dnsmasqCapsOnceInit(void)
 {
-    if (!(dnsmasqCapsClass = virClassNew(virClassForObject(),
-                                         "dnsmasqCaps",
-                                         sizeof(dnsmasqCaps),
-                                         dnsmasqCapsDispose))) {
+    if (!(dnsmasqCapsClass = VIR_CLASS_NEW(virClassForObject(),
+                                           dnsmasqCaps)))
         return -1;
-    }
 
     return 0;
 }
diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index e2d3f365cd..e7d2e4a954 100644
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -129,10 +129,8 @@ virFDStreamDataDispose(void *obj)
 
 static int virFDStreamDataOnceInit(void)
 {
-    if (!(virFDStreamDataClass = virClassNew(virClassForObjectLockable(),
-                                             "virFDStreamData",
-                                             sizeof(virFDStreamData),
-                                             virFDStreamDataDispose)))
+    if (!(virFDStreamDataClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                               virFDStreamData)))
         return -1;
 
     return 0;
diff --git a/src/util/virfilecache.c b/src/util/virfilecache.c
index e24eb5a68c..a1ff518386 100644
--- a/src/util/virfilecache.c
+++ b/src/util/virfilecache.c
@@ -86,10 +86,8 @@ virFileCacheDispose(void *obj)
 static int
 virFileCacheOnceInit(void)
 {
-    if (!(virFileCacheClass = virClassNew(virClassForObjectLockable(),
-                                          "virFileCache",
-                                          sizeof(virFileCache),
-                                          virFileCacheDispose)))
+    if (!(virFileCacheClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                            virFileCache)))
         return -1;
 
     return 0;
diff --git a/src/util/virhash.c b/src/util/virhash.c
index 475c2b0281..bd00b7f359 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -77,10 +77,8 @@ static void virHashAtomicDispose(void *obj);
 
 static int virHashAtomicOnceInit(void)
 {
-    virHashAtomicClass = virClassNew(virClassForObjectLockable(),
-                                     "virHashAtomic",
-                                     sizeof(virHashAtomic),
-                                     virHashAtomicDispose);
+    virHashAtomicClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                       virHashAtomic);
     if (!virHashAtomicClass)
         return -1;
     else
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index a12224c58f..b8a5d4d5d3 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -120,10 +120,8 @@ static int virHostdevIsPCINodeDeviceUsed(virPCIDeviceAddressPtr devAddr, void *o
 
 static int virHostdevManagerOnceInit(void)
 {
-    if (!(virHostdevManagerClass = virClassNew(virClassForObject(),
-                                               "virHostdevManager",
-                                               sizeof(virHostdevManager),
-                                               virHostdevManagerDispose)))
+    if (!(virHostdevManagerClass = VIR_CLASS_NEW(virClassForObject(),
+                                                 virHostdevManager)))
         return -1;
 
     if (!(manager = virHostdevManagerNew()))
diff --git a/src/util/viridentity.c b/src/util/viridentity.c
index 52a0a30a45..b9ffeecefa 100644
--- a/src/util/viridentity.c
+++ b/src/util/viridentity.c
@@ -54,10 +54,8 @@ static void virIdentityDispose(void *obj);
 
 static int virIdentityOnceInit(void)
 {
-    if (!(virIdentityClass = virClassNew(virClassForObject(),
-                                         "virIdentity",
-                                         sizeof(virIdentity),
-                                         virIdentityDispose)))
+    if (!(virIdentityClass = VIR_CLASS_NEW(virClassForObject(),
+                                           virIdentity)))
         return -1;
 
     if (virThreadLocalInit(&virIdentityCurrent,
diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c
index d3be3066cc..3e376c0c73 100644
--- a/src/util/virmacmap.c
+++ b/src/util/virmacmap.c
@@ -74,10 +74,8 @@ virMacMapDispose(void *obj)
 
 static int virMacMapOnceInit(void)
 {
-    if (!(virMacMapClass = virClassNew(virClassForObjectLockable(),
-                                       "virMacMapClass",
-                                       sizeof(virMacMap),
-                                       virMacMapDispose)))
+    if (!(virMacMapClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                         virMacMap)))
         return -1;
 
     return 0;
diff --git a/src/util/virmdev.c b/src/util/virmdev.c
index 27541cf34f..a74a7d5554 100644
--- a/src/util/virmdev.c
+++ b/src/util/virmdev.c
@@ -58,10 +58,8 @@ virMediatedDeviceListDispose(void *obj);
 static int
 virMediatedOnceInit(void)
 {
-    if (!(virMediatedDeviceListClass = virClassNew(virClassForObjectLockable(),
-                                                   "virMediatedDeviceList",
-                                                   sizeof(virMediatedDeviceList),
-                                                   virMediatedDeviceListDispose)))
+    if (!(virMediatedDeviceListClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                     virMediatedDeviceList)))
         return -1;
 
     return 0;
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 1723df6b2f..c5a98d21cc 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -81,16 +81,12 @@ virObjectOnceInit(void)
                                        NULL)))
         return -1;
 
-    if (!(virObjectLockableClass = virClassNew(virObjectClass,
-                                               "virObjectLockable",
-                                               sizeof(virObjectLockable),
-                                               virObjectLockableDispose)))
+    if (!(virObjectLockableClass = VIR_CLASS_NEW(virObjectClass,
+                                                 virObjectLockable)))
         return -1;
 
-    if (!(virObjectRWLockableClass = virClassNew(virObjectClass,
-                                                 "virObjectRWLockable",
-                                                 sizeof(virObjectRWLockable),
-                                                 virObjectRWLockableDispose)))
+    if (!(virObjectRWLockableClass = VIR_CLASS_NEW(virObjectClass,
+                                                   virObjectRWLockable)))
         return -1;
 
     return 0;
diff --git a/src/util/virobject.h b/src/util/virobject.h
index ac6cf22f9e..92dd512399 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -74,6 +74,10 @@ virClassPtr virClassForObjectRWLockable(void);
 # ifndef VIR_PARENT_REQUIRED
 #  define VIR_PARENT_REQUIRED ATTRIBUTE_NONNULL(1)
 # endif
+
+# define VIR_CLASS_NEW(prnt, name) \
+    virClassNew(prnt, #name, sizeof(name), name##Dispose)
+
 virClassPtr
 virClassNew(virClassPtr parent,
             const char *name,
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 55e4c3e492..ba8449116a 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -204,10 +204,8 @@ static void virPCIDeviceListDispose(void *obj);
 
 static int virPCIOnceInit(void)
 {
-    if (!(virPCIDeviceListClass = virClassNew(virClassForObjectLockable(),
-                                              "virPCIDeviceList",
-                                              sizeof(virPCIDeviceList),
-                                              virPCIDeviceListDispose)))
+    if (!(virPCIDeviceListClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                virPCIDeviceList)))
         return -1;
 
     return 0;
diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
index 25200fbbb2..63c0be0b6e 100644
--- a/src/util/virportallocator.c
+++ b/src/util/virportallocator.c
@@ -80,10 +80,8 @@ virPortAllocatorNew(void)
 static int
 virPortAllocatorOnceInit(void)
 {
-    if (!(virPortAllocatorClass = virClassNew(virClassForObjectLockable(),
-                                              "virPortAllocator",
-                                              sizeof(virPortAllocator),
-                                              virPortAllocatorDispose)))
+    if (!(virPortAllocatorClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                virPortAllocator)))
         return -1;
 
     if (!(virPortAllocatorInstance = virPortAllocatorNew()))
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 9639e00468..d78612f521 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -133,10 +133,8 @@ virResctrlInfoDispose(void *obj)
 static int
 virResctrlInfoOnceInit(void)
 {
-    if (!(virResctrlInfoClass = virClassNew(virClassForObject(),
-                                            "virResctrlInfo",
-                                            sizeof(virResctrlInfo),
-                                            virResctrlInfoDispose)))
+    if (!(virResctrlInfoClass = VIR_CLASS_NEW(virClassForObject(),
+                                              virResctrlInfo)))
         return -1;
 
     return 0;
@@ -271,10 +269,8 @@ virResctrlAllocDispose(void *obj)
 static int
 virResctrlAllocOnceInit(void)
 {
-    if (!(virResctrlAllocClass = virClassNew(virClassForObject(),
-                                             "virResctrlAlloc",
-                                             sizeof(virResctrlAlloc),
-                                             virResctrlAllocDispose)))
+    if (!(virResctrlAllocClass = VIR_CLASS_NEW(virClassForObject(),
+                                               virResctrlAlloc)))
         return -1;
 
     return 0;
diff --git a/src/util/virscsi.c b/src/util/virscsi.c
index 22f2677a3f..c1e2eb3790 100644
--- a/src/util/virscsi.c
+++ b/src/util/virscsi.c
@@ -85,10 +85,8 @@ static void virSCSIDeviceListDispose(void *obj);
 static int
 virSCSIOnceInit(void)
 {
-    if (!(virSCSIDeviceListClass = virClassNew(virClassForObjectLockable(),
-                                               "virSCSIDeviceList",
-                                               sizeof(virSCSIDeviceList),
-                                               virSCSIDeviceListDispose)))
+    if (!(virSCSIDeviceListClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                 virSCSIDeviceList)))
         return -1;
 
     return 0;
diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c
index 5f176e177f..c28403809d 100644
--- a/src/util/virscsivhost.c
+++ b/src/util/virscsivhost.c
@@ -70,10 +70,8 @@ virSCSIVHostDeviceListDispose(void *obj)
 static int
 virSCSIVHostOnceInit(void)
 {
-    if (!(virSCSIVHostDeviceListClass = virClassNew(virClassForObjectLockable(),
-                                                    "virSCSIVHostDeviceList",
-                                                    sizeof(virSCSIVHostDeviceList),
-                                                    virSCSIVHostDeviceListDispose)))
+    if (!(virSCSIVHostDeviceListClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                      virSCSIVHostDeviceList)))
         return -1;
 
     return 0;
diff --git a/src/util/virusb.c b/src/util/virusb.c
index be7be5dc18..67a679afda 100644
--- a/src/util/virusb.c
+++ b/src/util/virusb.c
@@ -80,10 +80,8 @@ static void virUSBDeviceListDispose(void *obj);
 
 static int virUSBOnceInit(void)
 {
-    if (!(virUSBDeviceListClass = virClassNew(virClassForObjectLockable(),
-                                              "virUSBDeviceList",
-                                              sizeof(virUSBDeviceList),
-                                              virUSBDeviceListDispose)))
+    if (!(virUSBDeviceListClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                                virUSBDeviceList)))
         return -1;
 
     return 0;
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 920af4c010..2adfe0660d 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -118,10 +118,8 @@ vboxDriverDispose(void *obj)
 static int
 vboxDriverOnceInit(void)
 {
-    if (!(vboxDriverClass = virClassNew(virClassForObjectLockable(),
-                                        "vboxDriver",
-                                        sizeof(vboxDriver),
-                                        vboxDriverDispose)))
+    if (!(vboxDriverClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                          vboxDriver)))
         return -1;
 
     return 0;
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index e51d968f28..11120e6ff9 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -161,10 +161,8 @@ static void vzDriverDispose(void * obj)
 
 static int vzDriverOnceInit(void)
 {
-    if (!(vzDriverClass = virClassNew(virClassForObjectLockable(),
-                                      "vzDriver",
-                                      sizeof(vzDriver),
-                                      vzDriverDispose)))
+    if (!(vzDriverClass = VIR_CLASS_NEW(virClassForObjectLockable(),
+                                        vzDriver)))
         return -1;
 
     return 0;
diff --git a/tests/virfilecachetest.c b/tests/virfilecachetest.c
index d4a9d598ef..13724011b7 100644
--- a/tests/virfilecachetest.c
+++ b/tests/virfilecachetest.c
@@ -49,10 +49,8 @@ testFileCacheObjDispose(void *opaque)
 static int
 testFileCacheObjOnceInit(void)
 {
-    if (!(testFileCacheObjClass = virClassNew(virClassForObject(),
-                                              "testFileCacheObj",
-                                              sizeof(testFileCacheObj),
-                                              testFileCacheObjDispose)))
+    if (!(testFileCacheObjClass = VIR_CLASS_NEW(virClassForObject(),
+                                                testFileCacheObj)))
         return -1;
 
     return 0;
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/9] virobject: Introduce VIR_CLASS_NEW() macro
Posted by Erik Skultety, 13 weeks ago
On Fri, Apr 13, 2018 at 04:47:14PM +0200, Michal Privoznik wrote:
> So far we are repeating the following lines over and over:
>
>   virClassNew(virClassForObject(),
>               "virSomeObject",
>               sizeof(virSomeObject),
>               virSomeObjectDispose);
>
> While this works, it is impossible to do some checking. Firstly,
> the class name (the 2nd argument) doesn't match the name in the
> code in all cases (the 3rd argument). Secondly, the current style
> is needlessly verbose. This commit turns example into following:
>
>   VIR_CLASS_NEW(virClassForObject(),
>                 virSomeObject);
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/access/viraccessmanager.c           |   6 +-
>  src/bhyve/bhyve_conf.c                  |   6 +-
>  src/conf/capabilities.c                 |   6 +-
>  src/conf/domain_capabilities.c          |  12 +--
>  src/conf/domain_conf.c                  |  18 ++---
>  src/conf/domain_event.c                 | 126 +++++++++++---------------------
>  src/conf/network_event.c                |  12 +--
>  src/conf/node_device_event.c            |  18 ++---
>  src/conf/object_event.c                 |  12 +--
>  src/conf/secret_event.c                 |  18 ++---
>  src/conf/storage_event.c                |  18 ++---
>  src/conf/virdomainobjlist.c             |   6 +-
>  src/conf/virinterfaceobj.c              |  12 +--
>  src/conf/virnetworkobj.c                |  12 +--
>  src/conf/virnodedeviceobj.c             |  12 +--
>  src/conf/virsecretobj.c                 |  12 +--
>  src/conf/virstorageobj.c                |  24 ++----
>  src/datatypes.c                         |   6 +-
>  src/interface/interface_backend_netcf.c |   6 +-
>  src/libvirt-admin.c                     |   6 +-
>  src/libxl/libxl_conf.c                  |   6 +-
>  src/libxl/libxl_domain.c                |   6 +-
>  src/libxl/libxl_migration.c             |   6 +-
>  src/logging/log_handler.c               |   6 +-
>  src/lxc/lxc_conf.c                      |   6 +-
>  src/lxc/lxc_monitor.c                   |   6 +-
>  src/node_device/node_device_udev.c      |   6 +-
>  src/qemu/qemu_agent.c                   |   6 +-
>  src/qemu/qemu_capabilities.c            |   6 +-
>  src/qemu/qemu_conf.c                    |   6 +-
>  src/qemu/qemu_domain.c                  |  36 +++------
>  src/qemu/qemu_monitor.c                 |   6 +-
>  src/rpc/virkeepalive.c                  |   6 +-
>  src/rpc/virnetclient.c                  |   6 +-
>  src/rpc/virnetclientprogram.c           |   6 +-
>  src/rpc/virnetclientstream.c            |   6 +-
>  src/rpc/virnetdaemon.c                  |   6 +-
>  src/rpc/virnetlibsshsession.c           |   6 +-
>  src/rpc/virnetsaslcontext.c             |  12 +--
>  src/rpc/virnetserver.c                  |   6 +-
>  src/rpc/virnetserverclient.c            |   6 +-
>  src/rpc/virnetserverprogram.c           |   6 +-
>  src/rpc/virnetserverservice.c           |   6 +-
>  src/rpc/virnetsocket.c                  |   6 +-
>  src/rpc/virnetsshsession.c              |   6 +-
>  src/rpc/virnettlscontext.c              |  12 +--
>  src/security/security_manager.c         |   6 +-
>  src/util/virclosecallbacks.c            |   6 +-
>  src/util/virdnsmasq.c                   |   7 +-
>  src/util/virfdstream.c                  |   6 +-
>  src/util/virfilecache.c                 |   6 +-
>  src/util/virhash.c                      |   6 +-
>  src/util/virhostdev.c                   |   6 +-
>  src/util/viridentity.c                  |   6 +-
>  src/util/virmacmap.c                    |   6 +-
>  src/util/virmdev.c                      |   6 +-
>  src/util/virobject.c                    |  12 +--
>  src/util/virobject.h                    |   4 +
>  src/util/virpci.c                       |   6 +-
>  src/util/virportallocator.c             |   6 +-
>  src/util/virresctrl.c                   |  12 +--
>  src/util/virscsi.c                      |   6 +-
>  src/util/virscsivhost.c                 |   6 +-
>  src/util/virusb.c                       |   6 +-
>  src/vbox/vbox_common.c                  |   6 +-
>  src/vz/vz_driver.c                      |   6 +-
>  tests/virfilecachetest.c                |   6 +-

...

> diff --git a/src/datatypes.c b/src/datatypes.c
> index 0c3c66a9ce..3016e45076 100644
> --- a/src/datatypes.c
> +++ b/src/datatypes.c
> @@ -74,10 +74,8 @@ static int
>  virDataTypesOnceInit(void)
>  {
>  #define DECLARE_CLASS_COMMON(basename, parent) \
> -    if (!(basename ## Class = virClassNew(parent, \
> -                                          #basename, \
> -                                          sizeof(basename), \
> -                                          basename ## Dispose))) \
> +    if (!(basename ## Class = VIR_CLASS_NEW(parent, \
> +                                            basename))) \
>          return -1;

I'd probably go one step further and move the DECLARE_CLASS macros in a widely
accessible header and use that instead of repeating the following:

if (!(virSomethingClass = VIR_CLASS_NEW(virObject(Lockable)?, virSomething))
    return -1;

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/9] virobject: Introduce VIR_CLASS_NEW() macro
Posted by Michal Privoznik, 13 weeks ago
On 04/16/2018 11:52 AM, Erik Skultety wrote:
> On Fri, Apr 13, 2018 at 04:47:14PM +0200, Michal Privoznik wrote:
>> So far we are repeating the following lines over and over:
>>
>>   virClassNew(virClassForObject(),
>>               "virSomeObject",
>>               sizeof(virSomeObject),
>>               virSomeObjectDispose);
>>
>> While this works, it is impossible to do some checking. Firstly,
>> the class name (the 2nd argument) doesn't match the name in the
>> code in all cases (the 3rd argument). Secondly, the current style
>> is needlessly verbose. This commit turns example into following:
>>
>>   VIR_CLASS_NEW(virClassForObject(),
>>                 virSomeObject);
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/access/viraccessmanager.c           |   6 +-
>>  src/bhyve/bhyve_conf.c                  |   6 +-
>>  src/conf/capabilities.c                 |   6 +-
>>  src/conf/domain_capabilities.c          |  12 +--
>>  src/conf/domain_conf.c                  |  18 ++---
>>  src/conf/domain_event.c                 | 126 +++++++++++---------------------
>>  src/conf/network_event.c                |  12 +--
>>  src/conf/node_device_event.c            |  18 ++---
>>  src/conf/object_event.c                 |  12 +--
>>  src/conf/secret_event.c                 |  18 ++---
>>  src/conf/storage_event.c                |  18 ++---
>>  src/conf/virdomainobjlist.c             |   6 +-
>>  src/conf/virinterfaceobj.c              |  12 +--
>>  src/conf/virnetworkobj.c                |  12 +--
>>  src/conf/virnodedeviceobj.c             |  12 +--
>>  src/conf/virsecretobj.c                 |  12 +--
>>  src/conf/virstorageobj.c                |  24 ++----
>>  src/datatypes.c                         |   6 +-
>>  src/interface/interface_backend_netcf.c |   6 +-
>>  src/libvirt-admin.c                     |   6 +-
>>  src/libxl/libxl_conf.c                  |   6 +-
>>  src/libxl/libxl_domain.c                |   6 +-
>>  src/libxl/libxl_migration.c             |   6 +-
>>  src/logging/log_handler.c               |   6 +-
>>  src/lxc/lxc_conf.c                      |   6 +-
>>  src/lxc/lxc_monitor.c                   |   6 +-
>>  src/node_device/node_device_udev.c      |   6 +-
>>  src/qemu/qemu_agent.c                   |   6 +-
>>  src/qemu/qemu_capabilities.c            |   6 +-
>>  src/qemu/qemu_conf.c                    |   6 +-
>>  src/qemu/qemu_domain.c                  |  36 +++------
>>  src/qemu/qemu_monitor.c                 |   6 +-
>>  src/rpc/virkeepalive.c                  |   6 +-
>>  src/rpc/virnetclient.c                  |   6 +-
>>  src/rpc/virnetclientprogram.c           |   6 +-
>>  src/rpc/virnetclientstream.c            |   6 +-
>>  src/rpc/virnetdaemon.c                  |   6 +-
>>  src/rpc/virnetlibsshsession.c           |   6 +-
>>  src/rpc/virnetsaslcontext.c             |  12 +--
>>  src/rpc/virnetserver.c                  |   6 +-
>>  src/rpc/virnetserverclient.c            |   6 +-
>>  src/rpc/virnetserverprogram.c           |   6 +-
>>  src/rpc/virnetserverservice.c           |   6 +-
>>  src/rpc/virnetsocket.c                  |   6 +-
>>  src/rpc/virnetsshsession.c              |   6 +-
>>  src/rpc/virnettlscontext.c              |  12 +--
>>  src/security/security_manager.c         |   6 +-
>>  src/util/virclosecallbacks.c            |   6 +-
>>  src/util/virdnsmasq.c                   |   7 +-
>>  src/util/virfdstream.c                  |   6 +-
>>  src/util/virfilecache.c                 |   6 +-
>>  src/util/virhash.c                      |   6 +-
>>  src/util/virhostdev.c                   |   6 +-
>>  src/util/viridentity.c                  |   6 +-
>>  src/util/virmacmap.c                    |   6 +-
>>  src/util/virmdev.c                      |   6 +-
>>  src/util/virobject.c                    |  12 +--
>>  src/util/virobject.h                    |   4 +
>>  src/util/virpci.c                       |   6 +-
>>  src/util/virportallocator.c             |   6 +-
>>  src/util/virresctrl.c                   |  12 +--
>>  src/util/virscsi.c                      |   6 +-
>>  src/util/virscsivhost.c                 |   6 +-
>>  src/util/virusb.c                       |   6 +-
>>  src/vbox/vbox_common.c                  |   6 +-
>>  src/vz/vz_driver.c                      |   6 +-
>>  tests/virfilecachetest.c                |   6 +-
> 
> ...
> 
>> diff --git a/src/datatypes.c b/src/datatypes.c
>> index 0c3c66a9ce..3016e45076 100644
>> --- a/src/datatypes.c
>> +++ b/src/datatypes.c
>> @@ -74,10 +74,8 @@ static int
>>  virDataTypesOnceInit(void)
>>  {
>>  #define DECLARE_CLASS_COMMON(basename, parent) \
>> -    if (!(basename ## Class = virClassNew(parent, \
>> -                                          #basename, \
>> -                                          sizeof(basename), \
>> -                                          basename ## Dispose))) \
>> +    if (!(basename ## Class = VIR_CLASS_NEW(parent, \
>> +                                            basename))) \
>>          return -1;
> 
> I'd probably go one step further and move the DECLARE_CLASS macros in a widely
> accessible header and use that instead of repeating the following:
> 
> if (!(virSomethingClass = VIR_CLASS_NEW(virObject(Lockable)?, virSomething))
>     return -1;

Well, it's not that simple. There are multiple classes new ones are
derived from. We will still need two arguments to the macro. But it can
help us unify class variable names. Okay, I'll cook patch like that.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/9] virobject: Introduce VIR_CLASS_NEW() macro
Posted by Erik Skultety, 13 weeks ago
On Fri, Apr 13, 2018 at 04:47:14PM +0200, Michal Privoznik wrote:
> So far we are repeating the following lines over and over:
>
>   virClassNew(virClassForObject(),
>               "virSomeObject",
>               sizeof(virSomeObject),
>               virSomeObjectDispose);
>
> While this works, it is impossible to do some checking. Firstly,
> the class name (the 2nd argument) doesn't match the name in the
> code in all cases (the 3rd argument). Secondly, the current style
> is needlessly verbose. This commit turns example into following:
>
>   VIR_CLASS_NEW(virClassForObject(),
>                 virSomeObject);
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

...

> diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c
> index cc2402febb..8acccf8940 100644
> --- a/src/interface/interface_backend_netcf.c
> +++ b/src/interface/interface_backend_netcf.c
> @@ -55,10 +55,8 @@ static void virNetcfDriverStateDispose(void *obj);
>  static int
>  virNetcfDriverStateOnceInit(void)
>  {
> -    if (!(virNetcfDriverStateClass = virClassNew(virClassForObjectLockable(),
> -                                       "virNetcfDriverState",
> -                                       sizeof(virNetcfDriverState),
> -                                       virNetcfDriverStateDispose)))
> +    if (!(virNetcfDriverStateClass = VIR_CLASS_NEW(virClassForObjectLockable(),
> +                                                   virNetcfDriverStat)))

There's a typo in the 2nd parameter which makes the patch fail the compilation.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/9] virobject: Check if @parent is the first member in class
Posted by Michal Privoznik, 14 weeks ago
Our virObject code relies heavily on the fact that the first
member of the class struct is type of virObject (or some
derivation of if). Let's check for that.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/virobject.c | 31 +++++++++++++++++++++----------
 src/util/virobject.h |  5 ++++-
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/src/util/virobject.c b/src/util/virobject.c
index c5a98d21cc..e184f5349e 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -77,6 +77,7 @@ virObjectOnceInit(void)
 {
     if (!(virObjectClass = virClassNew(NULL,
                                        "virObject",
+                                       0,
                                        sizeof(virObject),
                                        NULL)))
         return -1;
@@ -159,21 +160,31 @@ virClassForObjectRWLockable(void)
 virClassPtr
 virClassNew(virClassPtr parent,
             const char *name,
+            size_t off,
             size_t objectSize,
             virObjectDisposeCallback dispose)
 {
     virClassPtr klass;
 
-    if (parent == NULL &&
-        STRNEQ(name, "virObject")) {
-        virReportInvalidNonNullArg(parent);
-        return NULL;
-    } else if (parent &&
-               objectSize <= parent->objectSize) {
-        virReportInvalidArg(objectSize,
-                            _("object size %zu of %s is smaller than parent class %zu"),
-                            objectSize, name, parent->objectSize);
-        return NULL;
+    if (parent == NULL) {
+        if (STRNEQ(name, "virObject")) {
+            virReportInvalidNonNullArg(parent);
+            return NULL;
+        }
+    } else {
+        if (objectSize <= parent->objectSize) {
+            virReportInvalidArg(objectSize,
+                                _("object size %zu of %s is smaller than parent class %zu"),
+                                objectSize, name, parent->objectSize);
+            return NULL;
+        }
+
+        if (off) {
+            virReportInvalidArg(off,
+                                _("struct %s doesn't have 'parent' as its first member"),
+                                name);
+            return NULL;
+        }
     }
 
     if (VIR_ALLOC(klass) < 0)
diff --git a/src/util/virobject.h b/src/util/virobject.h
index 92dd512399..25db3a0c22 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -76,11 +76,14 @@ virClassPtr virClassForObjectRWLockable(void);
 # endif
 
 # define VIR_CLASS_NEW(prnt, name) \
-    virClassNew(prnt, #name, sizeof(name), name##Dispose)
+    virClassNew(prnt, #name, \
+                offsetof(name, parent), \
+                sizeof(name), name##Dispose)
 
 virClassPtr
 virClassNew(virClassPtr parent,
             const char *name,
+            size_t off,
             size_t objectSize,
             virObjectDisposeCallback dispose)
     VIR_PARENT_REQUIRED ATTRIBUTE_NONNULL(2);
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 8/9] virobject: Check if @parent is the first member in class
Posted by Erik Skultety, 13 weeks ago
On Fri, Apr 13, 2018 at 04:47:15PM +0200, Michal Privoznik wrote:
> Our virObject code relies heavily on the fact that the first
> member of the class struct is type of virObject (or some
> derivation of if). Let's check for that.

If a class is missing 'parent' memeber, it's a bug in the definition of the
struct/class, therefore there should be a static assertion rather than a
runtime check.

>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/util/virobject.c | 31 +++++++++++++++++++++----------
>  src/util/virobject.h |  5 ++++-
>  2 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/src/util/virobject.c b/src/util/virobject.c
> index c5a98d21cc..e184f5349e 100644
> --- a/src/util/virobject.c
> +++ b/src/util/virobject.c
> @@ -77,6 +77,7 @@ virObjectOnceInit(void)
>  {
>      if (!(virObjectClass = virClassNew(NULL,
>                                         "virObject",
> +                                       0,
>                                         sizeof(virObject),
>                                         NULL)))

Also, I don't like this extra parameter, which really shouldn't be needed; you
created a macro which hides this parameter, but that doesn't mean that
design-wise it makes sense to have it there, think of it as a constructor, you
don't pass a constructor an offset of the class' member, because it shouldn't
have need for it, but you do, solely for the purpose of checking whether we have
a particular member in place.
So, to start a discussion about this (I also think Dan posted something related
to this recently, but I don't seem to be able to find it in the archives - do I
even archive?!!!), I came up with my first compile-time hack ever, it seems to
work like expected, but I'd like to hear your opinions both the macro itself
and the approach we're going to take, so here's my replacement patch:

diff --git a/src/util/virobject.h b/src/util/virobject.h
index 92dd51239..2a973d401 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -75,8 +75,12 @@ virClassPtr virClassForObjectRWLockable(void);
 #  define VIR_PARENT_REQUIRED ATTRIBUTE_NONNULL(1)
 # endif

+# define VIR_CLASS_HAS_PARENT(name) \
+        !sizeof(char[0-offsetof(name, parent)])
+
 # define VIR_CLASS_NEW(prnt, name) \
-    virClassNew(prnt, #name, sizeof(name), name##Dispose)
+    VIR_CLASS_HAS_PARENT(name) ? \
+        virClassNew(prnt, #name, sizeof(name), name##Dispose) : NULL

Notes:
 - I suppose mingw would handle this hack the same way it handles
   VIR_TYPEMATCH, IOW it works...

 - it also doesn't need to be a ternary, I suggested extending VIR_CLASS_NEW to
   do the complete assignment in [Patch 7/9], like this:

# define VIR_CLASS_NEW(prnt, name) \
    if (!(name##Class) = virClassNew(prnt, #name, sizeof(name), name##Dispose))
        return -1;

Regards,
Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 8/9] virobject: Check if @parent is the first member in class
Posted by Daniel P. Berrangé, 13 weeks ago
On Mon, Apr 16, 2018 at 02:30:40PM +0200, Erik Skultety wrote:
> On Fri, Apr 13, 2018 at 04:47:15PM +0200, Michal Privoznik wrote:
> > Our virObject code relies heavily on the fact that the first
> > member of the class struct is type of virObject (or some
> > derivation of if). Let's check for that.
> 
> If a class is missing 'parent' memeber, it's a bug in the definition of the
> struct/class, therefore there should be a static assertion rather than a
> runtime check.

Agreed, my suggestion was for a static assert too.

> 
> >
> > Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> > ---
> >  src/util/virobject.c | 31 +++++++++++++++++++++----------
> >  src/util/virobject.h |  5 ++++-
> >  2 files changed, 25 insertions(+), 11 deletions(-)
> >
> > diff --git a/src/util/virobject.c b/src/util/virobject.c
> > index c5a98d21cc..e184f5349e 100644
> > --- a/src/util/virobject.c
> > +++ b/src/util/virobject.c
> > @@ -77,6 +77,7 @@ virObjectOnceInit(void)
> >  {
> >      if (!(virObjectClass = virClassNew(NULL,
> >                                         "virObject",
> > +                                       0,
> >                                         sizeof(virObject),
> >                                         NULL)))
> 
> Also, I don't like this extra parameter, which really shouldn't be needed; you
> created a macro which hides this parameter, but that doesn't mean that
> design-wise it makes sense to have it there, think of it as a constructor, you
> don't pass a constructor an offset of the class' member, because it shouldn't
> have need for it, but you do, solely for the purpose of checking whether we have
> a particular member in place.
> So, to start a discussion about this (I also think Dan posted something related
> to this recently, but I don't seem to be able to find it in the archives - do I
> even archive?!!!), I came up with my first compile-time hack ever, it seems to
> work like expected, but I'd like to hear your opinions both the macro itself
> and the approach we're going to take, so here's my replacement patch:

https://www.redhat.com/archives/libvir-list/2018-April/msg00984.html

I had suggested something in the virObjectNew function:

  #define virObjectNew(typ) \
      (typ *)(&((typ *)virObjectNewImpl(typ # Class)).parent)

catching it with virClassNew works fine too, as it would be a compile
time check too

> diff --git a/src/util/virobject.h b/src/util/virobject.h
> index 92dd51239..2a973d401 100644
> --- a/src/util/virobject.h
> +++ b/src/util/virobject.h
> @@ -75,8 +75,12 @@ virClassPtr virClassForObjectRWLockable(void);
>  #  define VIR_PARENT_REQUIRED ATTRIBUTE_NONNULL(1)
>  # endif
> 
> +# define VIR_CLASS_HAS_PARENT(name) \
> +        !sizeof(char[0-offsetof(name, parent)])
> +
>  # define VIR_CLASS_NEW(prnt, name) \
> -    virClassNew(prnt, #name, sizeof(name), name##Dispose)
> +    VIR_CLASS_HAS_PARENT(name) ? \
> +        virClassNew(prnt, #name, sizeof(name), name##Dispose) : NULL

So we're relying on the fact the the ': NULL" will never execute
because VIR_CLASS_HAS_PARENT will trigger a compile time error.

> 
> Notes:
>  - I suppose mingw would handle this hack the same way it handles
>    VIR_TYPEMATCH, IOW it works...
> 
>  - it also doesn't need to be a ternary, I suggested extending VIR_CLASS_NEW to
>    do the complete assignment in [Patch 7/9], like this:
> 
> # define VIR_CLASS_NEW(prnt, name) \
>     if (!(name##Class) = virClassNew(prnt, #name, sizeof(name), name##Dispose))
>         return -1;

This has the added benefit of enforcing class variable naming scheme
which removes another source of developer error, and is in keeping
with VIR_ALLOC() style.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 8/9] virobject: Check if @parent is the first member in class
Posted by Michal Privoznik, 13 weeks ago
On 04/16/2018 02:30 PM, Erik Skultety wrote:
> On Fri, Apr 13, 2018 at 04:47:15PM +0200, Michal Privoznik wrote:
>> Our virObject code relies heavily on the fact that the first
>> member of the class struct is type of virObject (or some
>> derivation of if). Let's check for that.
> 
> If a class is missing 'parent' memeber, it's a bug in the definition of the
> struct/class, therefore there should be a static assertion rather than a
> runtime check.

If a class is missing parent then you'd hit compile time error because
offsetof() is trying to get offset of a non-existent member.

> 
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/util/virobject.c | 31 +++++++++++++++++++++----------
>>  src/util/virobject.h |  5 ++++-
>>  2 files changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/util/virobject.c b/src/util/virobject.c
>> index c5a98d21cc..e184f5349e 100644
>> --- a/src/util/virobject.c
>> +++ b/src/util/virobject.c
>> @@ -77,6 +77,7 @@ virObjectOnceInit(void)
>>  {
>>      if (!(virObjectClass = virClassNew(NULL,
>>                                         "virObject",
>> +                                       0,
>>                                         sizeof(virObject),
>>                                         NULL)))
> 
> Also, I don't like this extra parameter, which really shouldn't be needed; you
> created a macro which hides this parameter, but that doesn't mean that
> design-wise it makes sense to have it there, think of it as a constructor, you
> don't pass a constructor an offset of the class' member, because it shouldn't
> have need for it, but you do, solely for the purpose of checking whether we have
> a particular member in place.
> So, to start a discussion about this (I also think Dan posted something related
> to this recently, but I don't seem to be able to find it in the archives - do I
> even archive?!!!), I came up with my first compile-time hack ever, it seems to
> work like expected, but I'd like to hear your opinions both the macro itself
> and the approach we're going to take, so here's my replacement patch:
> 
> diff --git a/src/util/virobject.h b/src/util/virobject.h
> index 92dd51239..2a973d401 100644
> --- a/src/util/virobject.h
> +++ b/src/util/virobject.h
> @@ -75,8 +75,12 @@ virClassPtr virClassForObjectRWLockable(void);
>  #  define VIR_PARENT_REQUIRED ATTRIBUTE_NONNULL(1)
>  # endif
> 
> +# define VIR_CLASS_HAS_PARENT(name) \
> +        !sizeof(char[0-offsetof(name, parent)])

I don't quite understand why this is so obfuscated. Anyway, since
VIR_CLASS_NEW() is going to be a stand alone macro (like VIR_ENUM_DECL
for instance) we can do plain:

#define VIR_CLASS_NEW(prt, name) \
  verify(offsetof(name, parent) == 0); \
  if (!(name##Class = virClassNew(prt, #name, sizeof(name), name##Dispose))) \
    return -1;

(written from the top of my head, not tested, not compiled, don't take
it too much literally)

We couldn't do that if VIR_CLASS_NEW() is still a function-like macro
( if (!(nameClass = VIR_CLASS_NEW(...))) return -1; ).

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 8/9] virobject: Check if @parent is the first member in class
Posted by Erik Skultety, 13 weeks ago
On Mon, Apr 16, 2018 at 06:27:45PM +0200, Michal Privoznik wrote:
> On 04/16/2018 02:30 PM, Erik Skultety wrote:
> > On Fri, Apr 13, 2018 at 04:47:15PM +0200, Michal Privoznik wrote:
> >> Our virObject code relies heavily on the fact that the first
> >> member of the class struct is type of virObject (or some
> >> derivation of if). Let's check for that.
> >
> > If a class is missing 'parent' memeber, it's a bug in the definition of the
> > struct/class, therefore there should be a static assertion rather than a
> > runtime check.
>
> If a class is missing parent then you'd hit compile time error because
> offsetof() is trying to get offset of a non-existent member.

Sigh, poor choice of words, you're right, I meant the scenario where you put it
somewhere else...

>
> >
> >>
> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> >> ---
> >>  src/util/virobject.c | 31 +++++++++++++++++++++----------
> >>  src/util/virobject.h |  5 ++++-
> >>  2 files changed, 25 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/src/util/virobject.c b/src/util/virobject.c
> >> index c5a98d21cc..e184f5349e 100644
> >> --- a/src/util/virobject.c
> >> +++ b/src/util/virobject.c
> >> @@ -77,6 +77,7 @@ virObjectOnceInit(void)
> >>  {
> >>      if (!(virObjectClass = virClassNew(NULL,
> >>                                         "virObject",
> >> +                                       0,
> >>                                         sizeof(virObject),
> >>                                         NULL)))
> >
> > Also, I don't like this extra parameter, which really shouldn't be needed; you
> > created a macro which hides this parameter, but that doesn't mean that
> > design-wise it makes sense to have it there, think of it as a constructor, you
> > don't pass a constructor an offset of the class' member, because it shouldn't
> > have need for it, but you do, solely for the purpose of checking whether we have
> > a particular member in place.
> > So, to start a discussion about this (I also think Dan posted something related
> > to this recently, but I don't seem to be able to find it in the archives - do I
> > even archive?!!!), I came up with my first compile-time hack ever, it seems to
> > work like expected, but I'd like to hear your opinions both the macro itself
> > and the approach we're going to take, so here's my replacement patch:
> >
> > diff --git a/src/util/virobject.h b/src/util/virobject.h
> > index 92dd51239..2a973d401 100644
> > --- a/src/util/virobject.h
> > +++ b/src/util/virobject.h
> > @@ -75,8 +75,12 @@ virClassPtr virClassForObjectRWLockable(void);
> >  #  define VIR_PARENT_REQUIRED ATTRIBUTE_NONNULL(1)
> >  # endif
> >
> > +# define VIR_CLASS_HAS_PARENT(name) \
> > +        !sizeof(char[0-offsetof(name, parent)])
>
> I don't quite understand why this is so obfuscated. Anyway, since
> VIR_CLASS_NEW() is going to be a stand alone macro (like VIR_ENUM_DECL
> for instance) we can do plain:

Well, this was to accommodate the macro to the original form of having it
behave function-like. But as I said, if we adjust 7/9, we have other options as
well and frankly, I like the usage of verify below, which I didn't know gnulib
had.

>
> #define VIR_CLASS_NEW(prt, name) \
>   verify(offsetof(name, parent) == 0); \
>   if (!(name##Class = virClassNew(prt, #name, sizeof(name), name##Dispose))) \
>     return -1;
>
> (written from the top of my head, not tested, not compiled, don't take
> it too much literally)

I did and it works. With the above change:
Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 9/9] cfg.mk: Introduce syntax-check rule to prefer VIR_CLASS_NEW
Posted by Michal Privoznik, 14 weeks ago
Now that we have macro that does some checks lets forbid raw
usage of virClassNew() in favor of VIR_CLASS_NEW().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 cfg.mk | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/cfg.mk b/cfg.mk
index 4078bc2c63..5b7a9728d2 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -321,6 +321,11 @@ sc_prohibit_internal_functions:
 	halt='use VIR_ macros instead of internal functions' \
 	  $(_sc_search_regexp)
 
+sc_prohibit_raw_virclassnew:
+	@prohibit='virClassNew *\(' \
+	halt='use VIR_CLASS_NEW macro instead of virClassNew' \
+	  $(_sc_search_regexp)
+
 # Avoid raw malloc and free, except in documentation comments.
 sc_prohibit_raw_allocation:
 	@prohibit='^.[^*].*\<((m|c|re)alloc|free) *\([^)]' \
@@ -1188,6 +1193,9 @@ exclude_file_name_regexp--sc_prohibit_gethostname = ^src/util/vir(util|log)\.c$$
 exclude_file_name_regexp--sc_prohibit_internal_functions = \
   ^src/(util/(viralloc|virutil|virfile)\.[hc]|esx/esx_vi\.c)$$
 
+exclude_file_name_regexp--sc_prohibit_raw_virclassnew = \
+  ^src/util/virobject\.[hc]$$
+
 exclude_file_name_regexp--sc_prohibit_newline_at_end_of_diagnostic = \
   ^src/rpc/gendispatch\.pl$$
 
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 9/9] cfg.mk: Introduce syntax-check rule to prefer VIR_CLASS_NEW
Posted by Erik Skultety, 13 weeks ago
On Fri, Apr 13, 2018 at 04:47:16PM +0200, Michal Privoznik wrote:
> Now that we have macro that does some checks lets forbid raw
> usage of virClassNew() in favor of VIR_CLASS_NEW().
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  cfg.mk | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/cfg.mk b/cfg.mk
> index 4078bc2c63..5b7a9728d2 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -321,6 +321,11 @@ sc_prohibit_internal_functions:
>  	halt='use VIR_ macros instead of internal functions' \
>  	  $(_sc_search_regexp)
>
> +sc_prohibit_raw_virclassnew:
> +	@prohibit='virClassNew *\(' \
> +	halt='use VIR_CLASS_NEW macro instead of virClassNew' \

s/macro//

Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list