From: Moteen Shah <moteenshah.02@gmail.com>

Move validation from virDomainDiskDefIotuneParse into the validation callback
Signed-off-by: Moteen Shah <moteenshah.02@gmail.com>
---
 src/conf/domain_conf.c     | 40 -------------------------------------
 src/conf/domain_validate.c | 41 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 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..71dc74abc8 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -651,6 +651,47 @@ 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

On 4/9/22 11:38, Moteen Shah wrote:

I recomend using 'git send-email' which does the right thing (as in not
send the patch as an attachment).

Quick glance at the patch though: since you are touching the error
messages (moving them, not changing them), please do put them on one
line, like this:


+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("total and read/write bytes_sec cannot be set at the same time"));

Error message are exempt from the 80 chars long line rule, so that they
are easy to 'git grep'.

Michal