[PATCH v3] conf: Move validation checks from virDomainDiskDefIotuneParse into domain_validate.c

Moteen Shah posted 1 patch 2 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20220411124049.98272-1-codeguy.moteen@gmail.com
Test syntax-check passed
src/conf/domain_conf.c     | 40 --------------------------------------
src/conf/domain_validate.c | 37 +++++++++++++++++++++++++++++++++++
2 files changed, 37 insertions(+), 40 deletions(-)
[PATCH v3] conf: Move validation checks from virDomainDiskDefIotuneParse into domain_validate.c
Posted by Moteen Shah 2 years ago
Move validation from virDomainDiskDefIotuneParse into the validation callback

Signed-off-by: Moteen Shah <codeguy.moteen@gmail.com>
---
 src/conf/domain_conf.c     | 40 --------------------------------------
 src/conf/domain_validate.c | 37 +++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 40 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5dd269b283..bd2884088c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8790,46 +8790,6 @@ virDomainDiskDefIotuneParse(virDomainDiskDef *def,
     def->blkdeviotune.group_name =
         virXPathString("string(./iotune/group_name)", ctxt);
 
-    if ((def->blkdeviotune.total_bytes_sec &&
-         def->blkdeviotune.read_bytes_sec) ||
-        (def->blkdeviotune.total_bytes_sec &&
-         def->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 ((def->blkdeviotune.total_iops_sec &&
-         def->blkdeviotune.read_iops_sec) ||
-        (def->blkdeviotune.total_iops_sec &&
-         def->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 ((def->blkdeviotune.total_bytes_sec_max &&
-         def->blkdeviotune.read_bytes_sec_max) ||
-        (def->blkdeviotune.total_bytes_sec_max &&
-         def->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 ((def->blkdeviotune.total_iops_sec_max &&
-         def->blkdeviotune.read_iops_sec_max) ||
-        (def->blkdeviotune.total_iops_sec_max &&
-         def->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;
 }
 #undef PARSE_IOTUNE
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index d6869e8fd8..68190fc3e2 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -651,6 +651,43 @@ 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"));
+        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
      * handled in common code. For other bus types it's not possible
-- 
2.35.1
Re: [PATCH v3] conf: Move validation checks from virDomainDiskDefIotuneParse into domain_validate.c
Posted by Michal Prívozník 2 years ago
On 4/11/22 14:40, Moteen Shah wrote:
> Move validation from virDomainDiskDefIotuneParse into the validation callback
> 
> Signed-off-by: Moteen Shah <codeguy.moteen@gmail.com>
> ---
>  src/conf/domain_conf.c     | 40 --------------------------------------
>  src/conf/domain_validate.c | 37 +++++++++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+), 40 deletions(-)
>

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and pushed. Congratulations on your first libvirt contribution!

Michal
Re: [PATCH v3] conf: Move validation checks from virDomainDiskDefIotuneParse into domain_validate.c
Posted by Moteen Shah 2 years ago
Awesome, thank you so much!

On Tue, 12 Apr 2022 at 13:57, Michal Prívozník <mprivozn@redhat.com> wrote:

> On 4/11/22 14:40, Moteen Shah wrote:
> > Move validation from virDomainDiskDefIotuneParse into the validation
> callback
> >
> > Signed-off-by: Moteen Shah <codeguy.moteen@gmail.com>
> > ---
> >  src/conf/domain_conf.c     | 40 --------------------------------------
> >  src/conf/domain_validate.c | 37 +++++++++++++++++++++++++++++++++++
> >  2 files changed, 37 insertions(+), 40 deletions(-)
> >
>
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
>
> and pushed. Congratulations on your first libvirt contribution!
>
> Michal
>
>