On Mon, Jan 11, 2021 at 12:50:01 +0300, Nikolay Shirokovskiy wrote:
> Currenly API is not very convinient when switching from read/write to total
> tunes back and forth. read/write and total tunes can not be set simulaneously
> so one need to choose one. Now if for example total_bytes_sec and
> total_bytes_sec_max are set and we set read_bytes_sec only then API fails. The
> issue is new max settings are copied from the old ones in this case and as
> a result after defaults are applied we got settings for read_bytes_sec and
> total_bytes_sec_max which is incorrect.
>
> In order to handle such situation nicely let's reset max settings if
> appropriate avg settings is reseted explicitly or implicitly.
NACK, IMO we must not change any fields which the user didn't touch.
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
> ---
> src/qemu/qemu_driver.c | 31 ++++++++++++++++++++-----------
> 1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 117c7b7..9093baf 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -15901,6 +15901,22 @@ qemuDomainSetBlockIoTuneDefaults(virDomainBlockIoTuneInfoPtr newinfo,
> SET_IOTUNE_DEFAULTS(IOPS_MAX, iops_sec_max);
> #undef SET_IOTUNE_DEFAULTS
>
> +#define RESET_IOTUNE_MAX(BOOL, FIELD) \
Also the series sells removing macros, so adding new ones isn't very
productive ;)
> + do { \
> + if (set_fields & QEMU_BLOCK_IOTUNE_SET_##BOOL) { \
> + if (!newinfo->total_##FIELD) \
> + newinfo->total_##FIELD##_max = 0; \
> + if (!newinfo->read_##FIELD) \
> + newinfo->read_##FIELD##_max = 0; \
> + if (!newinfo->write_##FIELD) \
> + newinfo->write_##FIELD##_max = 0; \
> + } \
> + } while (0)
> +
> + RESET_IOTUNE_MAX(BYTES, bytes_sec);
> + RESET_IOTUNE_MAX(IOPS, iops_sec);
> +#undef RESET_IOTUNE_MAX
