From: Chun Feng Wu <danielwuwy@163.com>

Refactor iotune verification, and verify some rules

* Disk iotune validation can be reused for throttle group validation,
  refactor it into common method "virDomainDiskIoTuneValidate"
* Add "virDomainDefValidateThrottleGroups" to validate throttle groups,
  which in turn calls "virDomainDiskIoTuneValidate"
* Make sure referenced throttle group exists
* Use "iotune" and "throttlefilters" exclusively for specific disk
* Throttle filters cannot be used together with CDROM

Signed-off-by: Chun Feng Wu <danielwuwy@163.com>
---
 src/conf/domain_validate.c | 119 ++++++++++++++++++++++++++-----------
 src/qemu/qemu_driver.c     |   6 ++
 src/qemu/qemu_hotplug.c    |   8 +++
 3 files changed, 99 insertions(+), 34 deletions(-)

diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index b352cd874a..916d5c9986 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -659,11 +659,55 @@ 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)
 {
     virStorageSource *next;
+    size_t i;
 
     /* disk target is used widely in other code so it must be validated first */
     if (!disk->dst) {
@@ -713,41 +757,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"));
+    if (virDomainDiskIoTuneValidate(disk->blkdeviotune) < 0)
         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
@@ -943,6 +954,26 @@ virDomainDiskDefValidate(const virDomainDef *def,
         return -1;
     }
 
+    /* referenced group should be defined */
+    for (i = 0; i < disk->nthrottlefilters; i++) {
+        virDomainThrottleFilterDef *filter = disk->throttlefilters[i];
+        if (!virDomainThrottleGroupByName(def, filter->group_name)) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("throttle group '%1$s' not found"),
+                           filter->group_name);
+            return -1;
+        }
+    }
+
+    if (disk->throttlefilters &&
+        (disk->blkdeviotune.group_name ||
+        virDomainBlockIoTuneInfoHasAny(&disk->blkdeviotune))) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+                       _("block 'throttlefilters' can't be used together with 'iotune' for disk '%1$s'"),
+                       disk->dst);
+        return -1;
+    }
+
     return 0;
 }
 
@@ -1843,6 +1874,23 @@ virDomainDefLaunchSecurityValidate(const virDomainDef *def)
 
 #undef CHECK_BASE64_LEN
 
+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)
@@ -1901,6 +1949,9 @@ virDomainDefValidateInternal(const virDomainDef *def,
     if (virDomainDefLaunchSecurityValidate(def) < 0)
         return -1;
 
+    if (virDomainDefValidateThrottleGroups(def) < 0)
+        return -1;
+
     return 0;
 }
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d65d5fd2ff..2a5f58635c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14862,6 +14862,12 @@ qemuDomainDiskBlockIoTuneIsSupported(virDomainDiskDef *disk)
         return false;
     }
 
+    if (disk->throttlefilters) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("block 'iotune' can't be used together with 'throttlefilters' for disk '%1$s'"), disk->dst);
+        return false;
+    }
+
     return true;
 }
 
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index bf001aa902..8c41d27704 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -988,6 +988,14 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver *driver,
     bool releaseSeclabel = false;
     int ret = -1;
 
+    if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
+        if (disk->nthrottlefilters > 0) {
+            virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                        _("cdrom device with throttle filters isn't supported"));
+            return -1;
+        }
+    }
+
     if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
                        _("floppy device hotplug isn't supported"));
-- 
2.39.5 (Apple Git-154)