[PATCH v8 15/18] test_driver: Test throttle group lifecycle APIs

Harikumar Rajkumar posted 18 patches 9 months, 2 weeks ago
[PATCH v8 15/18] test_driver: Test throttle group lifecycle APIs
Posted by Harikumar Rajkumar 9 months, 2 weeks ago
From: Chun Feng Wu <danielwuwy@163.com>

Test throttle group APIs

* Extract common methods for both "testDomainSetThrottleGroup" and "testDomainSetBlockIoTune":
 testDomainValidateBlockIoTune, testDomainSetBlockIoTuneFields,
testDomainCheckBlockIoTuneMutualExclusion, testDomainCheckBlockIoTuneMax
* Test "Set": testDomainSetThrottleGroup
* Test "Get": testDomainGetThrottleGroup
* Test "Del": testDomainDelThrottleGroup

Signed-off-by: Chun Feng Wu <danielwuwy@163.com>

* Cleanup Test "Get": testDomainGetThrottleGroup
* Update the version to 11.1.0.

Signed-off-by: Harikumar Rajkumar <harirajkumar230@gmail.com>
---
 src/test/test_driver.c | 367 +++++++++++++++++++++++++++--------------
 1 file changed, 245 insertions(+), 122 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 6f18b2b2c8..72eab99184 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3783,25 +3783,8 @@ testDomainGetInterfaceParameters(virDomainPtr dom,
 #define TEST_BLOCK_IOTUNE_MAX 1000000000000000LL
 
 static int
-testDomainSetBlockIoTune(virDomainPtr dom,
-                         const char *path,
-                         virTypedParameterPtr params,
-                         int nparams,
-                         unsigned int flags)
+testDomainValidateBlockIoTune(virTypedParameterPtr params, int nparams)
 {
-    virDomainObj *vm = NULL;
-    virDomainDef *def = NULL;
-    virDomainBlockIoTuneInfo info = {0};
-    virDomainDiskDef *conf_disk = NULL;
-    virTypedParameterPtr eventParams = NULL;
-    int eventNparams = 0;
-    int eventMaxparams = 0;
-    size_t i;
-    int ret = -1;
-
-    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,
@@ -3846,34 +3829,29 @@ testDomainSetBlockIoTune(virDomainPtr dom,
                                NULL) < 0)
         return -1;
 
-    if (!(vm = testDomObjFromDomain(dom)))
-        return -1;
-
-    if (!(def = virDomainObjGetOneDef(vm, flags)))
-        goto cleanup;
-
-    if (!(conf_disk = virDomainDiskByName(def, path, true))) {
-        virReportError(VIR_ERR_INVALID_ARG,
-                       _("missing persistent configuration for disk '%1$s'"),
-                       path);
-        goto cleanup;
-    }
+    return 0;
+}
 
-    info = conf_disk->blkdeviotune;
-    info.group_name = g_strdup(conf_disk->blkdeviotune.group_name);
 
-    if (virTypedParamsAddString(&eventParams, &eventNparams, &eventMaxparams,
-                                VIR_DOMAIN_TUNABLE_BLKDEV_DISK, path) < 0)
-        goto cleanup;
+static int
+testDomainSetBlockIoTuneFields(virDomainBlockIoTuneInfo *info,
+                               virTypedParameterPtr params,
+                               int nparams,
+                               virTypedParameterPtr *eventParams,
+                               int *eventNparams,
+                               int *eventMaxparams)
+{
+    size_t i;
+    int ret = -1;
 
-#define SET_IOTUNE_FIELD(FIELD, STR, TUNABLE_STR) \
-    if (STREQ(param->field, STR)) { \
-        info.FIELD = param->value.ul; \
-        if (virTypedParamsAddULLong(&eventParams, &eventNparams, \
-                                    &eventMaxparams, \
-                                    TUNABLE_STR, \
+#define SET_IOTUNE_FIELD(FIELD, CONST) \
+    if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_##CONST)) { \
+        info->FIELD = param->value.ul; \
+        if (virTypedParamsAddULLong(eventParams, eventNparams, \
+                                    eventMaxparams, \
+                                    VIR_DOMAIN_TUNABLE_BLKDEV_##CONST, \
                                     param->value.ul) < 0) \
-            goto cleanup; \
+            goto endjob; \
         continue; \
     }
 
@@ -3884,119 +3862,99 @@ testDomainSetBlockIoTune(virDomainPtr dom,
             virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
                            _("block I/O throttle limit value must be no more than %1$llu"),
                            TEST_BLOCK_IOTUNE_MAX);
-            goto cleanup;
+            goto endjob;
         }
 
-        SET_IOTUNE_FIELD(total_bytes_sec,
-                         VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC);
-        SET_IOTUNE_FIELD(read_bytes_sec,
-                         VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC);
-        SET_IOTUNE_FIELD(write_bytes_sec,
-                         VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC);
-        SET_IOTUNE_FIELD(total_iops_sec,
-                         VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC);
-        SET_IOTUNE_FIELD(read_iops_sec,
-                         VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC);
-        SET_IOTUNE_FIELD(write_iops_sec,
-                         VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC);
-
-        SET_IOTUNE_FIELD(total_bytes_sec_max,
-                         VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX);
-        SET_IOTUNE_FIELD(read_bytes_sec_max,
-                         VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX);
-        SET_IOTUNE_FIELD(write_bytes_sec_max,
-                         VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX);
-        SET_IOTUNE_FIELD(total_iops_sec_max,
-                         VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX);
-        SET_IOTUNE_FIELD(read_iops_sec_max,
-                         VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX);
-        SET_IOTUNE_FIELD(write_iops_sec_max,
-                         VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX);
-        SET_IOTUNE_FIELD(size_iops_sec,
-                         VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC);
-
+        SET_IOTUNE_FIELD(total_bytes_sec, TOTAL_BYTES_SEC);
+        SET_IOTUNE_FIELD(read_bytes_sec, READ_BYTES_SEC);
+        SET_IOTUNE_FIELD(write_bytes_sec, WRITE_BYTES_SEC);
+        SET_IOTUNE_FIELD(total_iops_sec, TOTAL_IOPS_SEC);
+        SET_IOTUNE_FIELD(read_iops_sec, READ_IOPS_SEC);
+        SET_IOTUNE_FIELD(write_iops_sec, WRITE_IOPS_SEC);
+
+        SET_IOTUNE_FIELD(total_bytes_sec_max, TOTAL_BYTES_SEC_MAX);
+        SET_IOTUNE_FIELD(read_bytes_sec_max, READ_BYTES_SEC_MAX);
+        SET_IOTUNE_FIELD(write_bytes_sec_max, WRITE_BYTES_SEC_MAX);
+        SET_IOTUNE_FIELD(total_iops_sec_max, TOTAL_IOPS_SEC_MAX);
+        SET_IOTUNE_FIELD(read_iops_sec_max, READ_IOPS_SEC_MAX);
+        SET_IOTUNE_FIELD(write_iops_sec_max, WRITE_IOPS_SEC_MAX);
+        SET_IOTUNE_FIELD(size_iops_sec, SIZE_IOPS_SEC);
+
+        /* NB: Cannot use macro since this is a value.s not a value.ul */
         if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME)) {
-            VIR_FREE(info.group_name);
-            info.group_name = g_strdup(param->value.s);
-            if (virTypedParamsAddString(&eventParams,
-                                        &eventNparams,
-                                        &eventMaxparams,
+            VIR_FREE(info->group_name);
+            info->group_name = g_strdup(param->value.s);
+            if (virTypedParamsAddString(eventParams, eventNparams,
+                                        eventMaxparams,
                                         VIR_DOMAIN_TUNABLE_BLKDEV_GROUP_NAME,
                                         param->value.s) < 0)
-                goto cleanup;
+                goto endjob;
             continue;
         }
 
-        SET_IOTUNE_FIELD(total_bytes_sec_max_length,
-                         VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX_LENGTH);
-        SET_IOTUNE_FIELD(read_bytes_sec_max_length,
-                         VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX_LENGTH);
-        SET_IOTUNE_FIELD(write_bytes_sec_max_length,
-                         VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX_LENGTH);
-        SET_IOTUNE_FIELD(total_iops_sec_max_length,
-                         VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX_LENGTH);
-        SET_IOTUNE_FIELD(read_iops_sec_max_length,
-                         VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX_LENGTH);
-        SET_IOTUNE_FIELD(write_iops_sec_max_length,
-                         VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH,
-                         VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX_LENGTH);
+        SET_IOTUNE_FIELD(total_bytes_sec_max_length, TOTAL_BYTES_SEC_MAX_LENGTH);
+        SET_IOTUNE_FIELD(read_bytes_sec_max_length, READ_BYTES_SEC_MAX_LENGTH);
+        SET_IOTUNE_FIELD(write_bytes_sec_max_length, WRITE_BYTES_SEC_MAX_LENGTH);
+        SET_IOTUNE_FIELD(total_iops_sec_max_length, TOTAL_IOPS_SEC_MAX_LENGTH);
+        SET_IOTUNE_FIELD(read_iops_sec_max_length, READ_IOPS_SEC_MAX_LENGTH);
+        SET_IOTUNE_FIELD(write_iops_sec_max_length, WRITE_IOPS_SEC_MAX_LENGTH);
     }
+
 #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
+testDomainCheckBlockIoTuneMutualExclusion(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 cleanup;
+        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 cleanup;
+        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 cleanup;
+        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 cleanup;
+        return -1;
     }
 
+    return 0;
+}
+
+
+static int
+testDomainCheckBlockIoTuneMax(virDomainBlockIoTuneInfo *info)
+{
+    int ret = -1;
 
 #define TEST_BLOCK_IOTUNE_MAX_CHECK(FIELD, FIELD_MAX) \
     do { \
-        if (info.FIELD > info.FIELD_MAX) { \
+        if (info->FIELD > info->FIELD_MAX) { \
             virReportError(VIR_ERR_INVALID_ARG, \
                            _("%1$s cannot be set higher than %2$s"), \
                              #FIELD, #FIELD_MAX); \
-            goto cleanup; \
+            goto endjob; \
         } \
     } while (0);
 
@@ -4009,6 +3967,69 @@ testDomainSetBlockIoTune(virDomainPtr dom,
 
 #undef TEST_BLOCK_IOTUNE_MAX_CHECK
 
+    ret = 0;
+ endjob:
+    return ret;
+}
+
+
+static int
+testDomainSetBlockIoTune(virDomainPtr dom,
+                         const char *path,
+                         virTypedParameterPtr params,
+                         int nparams,
+                         unsigned int flags)
+{
+    virDomainObj *vm = NULL;
+    virDomainDef *def = NULL;
+    virDomainBlockIoTuneInfo info = {0};
+    virDomainDiskDef *conf_disk = NULL;
+    virTypedParameterPtr eventParams = NULL;
+    int eventNparams = 0;
+    int eventMaxparams = 0;
+    int ret = -1;
+
+    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+                  VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+    if (testDomainValidateBlockIoTune(params, nparams) < 0)
+        return -1;
+
+    if (!(vm = testDomObjFromDomain(dom)))
+        return -1;
+
+    if (!(def = virDomainObjGetOneDef(vm, flags)))
+        goto cleanup;
+
+    if (!(conf_disk = virDomainDiskByName(def, path, true))) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("missing persistent configuration for disk '%1$s'"),
+                       path);
+        goto cleanup;
+    }
+
+    info = conf_disk->blkdeviotune;
+    info.group_name = g_strdup(conf_disk->blkdeviotune.group_name);
+
+    if (virTypedParamsAddString(&eventParams, &eventNparams, &eventMaxparams,
+                                VIR_DOMAIN_TUNABLE_BLKDEV_DISK, path) < 0)
+        goto cleanup;
+
+    if (testDomainSetBlockIoTuneFields(&info,
+                                       params,
+                                       nparams,
+                                       &eventParams,
+                                       &eventNparams,
+                                       &eventMaxparams) < 0)
+        goto cleanup;
+
+    if (testDomainCheckBlockIoTuneMutualExclusion(&info) < 0)
+        goto cleanup;
+
+
+    if (testDomainCheckBlockIoTuneMax(&info) < 0)
+        goto cleanup;
+
     virDomainDiskSetBlockIOTune(conf_disk, &info);
 
     ret = 0;
@@ -4119,6 +4140,106 @@ testDomainGetBlockIoTune(virDomainPtr dom,
     virDomainObjEndAPI(&vm);
     return ret;
 }
+
+
+static int
+testDomainSetThrottleGroup(virDomainPtr dom,
+                           const char *group,
+                           virTypedParameterPtr params,
+                           int nparams,
+                           unsigned int flags)
+{
+    virDomainObj *vm = NULL;
+    virDomainDef *def = NULL;
+    virDomainThrottleGroupDef info = { 0 };
+    virDomainThrottleGroupDef *cur_info = NULL;
+    virTypedParameterPtr eventParams = NULL;
+    int eventNparams = 0;
+    int eventMaxparams = 0;
+    int ret = -1;
+
+    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+                  VIR_DOMAIN_AFFECT_CONFIG, -1);
+    if (testDomainValidateBlockIoTune(params, nparams) < 0)
+        return -1;
+
+    if (!(vm = testDomObjFromDomain(dom)))
+        return -1;
+
+    if (!(def = virDomainObjGetOneDef(vm, flags)))
+        goto cleanup;
+
+    if (virTypedParamsAddString(&eventParams, &eventNparams, &eventMaxparams,
+                                VIR_DOMAIN_TUNABLE_BLKDEV_GROUP_NAME, group) < 0)
+        goto cleanup;
+
+    if (testDomainSetBlockIoTuneFields(&info,
+                                       params,
+                                       nparams,
+                                       &eventParams,
+                                       &eventNparams,
+                                       &eventMaxparams) < 0)
+        goto cleanup;
+
+    if (testDomainCheckBlockIoTuneMutualExclusion(&info) < 0)
+        goto cleanup;
+
+    if (testDomainCheckBlockIoTuneMax(&info) < 0)
+        goto cleanup;
+
+    cur_info = virDomainThrottleGroupByName(def, group);
+    if (cur_info != NULL) {
+        virDomainThrottleGroupUpdate(def, &info);
+    } else {
+        virDomainThrottleGroupAdd(def, &info);
+    }
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(info.group_name);
+    virDomainObjEndAPI(&vm);
+    if (eventNparams)
+        virTypedParamsFree(eventParams, eventNparams);
+    return ret;
+}
+
+
+static int
+testDomainDelThrottleGroup(virDomainPtr dom,
+                           const char *groupname,
+                           unsigned int flags)
+{
+    virDomainObj *vm = NULL;
+    virDomainDef *def = NULL;
+    virDomainThrottleGroupDef groupDef = {0};
+    virDomainThrottleGroupDef *reply = &groupDef;
+    int ret = -1;
+
+    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+                  VIR_DOMAIN_AFFECT_CONFIG |
+                  VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+
+    if (!(vm = testDomObjFromDomain(dom)))
+        return -1;
+
+    if (!(def = virDomainObjGetOneDef(vm, flags)))
+        goto cleanup;
+
+    if (!(reply = virDomainThrottleGroupByName(def, groupname))) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("throttle group '%1$s' was not found in the domain config"),
+                       groupname);
+        goto cleanup;
+    }
+
+    ret = 0;
+
+ cleanup:
+    virDomainObjEndAPI(&vm);
+    return ret;
+}
+
 #undef TEST_SET_PARAM
 
 
@@ -11218,6 +11339,8 @@ static virHypervisorDriver testHypervisorDriver = {
     .domainGetInterfaceParameters = testDomainGetInterfaceParameters, /* 5.6.0 */
     .domainSetBlockIoTune = testDomainSetBlockIoTune, /* 5.7.0 */
     .domainGetBlockIoTune = testDomainGetBlockIoTune, /* 5.7.0 */
+    .domainSetThrottleGroup = testDomainSetThrottleGroup, /* 11.1.0 */
+    .domainDelThrottleGroup = testDomainDelThrottleGroup, /* 11.1.0 */
     .domainSetBlkioParameters = testDomainSetBlkioParameters, /* 7.7.0 */
     .domainGetBlkioParameters = testDomainGetBlkioParameters, /* 7.7.0 */
     .connectListDefinedDomains = testConnectListDefinedDomains, /* 0.1.11 */
-- 
2.39.5 (Apple Git-154)
Re: [PATCH v8 15/18] test_driver: Test throttle group lifecycle APIs
Posted by Peter Krempa 8 months, 4 weeks ago
On Wed, Feb 19, 2025 at 22:27:19 +0530, Harikumar Rajkumar wrote:
> From: Chun Feng Wu <danielwuwy@163.com>
> 
> Test throttle group APIs
> 
> * Extract common methods for both "testDomainSetThrottleGroup" and "testDomainSetBlockIoTune":
>  testDomainValidateBlockIoTune, testDomainSetBlockIoTuneFields,
> testDomainCheckBlockIoTuneMutualExclusion, testDomainCheckBlockIoTuneMax
> * Test "Set": testDomainSetThrottleGroup
> * Test "Get": testDomainGetThrottleGroup
> * Test "Del": testDomainDelThrottleGroup
> 
> Signed-off-by: Chun Feng Wu <danielwuwy@163.com>
> 
> * Cleanup Test "Get": testDomainGetThrottleGroup
> * Update the version to 11.1.0.
> 
> Signed-off-by: Harikumar Rajkumar <harirajkumar230@gmail.com>
> ---
>  src/test/test_driver.c | 367 +++++++++++++++++++++++++++--------------
>  1 file changed, 245 insertions(+), 122 deletions(-)

For the sake of not prolonging the review any more I'm dropping this
patch for now. Please submit it once we're done with the qemu driver
impl.
Re: [PATCH v8 15/18] test_driver: Test throttle group lifecycle APIs
Posted by Harikumar Rajkumar 8 months, 3 weeks ago
> On Wed, Feb 19, 2025 at 22:27:19 +0530, Harikumar Rajkumar wrote:
> 
> For the sake of not prolonging the review any more I'm dropping this
> patch for now. Please submit it once we're done with the qemu driver
> impl.

Could you kindly confirm if the patch review has been completed? If so, we would appreciate a summary of the review comments.

Here is the summary of my understanding of the changes:
* Cleanup of the get_throttle_group RPC and cleanup of dead code.
* Removal of QEMU_CAPS_OBJECT_JSON in the QEMU driver implementation.
* Test cases for when copy-on-read is enabled.
* Version number update.

Can we submit the new patch (v9), or will you provide a branch for us to address the changes?

Thanks & regards,
Harikumar R
Re: [PATCH v8 15/18] test_driver: Test throttle group lifecycle APIs
Posted by Peter Krempa 8 months, 3 weeks ago
On Fri, Mar 14, 2025 at 09:03:09 -0000, Harikumar Rajkumar wrote:
> > On Wed, Feb 19, 2025 at 22:27:19 +0530, Harikumar Rajkumar wrote:
> > 
> > For the sake of not prolonging the review any more I'm dropping this
> > patch for now. Please submit it once we're done with the qemu driver
> > impl.
> 
> Could you kindly confirm if the patch review has been completed? If so, we would appreciate a summary of the review comments

The review is not done and once it will be done I'll notify you. The
more I have to reply to these requests for updates the less
time and motivation I have to work on the actual review.

> 
> Here is the summary of my understanding of the changes:
> * Cleanup of the get_throttle_group RPC and cleanup of dead code.
> * Removal of QEMU_CAPS_OBJECT_JSON in the QEMU driver implementation.
> * Test cases for when copy-on-read is enabled.
> * Version number update.
> 
> Can we submit the new patch (v9), or will you provide a branch for us to address the changes?

As I've said I will be doing the changes myself for the most improtant
parts because I don't want to spend any more time on this.

I've also found a few bugs that I'll fix.

For few patches or parts of patches that are not important and I don't
care about you'll have to re-send them and find someone to review them.

If this feature is so urgent for you or your employer you should
consider contributing to also aleviate the maintenance burden of the
code you are asking us to maintain. Without a long term commitment I
need to make sure that the code is in a state that it will not cause
problems for me or others who will be maintainig it.
Re: [PATCH v8 15/18] test_driver: Test throttle group lifecycle APIs
Posted by Harikumar Rajkumar 8 months, 3 weeks ago
Thanks for the update and for your efforts on the review. We understand your approach and we'll wait until it's completed.