[PATCH 09/23] qemu: add max iotune settings check to virDomainBlockIoTuneValidate

Nikolay Shirokovskiy posted 23 patches 5 years ago
[PATCH 09/23] qemu: add max iotune settings check to virDomainBlockIoTuneValidate
Posted by Nikolay Shirokovskiy 5 years ago
Now only qemu and test drivers support iotunes and for both of them this check
makes sense. I guess there is little chance that this patch will break loading
of some domains with incorrect config though. If this is the issue then we can
put this common check to a different place.

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
---
 src/conf/domain_conf.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 173424a..800bca5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8663,6 +8663,35 @@ virDomainBlockIoTuneValidate(virDomainBlockIoTuneInfoPtr iotune)
         return -1;
     }
 
+#define CHECK_MAX(val) \
+    do { \
+        if (iotune->val##_max) { \
+            if (!iotune->val) { \
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \
+                               _("value '%s' cannot be set if " \
+                                 "'%s' is not set"), \
+                               #val "_max", #val); \
+                return -1; \
+            } \
+            if (iotune->val##_max < iotune->val) { \
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \
+                               _("value '%s' cannot be " \
+                                 "smaller than '%s'"), \
+                               #val "_max", #val); \
+                return -1; \
+            } \
+        } \
+    } while (false)
+
+    CHECK_MAX(total_bytes_sec);
+    CHECK_MAX(read_bytes_sec);
+    CHECK_MAX(write_bytes_sec);
+    CHECK_MAX(total_iops_sec);
+    CHECK_MAX(read_iops_sec);
+    CHECK_MAX(write_iops_sec);
+
+#undef CHECK_MAX
+
     return 0;
 }
 
-- 
1.8.3.1

Re: [PATCH 09/23] qemu: add max iotune settings check to virDomainBlockIoTuneValidate
Posted by Peter Krempa 4 years, 11 months ago
On Mon, Jan 11, 2021 at 12:50:02 +0300, Nikolay Shirokovskiy wrote:
> Now only qemu and test drivers support iotunes and for both of them this check
> makes sense. I guess there is little chance that this patch will break loading
> of some domains with incorrect config though. If this is the issue then we can
> put this common check to a different place.
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
> ---
>  src/conf/domain_conf.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 173424a..800bca5 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8663,6 +8663,35 @@ virDomainBlockIoTuneValidate(virDomainBlockIoTuneInfoPtr iotune)
>          return -1;
>      }
>  
> +#define CHECK_MAX(val) \

As noted for previous patch, this series is meant to remove macros, not
add them.

Add an open-coded version.

> +    do { \
> +        if (iotune->val##_max) { \
> +            if (!iotune->val) { \
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \
> +                               _("value '%s' cannot be set if " \
> +                                 "'%s' is not set"), \

And don't break error messages.