qemu: validate: Validate blkdeviotune settings in the validator

Move the code from qemuCheckDiskConfigBlkdeviotune in
src/qemu/qemu_commandline.c to
qemuValidateDomainDeviceDefDiskBlkdeviotune.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 269bdbaf56b639a8077dfac318a7cba9807cccaf..87cb78e3505febde327cff0221049b40a6d2f190 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1131,102 +1131,6 @@ qemuDiskConfigBlkdeviotuneEnabled(virDomainDiskDefPtr disk)
 }
 
 
-/**
- * qemuCheckDiskConfigBlkdeviotune:
- * @disk: disk configuration
- * @qemuCaps: qemu capabilities, NULL if checking cold-configuration
- *
- * Checks whether block io tuning settings make sense. Returns -1 on error and
- * reports a proper libvirt error.
- */
-static int
-qemuCheckDiskConfigBlkdeviotune(virDomainDiskDefPtr disk,
-                                const virDomainDef *def,
-                                virQEMUCapsPtr qemuCaps)
-{
-    /* group_name by itself is ignored by qemu */
-    if (disk->blkdeviotune.group_name &&
-        !virDomainBlockIoTuneInfoHasAny(&disk->blkdeviotune)) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("group_name can be configured only together with "
-                         "settings"));
-        return -1;
-    }
-
-    /* checking def here is only for calling from tests */
-    if (disk->blkdeviotune.group_name) {
-        size_t i;
-
-        for (i = 0; i < def->ndisks; i++) {
-            virDomainDiskDefPtr d = def->disks[i];
-
-            if (STREQ(d->dst, disk->dst) ||
-                STRNEQ_NULLABLE(d->blkdeviotune.group_name,
-                                disk->blkdeviotune.group_name))
-                continue;
-
-            if (!virDomainBlockIoTuneInfoEqual(&d->blkdeviotune,
-                                               &disk->blkdeviotune)) {
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                               _("different iotunes for disks %s and %s"),
-                               disk->dst, d->dst);
-                return -1;
-            }
-        }
-    }
-
-    if (disk->blkdeviotune.total_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
-        disk->blkdeviotune.read_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
-        disk->blkdeviotune.write_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
-        disk->blkdeviotune.total_iops_sec > QEMU_BLOCK_IOTUNE_MAX ||
-        disk->blkdeviotune.read_iops_sec > QEMU_BLOCK_IOTUNE_MAX ||
-        disk->blkdeviotune.write_iops_sec > QEMU_BLOCK_IOTUNE_MAX ||
-        disk->blkdeviotune.total_bytes_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
-        disk->blkdeviotune.read_bytes_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
-        disk->blkdeviotune.write_bytes_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
-        disk->blkdeviotune.total_iops_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
-        disk->blkdeviotune.read_iops_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
-        disk->blkdeviotune.write_iops_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
-        disk->blkdeviotune.size_iops_sec > QEMU_BLOCK_IOTUNE_MAX) {
-        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
-                      _("block I/O throttle limit must "
-                        "be no more than %llu using QEMU"), QEMU_BLOCK_IOTUNE_MAX);
-        return -1;
-    }
-
-    if (qemuCaps) {
-        /* block I/O throttling 1.7 */
-        if (virDomainBlockIoTuneInfoHasMax(&disk->blkdeviotune) &&
-            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX)) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("there are some block I/O throttling parameters "
-                             "that are not supported with this QEMU binary"));
-            return -1;
-        }
-
-        /* block I/O group 2.4 */
-        if (disk->blkdeviotune.group_name &&
-            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_GROUP)) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("the block I/O throttling group parameter is "
-                             "not supported with this QEMU binary"));
-            return -1;
-        }
-
-        /* block I/O throttling length 2.6 */
-        if (virDomainBlockIoTuneInfoHasMaxLength(&disk->blkdeviotune) &&
-            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH)) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("there are some block I/O throttling length parameters "
-                             "that are not supported with this QEMU binary"));
-            return -1;
-        }
-    }
-
-    return 0;
-}
-
-
 /**
  * qemuCheckDiskConfig:
  * @disk: disk definition
@@ -1237,12 +1141,9 @@ qemuCheckDiskConfigBlkdeviotune(virDomainDiskDefPtr disk,
  */
 int
 qemuCheckDiskConfig(virDomainDiskDefPtr disk,
-                    const virDomainDef *def,
+                    const virDomainDef *def G_GNUC_UNUSED,
                     virQEMUCapsPtr qemuCaps)
 {
-    if (qemuCheckDiskConfigBlkdeviotune(disk, def, qemuCaps) < 0)
-        return -1;
-
     if (disk->wwn) {
         if ((disk->bus != VIR_DOMAIN_DISK_BUS_IDE) &&
             (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI)) {

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 6f13a1df1b02dd8d4cd2665f930780bb5dd5cb6c..17cfcddd30612645afca702a6f8ddd9a184a2b13 100644 (file)
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1956,8 +1956,105 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk)
 }
 
 
+/**
+ * qemuValidateDomainDeviceDefDiskBlkdeviotune:
+ * @disk: disk configuration
+ * @qemuCaps: qemu capabilities, NULL if checking cold-configuration
+ *
+ * Checks whether block io tuning settings make sense. Returns -1 on error and
+ * reports a proper libvirt error.
+ */
+static int
+qemuValidateDomainDeviceDefDiskBlkdeviotune(const virDomainDiskDef *disk,
+                                            const virDomainDef *def,
+                                            virQEMUCapsPtr qemuCaps)
+{
+    /* group_name by itself is ignored by qemu */
+    if (disk->blkdeviotune.group_name &&
+        !virDomainBlockIoTuneInfoHasAny(&disk->blkdeviotune)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("group_name can be configured only together with "
+                         "settings"));
+        return -1;
+    }
+
+    /* checking def here is only for calling from tests */
+    if (disk->blkdeviotune.group_name) {
+        size_t i;
+
+        for (i = 0; i < def->ndisks; i++) {
+            virDomainDiskDefPtr d = def->disks[i];
+
+            if (STREQ(d->dst, disk->dst) ||
+                STRNEQ_NULLABLE(d->blkdeviotune.group_name,
+                                disk->blkdeviotune.group_name))
+                continue;
+
+            if (!virDomainBlockIoTuneInfoEqual(&d->blkdeviotune,
+                                               &disk->blkdeviotune)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("different iotunes for disks %s and %s"),
+                               disk->dst, d->dst);
+                return -1;
+            }
+        }
+    }
+
+    if (disk->blkdeviotune.total_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
+        disk->blkdeviotune.read_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
+        disk->blkdeviotune.write_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
+        disk->blkdeviotune.total_iops_sec > QEMU_BLOCK_IOTUNE_MAX ||
+        disk->blkdeviotune.read_iops_sec > QEMU_BLOCK_IOTUNE_MAX ||
+        disk->blkdeviotune.write_iops_sec > QEMU_BLOCK_IOTUNE_MAX ||
+        disk->blkdeviotune.total_bytes_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
+        disk->blkdeviotune.read_bytes_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
+        disk->blkdeviotune.write_bytes_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
+        disk->blkdeviotune.total_iops_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
+        disk->blkdeviotune.read_iops_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
+        disk->blkdeviotune.write_iops_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
+        disk->blkdeviotune.size_iops_sec > QEMU_BLOCK_IOTUNE_MAX) {
+        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+                      _("block I/O throttle limit must "
+                        "be no more than %llu using QEMU"), QEMU_BLOCK_IOTUNE_MAX);
+        return -1;
+    }
+
+    if (qemuCaps) {
+        /* block I/O throttling 1.7 */
+        if (virDomainBlockIoTuneInfoHasMax(&disk->blkdeviotune) &&
+            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("there are some block I/O throttling parameters "
+                             "that are not supported with this QEMU binary"));
+            return -1;
+        }
+
+        /* block I/O group 2.4 */
+        if (disk->blkdeviotune.group_name &&
+            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_GROUP)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("the block I/O throttling group parameter is "
+                             "not supported with this QEMU binary"));
+            return -1;
+        }
+
+        /* block I/O throttling length 2.6 */
+        if (virDomainBlockIoTuneInfoHasMaxLength(&disk->blkdeviotune) &&
+            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("there are some block I/O throttling length parameters "
+                             "that are not supported with this QEMU binary"));
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+
 int
 qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk,
+                                const virDomainDef *def,
                                 virQEMUCapsPtr qemuCaps)
 {
     const char *driverName = virDomainDiskGetDriver(disk);
@@ -1968,6 +2065,9 @@ qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk,
     if (qemuValidateDomainDeviceDefDiskFrontend(disk) < 0)
         return -1;
 
+    if (qemuValidateDomainDeviceDefDiskBlkdeviotune(disk, def, qemuCaps) < 0)
+        return -1;
+
     if (disk->src->shared && !disk->src->readonly &&
         !qemuBlockStorageSourceSupportsConcurrentAccess(disk->src)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -3679,7 +3779,7 @@ qemuValidateDomainDeviceDef(const virDomainDeviceDef *dev,
         break;
 
     case VIR_DOMAIN_DEVICE_DISK:
-        ret = qemuValidateDomainDeviceDefDisk(dev->data.disk, qemuCaps);
+        ret = qemuValidateDomainDeviceDefDisk(dev->data.disk, def, qemuCaps);
         break;
 
     case VIR_DOMAIN_DEVICE_CONTROLLER:

diff --git a/src/qemu/qemu_validate.h b/src/qemu/qemu_validate.h
index f667a57195c880681cea0105c376ba0a1de04a6f..acf7d26ce009e4bac02cf05068c869e9e98e528d 100644 (file)
--- a/src/qemu/qemu_validate.h
+++ b/src/qemu/qemu_validate.h
@@ -28,6 +28,7 @@
 
 int qemuValidateDomainDef(const virDomainDef *def, void *opaque);
 int qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk,
+                                    const virDomainDef *def,
                                     virQEMUCapsPtr qemuCaps);
 int qemuValidateDomainDeviceDef(const virDomainDeviceDef *dev,
                                 const virDomainDef *def,

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 80018075526891c3cf1d913475b2e0f2915620bd..fbe4972981852c0bf894b1f12f9bf1a55e2b344d 100644 (file)
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -284,7 +284,7 @@ testQemuDiskXMLToProps(const void *opaque)
         return -1;
 
     if (qemuCheckDiskConfig(disk, vmdef, data->qemuCaps) < 0 ||
-        qemuValidateDomainDeviceDefDisk(disk, data->qemuCaps) < 0) {
+        qemuValidateDomainDeviceDefDisk(disk, vmdef, data->qemuCaps) < 0) {
         VIR_TEST_VERBOSE("invalid configuration for disk");
         return -1;
     }