From: Chun Feng Wu <danielwuwy@xxxxxxx> 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@xxxxxxx> --- 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 1034bb57f5..03dfff93c0 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -685,11 +685,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) { @@ -739,41 +783,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 @@ -969,6 +980,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; } @@ -1869,6 +1900,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) @@ -1927,6 +1975,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 7217e4d881..78d1d83770 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14863,6 +14863,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 855ce9ca44..e341cac80c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -989,6 +989,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)