[PATCH v5 08/18] qemu: Refactor qemuDomainSetBlockIoTune to extract common methods

Harikumar R posted 18 patches 2 weeks, 1 day ago
[PATCH v5 08/18] qemu: Refactor qemuDomainSetBlockIoTune to extract common methods
Posted by Harikumar R 2 weeks, 1 day ago
From: Chun Feng Wu <danielwuwy@163.com>

extract common methods from "qemuDomainSetBlockIoTune" to be reused
by throttle handling later, common methods include:
* "qemuDomainValidateBlockIoTune", which is to validate that PARAMS
  contains only recognized parameter names with correct types
* "qemuDomainSetBlockIoTuneFields", which is to load parameters into
  internal object virDomainBlockIoTuneInfo
* "qemuDomainCheckBlockIoTuneMutualExclusion", which is to check rules
  like "total and read/write of bytes_sec cannot be set at the same time"
* "qemuDomainCheckBlockIoTuneMax", which is to check "max" rules within iotune

Signed-off-by: Chun Feng Wu <danielwuwy@163.com>
---
 src/qemu/qemu_driver.c | 225 ++++++++++++++++++++++++++---------------
 1 file changed, 142 insertions(+), 83 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index aa8a78da69..e80e5fc511 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14989,35 +14989,8 @@ qemuDomainCheckBlockIoTuneReset(virDomainDiskDef *disk,
 
 
 static int
-qemuDomainSetBlockIoTune(virDomainPtr dom,
-                         const char *path,
-                         virTypedParameterPtr params,
-                         int nparams,
-                         unsigned int flags)
+qemuDomainValidateBlockIoTune(virTypedParameterPtr params, int nparams)
 {
-    virQEMUDriver *driver = dom->conn->privateData;
-    virDomainObj *vm = NULL;
-    qemuDomainObjPrivate *priv;
-    virDomainDef *def = NULL;
-    virDomainDef *persistentDef = NULL;
-    virDomainBlockIoTuneInfo info = { 0 };
-    virDomainBlockIoTuneInfo conf_info = { 0 };
-    int ret = -1;
-    size_t i;
-    virDomainDiskDef *conf_disk = NULL;
-    virDomainDiskDef *disk;
-    qemuBlockIoTuneSetFlags set_fields = 0;
-    g_autoptr(virQEMUDriverConfig) cfg = NULL;
-    virObjectEvent *event = NULL;
-    virTypedParameterPtr eventParams = NULL;
-    int eventNparams = 0;
-    int eventMaxparams = 0;
-    virDomainBlockIoTuneInfo *cur_info;
-    virDomainBlockIoTuneInfo *conf_cur_info;
-
-
-    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
-                  VIR_DOMAIN_AFFECT_CONFIG, -1);
     if (virTypedParamsValidate(params, nparams,
                                VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC,
                                VIR_TYPED_PARAM_ULLONG,
@@ -15062,32 +15035,28 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
                                NULL) < 0)
         return -1;
 
-    if (!(vm = qemuDomainObjFromDomain(dom)))
-        return -1;
-
-    if (virDomainSetBlockIoTuneEnsureACL(dom->conn, vm->def, flags) < 0)
-        goto cleanup;
-
-    cfg = virQEMUDriverGetConfig(driver);
-
-    if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
-        goto cleanup;
-
-    priv = vm->privateData;
+    return 0;
+}
 
-    if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
-        goto endjob;
 
-    if (virTypedParamsAddString(&eventParams, &eventNparams, &eventMaxparams,
-                                VIR_DOMAIN_TUNABLE_BLKDEV_DISK, path) < 0)
-        goto endjob;
+static int
+qemuDomainSetBlockIoTuneFields(virDomainBlockIoTuneInfo *info,
+                               virTypedParameterPtr params,
+                               int nparams,
+                               qemuBlockIoTuneSetFlags *set_fields,
+                               virTypedParameterPtr *eventParams,
+                               int *eventNparams,
+                               int *eventMaxparams)
+{
+    size_t i;
+    int ret = -1;
 
 #define SET_IOTUNE_FIELD(FIELD, BOOL, CONST) \
     if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_##CONST)) { \
-        info.FIELD = param->value.ul; \
-        set_fields |= QEMU_BLOCK_IOTUNE_SET_##BOOL; \
-        if (virTypedParamsAddULLong(&eventParams, &eventNparams, \
-                                    &eventMaxparams, \
+        info->FIELD = param->value.ul; \
+        *set_fields |= QEMU_BLOCK_IOTUNE_SET_##BOOL; \
+        if (virTypedParamsAddULLong(eventParams, eventNparams, \
+                                    eventMaxparams, \
                                     VIR_DOMAIN_TUNABLE_BLKDEV_##CONST, \
                                     param->value.ul) < 0) \
             goto endjob; \
@@ -15127,10 +15096,10 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
 
         /* NB: Cannot use macro since this is a value.s not a value.ul */
         if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME)) {
-            info.group_name = g_strdup(param->value.s);
-            set_fields |= QEMU_BLOCK_IOTUNE_SET_GROUP_NAME;
-            if (virTypedParamsAddString(&eventParams, &eventNparams,
-                                        &eventMaxparams,
+            info->group_name = g_strdup(param->value.s);
+            *set_fields |= QEMU_BLOCK_IOTUNE_SET_GROUP_NAME;
+            if (virTypedParamsAddString(eventParams, eventNparams,
+                                        eventMaxparams,
                                         VIR_DOMAIN_TUNABLE_BLKDEV_GROUP_NAME,
                                         param->value.s) < 0)
                 goto endjob;
@@ -15153,56 +15122,56 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
 
 #undef SET_IOTUNE_FIELD
 
-    if ((info.total_bytes_sec && info.read_bytes_sec) ||
-        (info.total_bytes_sec && info.write_bytes_sec)) {
+    ret = 0;
+ endjob:
+    return ret;
+}
+
+
+static int
+qemuDomainCheckBlockIoTuneMutualExclusion(virDomainBlockIoTuneInfo *info)
+{
+    if ((info->total_bytes_sec && info->read_bytes_sec) ||
+        (info->total_bytes_sec && info->write_bytes_sec)) {
         virReportError(VIR_ERR_INVALID_ARG, "%s",
                        _("total and read/write of bytes_sec cannot be set at the same time"));
-        goto endjob;
+        return -1;
     }
 
-    if ((info.total_iops_sec && info.read_iops_sec) ||
-        (info.total_iops_sec && info.write_iops_sec)) {
+    if ((info->total_iops_sec && info->read_iops_sec) ||
+        (info->total_iops_sec && info->write_iops_sec)) {
         virReportError(VIR_ERR_INVALID_ARG, "%s",
                        _("total and read/write of iops_sec cannot be set at the same time"));
-        goto endjob;
+        return -1;
     }
 
-    if ((info.total_bytes_sec_max && info.read_bytes_sec_max) ||
-        (info.total_bytes_sec_max && info.write_bytes_sec_max)) {
+    if ((info->total_bytes_sec_max && info->read_bytes_sec_max) ||
+        (info->total_bytes_sec_max && info->write_bytes_sec_max)) {
         virReportError(VIR_ERR_INVALID_ARG, "%s",
                        _("total and read/write of bytes_sec_max cannot be set at the same time"));
-        goto endjob;
+        return -1;
     }
 
-    if ((info.total_iops_sec_max && info.read_iops_sec_max) ||
-        (info.total_iops_sec_max && info.write_iops_sec_max)) {
+    if ((info->total_iops_sec_max && info->read_iops_sec_max) ||
+        (info->total_iops_sec_max && info->write_iops_sec_max)) {
         virReportError(VIR_ERR_INVALID_ARG, "%s",
                        _("total and read/write of iops_sec_max cannot be set at the same time"));
-        goto endjob;
+        return -1;
     }
 
-    virDomainBlockIoTuneInfoCopy(&info, &conf_info);
-
-    if (def) {
-        if (!(disk = qemuDomainDiskByName(def, path)))
-            goto endjob;
-
-        if (!qemuDomainDiskBlockIoTuneIsSupported(disk))
-            goto endjob;
-
-        cur_info = qemuDomainFindGroupBlockIoTune(def, disk, &info);
+    return 0;
+}
 
-        if (qemuDomainSetBlockIoTuneDefaults(&info, cur_info,
-                                             set_fields) < 0)
-            goto endjob;
 
-        if (qemuDomainCheckBlockIoTuneReset(disk, &info) < 0)
-            goto endjob;
+static int
+qemuDomainCheckBlockIoTuneMax(virDomainBlockIoTuneInfo *info)
+{
+    int ret = -1;
 
 #define CHECK_MAX(val, _bool) \
         do { \
-            if (info.val##_max) { \
-                if (!info.val) { \
+            if (info->val##_max) { \
+                if (!info->val) { \
                     if (QEMU_BLOCK_IOTUNE_SET_##_bool) { \
                         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \
                                        _("cannot reset '%1$s' when '%2$s' is set"), \
@@ -15214,7 +15183,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
                     } \
                     goto endjob; \
                 } \
-                if (info.val##_max < info.val) { \
+                if (info->val##_max < info->val) { \
                     virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \
                                    _("value '%1$s' cannot be smaller than '%2$s'"), \
                                    #val "_max", #val); \
@@ -15232,6 +15201,96 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
 
 #undef CHECK_MAX
 
+    ret = 0;
+ endjob:
+    return ret;
+}
+
+
+static int
+qemuDomainSetBlockIoTune(virDomainPtr dom,
+                         const char *path,
+                         virTypedParameterPtr params,
+                         int nparams,
+                         unsigned int flags)
+{
+    virQEMUDriver *driver = dom->conn->privateData;
+    virDomainObj *vm = NULL;
+    qemuDomainObjPrivate *priv;
+    virDomainDef *def = NULL;
+    virDomainDef *persistentDef = NULL;
+    virDomainBlockIoTuneInfo info = { 0 };
+    virDomainBlockIoTuneInfo conf_info = { 0 };
+    int ret = -1;
+    virDomainDiskDef *conf_disk = NULL;
+    virDomainDiskDef *disk;
+    qemuBlockIoTuneSetFlags set_fields = 0;
+    g_autoptr(virQEMUDriverConfig) cfg = NULL;
+    virObjectEvent *event = NULL;
+    virTypedParameterPtr eventParams = NULL;
+    int eventNparams = 0;
+    int eventMaxparams = 0;
+    virDomainBlockIoTuneInfo *cur_info;
+    virDomainBlockIoTuneInfo *conf_cur_info;
+
+
+    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+                  VIR_DOMAIN_AFFECT_CONFIG, -1);
+    if (qemuDomainValidateBlockIoTune(params, nparams) < 0)
+        return -1;
+
+    if (!(vm = qemuDomainObjFromDomain(dom)))
+        return -1;
+
+    if (virDomainSetBlockIoTuneEnsureACL(dom->conn, vm->def, flags) < 0)
+        goto cleanup;
+
+    cfg = virQEMUDriverGetConfig(driver);
+
+    if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
+        goto cleanup;
+
+    priv = vm->privateData;
+
+    if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
+        goto endjob;
+
+    if (virTypedParamsAddString(&eventParams, &eventNparams, &eventMaxparams,
+                                VIR_DOMAIN_TUNABLE_BLKDEV_DISK, path) < 0)
+        goto endjob;
+
+    if (qemuDomainSetBlockIoTuneFields(&info,
+                                       params,
+                                       nparams,
+                                       &set_fields,
+                                       &eventParams,
+                                       &eventNparams,
+                                       &eventMaxparams) < 0)
+        goto endjob;
+
+    if (qemuDomainCheckBlockIoTuneMutualExclusion(&info) < 0)
+        goto endjob;
+
+    virDomainBlockIoTuneInfoCopy(&info, &conf_info);
+
+    if (def) {
+        if (!(disk = qemuDomainDiskByName(def, path)))
+            goto endjob;
+
+        if (!qemuDomainDiskBlockIoTuneIsSupported(disk))
+            goto endjob;
+
+        cur_info = qemuDomainFindGroupBlockIoTune(def, disk, &info);
+
+        if (qemuDomainSetBlockIoTuneDefaults(&info, cur_info, set_fields) < 0)
+            goto endjob;
+
+        if (qemuDomainCheckBlockIoTuneReset(disk, &info) < 0)
+            goto endjob;
+
+        if (qemuDomainCheckBlockIoTuneMax(&info) < 0)
+            goto endjob;
+
         /* blockdev-based qemu doesn't want to set the throttling when a cdrom
          * is empty. Skip the monitor call here since we will set the throttling
          * once new media is inserted */
-- 
2.39.5 (Apple Git-154)