[RFC PATCH 04/16] qemu: monitor: Rework qemuBlockStats into a g_object

Peter Krempa via Devel posted 16 patches 2 weeks, 4 days ago
[RFC PATCH 04/16] qemu: monitor: Rework qemuBlockStats into a g_object
Posted by Peter Krempa via Devel 2 weeks, 4 days ago
From: Peter Krempa <pkrempa@redhat.com>

Create the g_object boilerplate and store references in the hash table
instead of copies.

This will simplify upcoming code which will add allocated fields into
qemuBlockStats.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_driver.c       | 32 ++++++-----------------
 src/qemu/qemu_monitor.c      | 38 ++++++++++++++++++++++++++-
 src/qemu/qemu_monitor.h      |  7 ++++-
 src/qemu/qemu_monitor_json.c | 50 ++++++++----------------------------
 4 files changed, 62 insertions(+), 65 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ac72ea5cb0..b6d83f1dc6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9613,7 +9613,6 @@ qemuDomainBlocksStatsGather(virDomainObj *vm,
     qemuDomainObjPrivate *priv = vm->privateData;
     virDomainDiskDef *disk = NULL;
     g_autoptr(GHashTable) blockstats = NULL;
-    qemuBlockStats *stats;
     size_t i;
     int nstats;
     int rc = 0;
@@ -9654,29 +9653,15 @@ qemuDomainBlocksStatsGather(virDomainObj *vm,
     if (nstats < 0 || rc < 0)
         return -1;

-    *retstats = g_new0(qemuBlockStats, 1);
-
     if (entryname) {
-        qemuBlockStats *capstats;
-
-        if (!(stats = virHashLookup(blockstats, entryname))) {
+        if (!(*retstats = virHashSteal(blockstats, entryname))) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("cannot find statistics for device '%1$s'"), entryname);
             return -1;
         }
-
-        **retstats = *stats;
-
-        /* capacity are reported only per node-name so we need to transfer them */
-        if (disk && disk->src &&
-            (capstats = virHashLookup(blockstats, qemuBlockStorageSourceGetEffectiveNodename(disk->src)))) {
-            (*retstats)->capacity = capstats->capacity;
-            (*retstats)->physical = capstats->physical;
-            (*retstats)->wr_highest_offset = capstats->wr_highest_offset;
-            (*retstats)->wr_highest_offset_valid = capstats->wr_highest_offset_valid;
-            (*retstats)->write_threshold = capstats->write_threshold;
-        }
     } else {
+        g_autoptr(qemuBlockStats) stats = qemuBlockStatsNew();
+
         for (i = 0; i < vm->def->ndisks; i++) {
             disk = vm->def->disks[i];
             entryname = disk->info.alias;
@@ -9699,6 +9684,8 @@ qemuDomainBlocksStatsGather(virDomainObj *vm,

             qemuDomainBlockStatsGatherTotals(stats, *retstats);
         }
+
+        *retstats = g_steal_pointer(&stats);
     }

     return nstats;
@@ -9710,7 +9697,7 @@ qemuDomainBlockStats(virDomainPtr dom,
                      const char *path,
                      virDomainBlockStatsPtr stats)
 {
-    qemuBlockStats *blockstats = NULL;
+    g_autoptr(qemuBlockStats) blockstats = NULL;
     int ret = -1;
     virDomainObj *vm;

@@ -9747,7 +9734,6 @@ qemuDomainBlockStats(virDomainPtr dom,

  cleanup:
     virDomainObjEndAPI(&vm);
-    VIR_FREE(blockstats);
     return ret;
 }

@@ -9760,7 +9746,7 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
                           unsigned int flags)
 {
     virDomainObj *vm;
-    qemuBlockStats *blockstats = NULL;
+    g_autoptr(qemuBlockStats) blockstats = NULL;
     int nstats;
     int ret = -1;

@@ -9833,7 +9819,6 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
     virDomainObjEndJob(vm);

  cleanup:
-    VIR_FREE(blockstats);
     virDomainObjEndAPI(&vm);
     return ret;
 }
@@ -10565,7 +10550,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
     int ret = -1;
     virDomainDiskDef *disk;
     g_autoptr(virQEMUDriverConfig) cfg = NULL;
-    qemuBlockStats *entry = NULL;
+    g_autoptr(qemuBlockStats) entry = NULL;

     virCheckFlags(0, -1);

@@ -10663,7 +10648,6 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
  endjob:
     virDomainObjEndJob(vm);
  cleanup:
-    VIR_FREE(entry);
     virDomainObjEndAPI(&vm);
     return ret;
 }
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 7b09792c5d..ce6220fbfb 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1967,6 +1967,42 @@ qemuMonitorGetBlockInfo(qemuMonitor *mon)
 }


+G_DEFINE_TYPE(qemuBlockStats, qemu_block_stats, G_TYPE_OBJECT);
+
+static void
+qemu_block_stats_init(qemuBlockStats *stats G_GNUC_UNUSED)
+{
+}
+
+
+static void
+qemuBlockStatsFinalize(GObject *object)
+{
+    qemuBlockStats *stats = QEMU_BLOCK_STATS(object);
+
+    if (!stats)
+        return;
+
+    G_OBJECT_CLASS(qemu_block_stats_parent_class)->finalize(object);
+}
+
+
+static void
+qemu_block_stats_class_init(qemuBlockStatsClass *klass)
+{
+    GObjectClass *obj = G_OBJECT_CLASS(klass);
+
+    obj->finalize = qemuBlockStatsFinalize;
+}
+
+
+qemuBlockStats *
+qemuBlockStatsNew(void)
+{
+    return g_object_new(qemu_block_stats_get_type(), NULL);
+}
+
+
 /**
  * qemuMonitorGetAllBlockStatsInfo:
  * @mon: monitor object
@@ -1982,7 +2018,7 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
                                 GHashTable **ret_stats)
 {
     int ret;
-    g_autoptr(GHashTable) stats = virHashNew(g_free);
+    g_autoptr(GHashTable) stats = virHashNew(g_object_unref);

     QEMU_CHECK_MONITOR(mon);

diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 82354a83d9..4f94edb05f 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -789,8 +789,9 @@ qemuMonitorBlockIOStatusToError(const char *status);
 GHashTable *
 qemuMonitorGetBlockInfo(qemuMonitor *mon);

-typedef struct _qemuBlockStats qemuBlockStats;
 struct _qemuBlockStats {
+    GObject parent;
+
     unsigned long long rd_req;
     unsigned long long rd_bytes;
     unsigned long long wr_req;
@@ -810,6 +811,10 @@ struct _qemuBlockStats {
     /* write_threshold is valid only if it's non-zero, conforming to qemu semantics */
     unsigned long long write_threshold;
 };
+G_DECLARE_FINAL_TYPE(qemuBlockStats, qemu_block_stats, QEMU, BLOCK_STATS, GObject);
+
+qemuBlockStats *
+qemuBlockStatsNew(void);

 int
 qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 2dbaa7a798..7b7d446b3a 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2390,7 +2390,7 @@ static qemuBlockStats *
 qemuMonitorJSONBlockStatsCollectData(virJSONValue *dev,
                                      int *nstats)
 {
-    g_autofree qemuBlockStats *bstats = NULL;
+    g_autoptr(qemuBlockStats) bstats = NULL;
     virJSONValue *parent;
     virJSONValue *parentstats;
     virJSONValue *stats;
@@ -2401,7 +2401,7 @@ qemuMonitorJSONBlockStatsCollectData(virJSONValue *dev,
         return NULL;
     }

-    bstats = g_new0(qemuBlockStats, 1);
+    bstats = qemuBlockStatsNew();

 #define QEMU_MONITOR_BLOCK_STAT_GET(NAME, VAR, MANDATORY) \
     if (MANDATORY || virJSONValueObjectHasKey(stats, NAME)) { \
@@ -2433,34 +2433,13 @@ qemuMonitorJSONBlockStatsCollectData(virJSONValue *dev,
 }


-static int
-qemuMonitorJSONAddOneBlockStatsInfo(qemuBlockStats *bstats,
-                                    const char *name,
-                                    GHashTable *stats)
-{
-    qemuBlockStats *copy = NULL;
-
-    copy = g_new0(qemuBlockStats, 1);
-
-    if (bstats)
-        *copy = *bstats;
-
-    if (virHashAddEntry(stats, name, copy) < 0) {
-        VIR_FREE(copy);
-        return -1;
-    }
-
-    return 0;
-}
-
-
 static int
 qemuMonitorJSONGetOneBlockStatsInfo(virJSONValue *dev,
                                     const char *dev_name,
                                     int depth,
                                     GHashTable *hash)
 {
-    g_autofree qemuBlockStats *bstats = NULL;
+    g_autoptr(qemuBlockStats) bstats = NULL;
     int nstats = 0;
     const char *qdevname = NULL;
     const char *nodename = NULL;
@@ -2482,17 +2461,14 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValue *dev,
     if (!(bstats = qemuMonitorJSONBlockStatsCollectData(dev, &nstats)))
         return -1;

-    if (devicename &&
-        qemuMonitorJSONAddOneBlockStatsInfo(bstats, devicename, hash) < 0)
-        return -1;
+    if (devicename)
+        g_hash_table_insert(hash, g_strdup(devicename), g_object_ref(bstats));

-    if (qdevname && STRNEQ_NULLABLE(qdevname, devicename) &&
-        qemuMonitorJSONAddOneBlockStatsInfo(bstats, qdevname, hash) < 0)
-        return -1;
+    if (qdevname && STRNEQ_NULLABLE(qdevname, devicename))
+        g_hash_table_insert(hash, g_strdup(qdevname), g_object_ref(bstats));

-    if (nodename &&
-        qemuMonitorJSONAddOneBlockStatsInfo(bstats, nodename, hash) < 0)
-        return -1;
+    if (nodename)
+        g_hash_table_insert(hash, g_strdup(nodename), g_object_ref(bstats));

     if ((backing = virJSONValueObjectGetObject(dev, "backing")) &&
         qemuMonitorJSONGetOneBlockStatsInfo(backing, dev_name, depth + 1, hash) < 0)
@@ -2617,12 +2593,8 @@ qemuMonitorJSONBlockStatsUpdateCapacityData(virJSONValue *image,
     qemuBlockStats *bstats;

     if (!(bstats = virHashLookup(stats, name))) {
-        bstats = g_new0(qemuBlockStats, 1);
-
-        if (virHashAddEntry(stats, name, bstats) < 0) {
-            VIR_FREE(bstats);
-            return -1;
-        }
+        bstats = qemuBlockStatsNew();
+        g_hash_table_insert(stats, g_strdup(name), bstats);
     }

     if (entry)
-- 
2.51.0