[PATCH 02/23] qemu: factor out qemuValidateDomainBlkdeviotune

[Nikolay Shirokovskiy]

ср, 3 мар. 2021 г. в 15:54, Peter Krempa <pkrempa at 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 at 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 at redhat.com>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20210311/ec3b833e/attachment-0001.htm>