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

Re: [libvirt] [PATCH v2 2/2] test_driver: implement virDomainSetBlockIoTune



On Fri, Aug 09, 2019 at 09:53:00PM +0300, Ilias Stamatis wrote:
> Signed-off-by: Ilias Stamatis <stamatis iliass gmail com>
> ---
>  src/test/test_driver.c | 259 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 259 insertions(+)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 14b7d683e1..009bc18a73 100755
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -3613,6 +3613,264 @@ testDomainGetInterfaceParameters(virDomainPtr dom,
>  }
>
>
> +#define TEST_BLOCK_IOTUNE_MAX 1000000000000000LL
> +
> +static int
> +testDomainSetBlockIoTune(virDomainPtr dom,
> +                         const char *path,
> +                         virTypedParameterPtr params,
> +                         int nparams,
> +                         unsigned int flags)
> +{
> +    virDomainObjPtr vm = NULL;
> +    virDomainDefPtr def = NULL;
> +    virDomainBlockIoTuneInfo info = {0};
> +    virDomainDiskDefPtr 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,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME,
> +                               VIR_TYPED_PARAM_STRING,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH,
> +                               VIR_TYPED_PARAM_ULLONG,
> +                               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 '%s'"),
> +                       path);
> +        goto cleanup;
> +    }
> +
> +    info = conf_disk->blkdeviotune;
> +    if (VIR_STRDUP(info.group_name, conf_disk->blkdeviotune.group_name) < 0)
> +        goto cleanup;
> +
> +    if (virTypedParamsAddString(&eventParams, &eventNparams, &eventMaxparams,
> +                                VIR_DOMAIN_TUNABLE_BLKDEV_DISK, path) < 0)
> +        goto cleanup;
> +
> +#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, \
> +                                    param->value.ul) < 0) \
> +            goto cleanup; \
> +        continue; \
> +    }
> +
> +    for (i = 0; i < nparams; i++) {
> +        virTypedParameterPtr param = &params[i];
> +
> +        if (param->value.ul > TEST_BLOCK_IOTUNE_MAX) {
> +            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> +                           _("block I/O throttle limit value must"
> +                             " be no more than %llu"), TEST_BLOCK_IOTUNE_MAX);
> +            goto cleanup;
> +        }
> +
> +        SET_IOTUNE_FIELD(total_bytes_sec, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC,
> +                         VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC);

Long lines, I'll wrap them before merging.

...
> +
> +    if (info.total_bytes_sec > info.total_bytes_sec_max) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("total_bytes_sec "
> +                         "cannot be set higher than its corresponding max value"));
> +        goto cleanup;
> +    }
> +
> +    if (info.read_bytes_sec > info.read_bytes_sec_max) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("read_bytes_sec "
> +                         "cannot be set higher than its corresponding max value"));
> +        goto cleanup;
> +    }
> +
> +    if (info.write_bytes_sec > info.write_bytes_sec_max) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("write_bytes_sec "
> +                         "cannot be set higher than its corresponding max value"));
> +        goto cleanup;
> +    }
> +
> +    if (info.total_iops_sec > info.total_iops_sec_max) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("total_iops_sec "
> +                         "cannot be set higher than its corresponding max value"));
> +        goto cleanup;
> +    }
> +
> +    if (info.read_iops_sec > info.read_iops_sec_max) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("read_iops_sec "
> +                         "cannot be set higher than its corresponding max value"));
> +        goto cleanup;
> +    }
> +
> +    if (info.write_iops_sec > info.write_iops_sec_max) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("write_iops_sec "
> +                         "cannot be set higher than its corresponding max value"));
> +        goto cleanup;
> +    }

Repetitive. I'll squash the following diff in before pushing:

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 4340c55e6f..5f5c512571 100755
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3828,47 +3828,25 @@ testDomainSetBlockIoTune(virDomainPtr dom,
         goto cleanup;
     }

-    if (info.total_bytes_sec > info.total_bytes_sec_max) {
-        virReportError(VIR_ERR_INVALID_ARG, "%s",
-                       _("total_bytes_sec "
-                         "cannot be set higher than its corresponding max value"));
-        goto cleanup;
-    }

-    if (info.read_bytes_sec > info.read_bytes_sec_max) {
-        virReportError(VIR_ERR_INVALID_ARG, "%s",
-                       _("read_bytes_sec "
-                         "cannot be set higher than its corresponding max value"));
-        goto cleanup;
-    }
+#define TEST_BLOCK_IOTUNE_MAX_CHECK(FIELD, FIELD_MAX) \
+    do { \
+        if (info.FIELD > info.FIELD_MAX) { \
+            virReportError(VIR_ERR_INVALID_ARG, \
+                           _("%s cannot be set higher than %s "), \
+                             #FIELD, #FIELD_MAX); \
+            goto cleanup; \
+        } \
+    } while (0);

-    if (info.write_bytes_sec > info.write_bytes_sec_max) {
-        virReportError(VIR_ERR_INVALID_ARG, "%s",
-                       _("write_bytes_sec "
-                         "cannot be set higher than its corresponding max value"));
-        goto cleanup;
-    }
+    TEST_BLOCK_IOTUNE_MAX_CHECK(total_bytes_sec, total_bytes_sec_max);
+    TEST_BLOCK_IOTUNE_MAX_CHECK(read_bytes_sec, read_bytes_sec_max);
+    TEST_BLOCK_IOTUNE_MAX_CHECK(write_bytes_sec, write_bytes_sec_max);
+    TEST_BLOCK_IOTUNE_MAX_CHECK(total_iops_sec, total_iops_sec_max);
+    TEST_BLOCK_IOTUNE_MAX_CHECK(read_iops_sec, read_iops_sec_max);
+    TEST_BLOCK_IOTUNE_MAX_CHECK(write_iops_sec, write_iops_sec_max);

-    if (info.total_iops_sec > info.total_iops_sec_max) {
-        virReportError(VIR_ERR_INVALID_ARG, "%s",
-                       _("total_iops_sec "
-                         "cannot be set higher than its corresponding max value"));
-        goto cleanup;
-    }
-
-    if (info.read_iops_sec > info.read_iops_sec_max) {
-        virReportError(VIR_ERR_INVALID_ARG, "%s",
-                       _("read_iops_sec "
-                         "cannot be set higher than its corresponding max value"));
-        goto cleanup;
-    }
-
-    if (info.write_iops_sec > info.write_iops_sec_max) {
-        virReportError(VIR_ERR_INVALID_ARG, "%s",
-                       _("write_iops_sec "
-                         "cannot be set higher than its corresponding max value"));
-        goto cleanup;
-    }
+#undef TEST_BLOCK_IOTUNE_MAX_CHECK

     if (virDomainDiskSetBlockIOTune(conf_disk, &info) < 0)
         goto cleanup;

Reviewed-by: Erik Skultety <eskultet redhat com>


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