1. Patchew
  2. Libvirt
  3. [PATCH RFC v2 00/12] Support throttle block filters
  4. View patch

[PATCH RFC v2 12/12] config: validate: Verify throttle group fields

wucf@linux.ibm.com posted 12 patches 1 year, 10 months ago
There is a newer version of this series
[PATCH RFC v2 12/12] config: validate: Verify throttle group fields
Posted by wucf@linux.ibm.com 1 year, 10 months ago 
From: Hao Ning Xin <xinhaong@linux.ibm.com>

Both throttlegroup and iotune share the same fields, so they share the same verification logic

Signed-off-by: Hao Ning Xin <xinhaong@linux.ibm.com>
---
 src/conf/domain_validate.c | 98 +++++++++++++++++++++++++-------------
 1 file changed, 64 insertions(+), 34 deletions(-)

diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index be51e66ac8..81dfeeac14 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -658,6 +658,49 @@ virDomainDiskDefValidateStartupPolicy(const virDomainDiskDef *disk)
 }
 
 
+static int
+virDomainDiskIoTuneValidate(const virDomainBlockIoTuneInfo blkdeviotune)
+{
+    if ((blkdeviotune.total_bytes_sec &&
+         blkdeviotune.read_bytes_sec) ||
+        (blkdeviotune.total_bytes_sec &&
+         blkdeviotune.write_bytes_sec)) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("total and read/write bytes_sec cannot be set at the same time"));
+        return -1;
+    }
+
+    if ((blkdeviotune.total_iops_sec &&
+         blkdeviotune.read_iops_sec) ||
+        (blkdeviotune.total_iops_sec &&
+         blkdeviotune.write_iops_sec)) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("total and read/write iops_sec cannot be set at the same time"));
+        return -1;
+    }
+
+    if ((blkdeviotune.total_bytes_sec_max &&
+         blkdeviotune.read_bytes_sec_max) ||
+        (blkdeviotune.total_bytes_sec_max &&
+         blkdeviotune.write_bytes_sec_max)) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("total and read/write bytes_sec_max cannot be set at the same time"));
+        return -1;
+    }
+
+    if ((blkdeviotune.total_iops_sec_max &&
+         blkdeviotune.read_iops_sec_max) ||
+        (blkdeviotune.total_iops_sec_max &&
+         blkdeviotune.write_iops_sec_max)) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("total and read/write iops_sec_max cannot be set at the same time"));
+        return -1;
+    }
+
+    return 0;
+}
+
+
 static int
 virDomainDiskDefValidate(const virDomainDef *def,
                          const virDomainDiskDef *disk)
@@ -713,41 +756,8 @@ virDomainDiskDefValidate(const virDomainDef *def,
     }
 
     /* Validate IotuneParse */
-    if ((disk->blkdeviotune.total_bytes_sec &&
-         disk->blkdeviotune.read_bytes_sec) ||
-        (disk->blkdeviotune.total_bytes_sec &&
-         disk->blkdeviotune.write_bytes_sec)) {
-        virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("total and read/write bytes_sec cannot be set at the same time"));
-        return -1;
-    }
-
-    if ((disk->blkdeviotune.total_iops_sec &&
-         disk->blkdeviotune.read_iops_sec) ||
-        (disk->blkdeviotune.total_iops_sec &&
-         disk->blkdeviotune.write_iops_sec)) {
-        virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("total and read/write iops_sec cannot be set at the same time"));
+    if (virDomainDiskIoTuneValidate(disk->blkdeviotune) < 0)
         return -1;
-    }
-
-    if ((disk->blkdeviotune.total_bytes_sec_max &&
-         disk->blkdeviotune.read_bytes_sec_max) ||
-        (disk->blkdeviotune.total_bytes_sec_max &&
-         disk->blkdeviotune.write_bytes_sec_max)) {
-        virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("total and read/write bytes_sec_max cannot be set at the same time"));
-        return -1;
-    }
-
-    if ((disk->blkdeviotune.total_iops_sec_max &&
-         disk->blkdeviotune.read_iops_sec_max) ||
-        (disk->blkdeviotune.total_iops_sec_max &&
-         disk->blkdeviotune.write_iops_sec_max)) {
-        virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("total and read/write iops_sec_max cannot be set at the same time"));
-        return -1;
-    }
 
     /* Reject disks with a bus type that is not compatible with the
      * given address type. The function considers only buses that are
@@ -1822,6 +1832,23 @@ virDomainDefValidateIOThreads(const virDomainDef *def)
 }
 
 
+static int
+virDomainDefValidateThrottleGroups(const virDomainDef *def)
+{
+    size_t i;
+
+    for (i = 0; i < def->nthrottlegroups; i++) {
+        virDomainThrottleGroupDef *throttleGroup = def->throttlegroups[i];
+
+        /* Validate Throttle Group */
+        if (virDomainDiskIoTuneValidate(*throttleGroup) < 0)
+            return -1;
+    }
+
+    return 0;
+}
+
+
 static int
 virDomainDefValidateInternal(const virDomainDef *def,
                              virDomainXMLOption *xmlopt)
@@ -1877,6 +1904,9 @@ virDomainDefValidateInternal(const virDomainDef *def,
     if (virDomainDefValidateIOThreads(def) < 0)
         return -1;
 
+    if (virDomainDefValidateThrottleGroups(def) < 0)
+        return -1;
+
     return 0;
 }
 
-- 
2.34.1
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH RFC v2 12/12] config: validate: Verify throttle group fields
Posted by Peter Krempa 1 year, 9 months ago 
On Thu, Apr 11, 2024 at 19:02:00 -0700, wucf@linux.ibm.com wrote:
> From: Hao Ning Xin <xinhaong@linux.ibm.com>
> 
> Both throttlegroup and iotune share the same fields, so they share the same verification logic
> 
> Signed-off-by: Hao Ning Xin <xinhaong@linux.ibm.com>
> ---

Split out the bit adding +virDomainDefValidateThrottleGroups, which
belongs to the patch adding the feature. The refactor itself is okay and
can be moved before the patch that requires it.


>  src/conf/domain_validate.c | 98 +++++++++++++++++++++++++-------------
>  1 file changed, 64 insertions(+), 34 deletions(-)
> 
> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
> index be51e66ac8..81dfeeac14 100644
> --- a/src/conf/domain_validate.c
> +++ b/src/conf/domain_validate.c
> @@ -658,6 +658,49 @@ virDomainDiskDefValidateStartupPolicy(const virDomainDiskDef *disk)
>  }
>  
>  
> +static int
> +virDomainDiskIoTuneValidate(const virDomainBlockIoTuneInfo blkdeviotune)
> +{
> +    if ((blkdeviotune.total_bytes_sec &&
> +         blkdeviotune.read_bytes_sec) ||
> +        (blkdeviotune.total_bytes_sec &&
> +         blkdeviotune.write_bytes_sec)) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("total and read/write bytes_sec cannot be set at the same time"));
> +        return -1;
> +    }
> +
> +    if ((blkdeviotune.total_iops_sec &&
> +         blkdeviotune.read_iops_sec) ||
> +        (blkdeviotune.total_iops_sec &&
> +         blkdeviotune.write_iops_sec)) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("total and read/write iops_sec cannot be set at the same time"));
> +        return -1;
> +    }
> +
> +    if ((blkdeviotune.total_bytes_sec_max &&
> +         blkdeviotune.read_bytes_sec_max) ||
> +        (blkdeviotune.total_bytes_sec_max &&
> +         blkdeviotune.write_bytes_sec_max)) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("total and read/write bytes_sec_max cannot be set at the same time"));
> +        return -1;
> +    }
> +
> +    if ((blkdeviotune.total_iops_sec_max &&
> +         blkdeviotune.read_iops_sec_max) ||
> +        (blkdeviotune.total_iops_sec_max &&
> +         blkdeviotune.write_iops_sec_max)) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("total and read/write iops_sec_max cannot be set at the same time"));
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
>  static int
>  virDomainDiskDefValidate(const virDomainDef *def,
>                           const virDomainDiskDef *disk)
> @@ -713,41 +756,8 @@ virDomainDiskDefValidate(const virDomainDef *def,
>      }
>  
>      /* Validate IotuneParse */
> -    if ((disk->blkdeviotune.total_bytes_sec &&
> -         disk->blkdeviotune.read_bytes_sec) ||
> -        (disk->blkdeviotune.total_bytes_sec &&
> -         disk->blkdeviotune.write_bytes_sec)) {
> -        virReportError(VIR_ERR_XML_ERROR, "%s",
> -                       _("total and read/write bytes_sec cannot be set at the same time"));
> -        return -1;
> -    }
> -
> -    if ((disk->blkdeviotune.total_iops_sec &&
> -         disk->blkdeviotune.read_iops_sec) ||
> -        (disk->blkdeviotune.total_iops_sec &&
> -         disk->blkdeviotune.write_iops_sec)) {
> -        virReportError(VIR_ERR_XML_ERROR, "%s",
> -                       _("total and read/write iops_sec cannot be set at the same time"));
> +    if (virDomainDiskIoTuneValidate(disk->blkdeviotune) < 0)
>          return -1;
> -    }
> -
> -    if ((disk->blkdeviotune.total_bytes_sec_max &&
> -         disk->blkdeviotune.read_bytes_sec_max) ||
> -        (disk->blkdeviotune.total_bytes_sec_max &&
> -         disk->blkdeviotune.write_bytes_sec_max)) {
> -        virReportError(VIR_ERR_XML_ERROR, "%s",
> -                       _("total and read/write bytes_sec_max cannot be set at the same time"));
> -        return -1;
> -    }
> -
> -    if ((disk->blkdeviotune.total_iops_sec_max &&
> -         disk->blkdeviotune.read_iops_sec_max) ||
> -        (disk->blkdeviotune.total_iops_sec_max &&
> -         disk->blkdeviotune.write_iops_sec_max)) {
> -        virReportError(VIR_ERR_XML_ERROR, "%s",
> -                       _("total and read/write iops_sec_max cannot be set at the same time"));
> -        return -1;
> -    }
>  
>      /* Reject disks with a bus type that is not compatible with the
>       * given address type. The function considers only buses that are
> @@ -1822,6 +1832,23 @@ virDomainDefValidateIOThreads(const virDomainDef *def)
>  }
>  
>  
> +static int
> +virDomainDefValidateThrottleGroups(const virDomainDef *def)
> +{
> +    size_t i;
> +
> +    for (i = 0; i < def->nthrottlegroups; i++) {
> +        virDomainThrottleGroupDef *throttleGroup = def->throttlegroups[i];
> +
> +        /* Validate Throttle Group */
> +        if (virDomainDiskIoTuneValidate(*throttleGroup) < 0)
> +            return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
>  static int
>  virDomainDefValidateInternal(const virDomainDef *def,
>                               virDomainXMLOption *xmlopt)
> @@ -1877,6 +1904,9 @@ virDomainDefValidateInternal(const virDomainDef *def,
>      if (virDomainDefValidateIOThreads(def) < 0)
>          return -1;
>  
> +    if (virDomainDefValidateThrottleGroups(def) < 0)
> +        return -1;
> +
>      return 0;
>  }
>  
> -- 
> 2.34.1
> _______________________________________________
> Devel mailing list -- devel@lists.libvirt.org
> To unsubscribe send an email to devel-leave@lists.libvirt.org

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.