ср, 3 мар. 2021 г. в 15:54, Peter Krempa <pkrempa@redhat.com>:
On Mon, Jan 11, 2021 at 12:49:55 +0300, Nikolay Shirokovskiy wrote:
> It can also be used for validation of input in qemuDomainSetBlockIoTune.
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
> ---
>  src/qemu/qemu_validate.c | 100 ++++++++++++++++++++++++++---------------------
>  src/qemu/qemu_validate.h |   4 ++
>  2 files changed, 60 insertions(+), 44 deletions(-)
>
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index eadf3af..6a27565 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -2696,6 +2696,61 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk,
>  }
> 
> 
> +int
> +qemuValidateDomainBlkdeviotune(const virDomainBlockIoTuneInfo *iotune,
> +                               virQEMUCapsPtr qemuCaps)
> +{

The check that group_name must be set along with other fields :

    /* 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;
    }


also belongs here.

I cannot put this check into qemuValidateDomainBlkdeviotune. The thing is I'm going to reuse this function
in qemuDomainSetBlockIoTune and in qemuDomainSetBlockIoTune is it fine to have group_name only in input.
This means "move this disk to that io tune group" or "create io tune group and put this disk into it" depending
on whether io tune with that name exists or not (this is what qemuDomainSetBlockIoTuneDefaults do).


Nikolay

 


> +    if (iotune->total_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
> +        iotune->read_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
> +        iotune->write_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
> +        iotune->total_iops_sec > QEMU_BLOCK_IOTUNE_MAX ||
> +        iotune->read_iops_sec > QEMU_BLOCK_IOTUNE_MAX ||
> +        iotune->write_iops_sec > QEMU_BLOCK_IOTUNE_MAX ||
> +        iotune->total_bytes_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
> +        iotune->read_bytes_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
> +        iotune->write_bytes_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
> +        iotune->total_iops_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
> +        iotune->read_iops_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
> +        iotune->write_iops_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
> +        iotune->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;
> +    }

We also nowadays prefer if the error detail strings are not broken up,
but that's not a required change.

With the group name check moved too:

Reviewed-by: Peter Krempa <pkrempa@redhat.com>