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

[libvirt] [PATCH] iotune: setting an invalid value now reports error



When trying to set an invalid value into iotune element, standard
behavior was to not report any error, rather to reset all affected
subelements of the iotune element back to 0 which results in ignoring
those particular subelements by XML generator. Patch further
examines the return code of the virXPathULongLong function
and in case of an invalid non-integer value raises an error.
Fixed to preserve consistency with invalid value checking
of other elements.

Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1131811
---
 src/conf/domain_conf.c | 55 +++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 43 insertions(+), 12 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9557020..592aa94 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5416,6 +5416,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
     char *mirrorType = NULL;
     int expected_secret_usage = -1;
     int auth_secret_usage = -1;
+    int ret = 0;
 
     if (!(def = virDomainDiskDefNew()))
         return NULL;
@@ -5644,40 +5645,70 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
                     goto error;
                 }
             } else if (xmlStrEqual(cur->name, BAD_CAST "iotune")) {
-                if (virXPathULongLong("string(./iotune/total_bytes_sec)",
+                ret = virXPathULongLong("string(./iotune/total_bytes_sec)",
                                       ctxt,
-                                      &def->blkdeviotune.total_bytes_sec) < 0) {
+                                      &def->blkdeviotune.total_bytes_sec);
+                if (ret < 0 && ret != -2) {
                     def->blkdeviotune.total_bytes_sec = 0;
+                } else if (ret == -2) {
+                    virReportError(VIR_ERR_XML_ERROR, "%s",
+                                   _("total throughput limit must be an integer"));
+                    goto error;
                 }
 
-                if (virXPathULongLong("string(./iotune/read_bytes_sec)",
+                ret = virXPathULongLong("string(./iotune/read_bytes_sec)",
                                       ctxt,
-                                      &def->blkdeviotune.read_bytes_sec) < 0) {
+                                      &def->blkdeviotune.read_bytes_sec);
+                if (ret < 0 && ret != -2) {
                     def->blkdeviotune.read_bytes_sec = 0;
+                } else if (ret == -2) {
+                    virReportError(VIR_ERR_XML_ERROR, "%s",
+                                   _("read throughput limit must be an integer"));
+                    goto error;
                 }
 
-                if (virXPathULongLong("string(./iotune/write_bytes_sec)",
+                ret = virXPathULongLong("string(./iotune/write_bytes_sec)",
                                       ctxt,
-                                      &def->blkdeviotune.write_bytes_sec) < 0) {
+                                      &def->blkdeviotune.write_bytes_sec);
+                if (ret < 0 && ret != -2) {
                     def->blkdeviotune.write_bytes_sec = 0;
+                } else if (ret == -2) {
+                    virReportError(VIR_ERR_XML_ERROR, "%s",
+                                   _("write throughput limit must be an integer"));
+                    goto error;
                 }
 
-                if (virXPathULongLong("string(./iotune/total_iops_sec)",
+                ret = virXPathULongLong("string(./iotune/total_iops_sec)",
                                       ctxt,
-                                      &def->blkdeviotune.total_iops_sec) < 0) {
+                                      &def->blkdeviotune.total_iops_sec);
+                if (ret < 0 && ret != -2) {
                     def->blkdeviotune.total_iops_sec = 0;
+                } else if (ret == -2) {
+                    virReportError(VIR_ERR_XML_ERROR, "%s",
+                                   _("total I/O operations limit must be an integer"));
+                    goto error;
                 }
 
-                if (virXPathULongLong("string(./iotune/read_iops_sec)",
+                ret = virXPathULongLong("string(./iotune/read_iops_sec)",
                                       ctxt,
-                                      &def->blkdeviotune.read_iops_sec) < 0) {
+                                      &def->blkdeviotune.read_iops_sec);
+                if (ret < 0 && ret != -2) {
                     def->blkdeviotune.read_iops_sec = 0;
+                } else if (ret == -2) {
+                    virReportError(VIR_ERR_XML_ERROR, "%s",
+                                   _("read I/O operations limit must be an integer"));
+                    goto error;
                 }
 
-                if (virXPathULongLong("string(./iotune/write_iops_sec)",
+                ret = virXPathULongLong("string(./iotune/write_iops_sec)",
                                       ctxt,
-                                      &def->blkdeviotune.write_iops_sec) < 0) {
+                                      &def->blkdeviotune.write_iops_sec);
+                if (ret < 0 && ret != -2) {
                     def->blkdeviotune.write_iops_sec = 0;
+                } else if (ret == -2) {
+                    virReportError(VIR_ERR_XML_ERROR, "%s",
+                                   _("write I/O operations limit must be an integer"));
+                    goto error;
                 }
 
                 if ((def->blkdeviotune.total_bytes_sec &&
-- 
1.9.3


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