[libvirt] [PATCH] conf: Introduce virDomainDefCputuneValidate helper

Erik Skultety eskultet at redhat.com
Thu Mar 14 09:08:25 UTC 2019


On Wed, Mar 13, 2019 at 09:54:31PM -0600, Suyang Chen wrote:
> move all the def->cputune 'period must be in range'
> and 'quota must be in range' errors into two macros
> and called in virDomainDefCputuneValidate function
> and have it called from virDomainDefValidateInternal function

I'll reformulate ^this a tiny bit to indicate we also moved the checks from the
'parse' phase into the 'validation' phase. For future reference, make sure you
start a sentence with a capital letter and end it with a period ;).

>
> Reason: Two macros maybe easier to debug and change in the future
>
> Solve the first two small tasks in bitsizedtask:
> "Move validation checks out of domain XML parsing"

This doesn't provide much information and in fact should be dropped from the
commit message.

>
> Signed-off-by: Suyang Chen <dawson0xff at gmail.com>
> ---
>  src/conf/domain_conf.c | 109 +++++++++++++++--------------------------
>  1 file changed, 40 insertions(+), 69 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 356a9ae56c..c159cffa05 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6565,6 +6565,43 @@ virDomainDefLifecycleActionValidate(const virDomainDef *def)
>      return 0;
>  }
>
> +#define CPUTUNE_VALIDATE_PERIOD(name) \
> +    if (def->cputune.name > 0 && \
> +        (def->cputune.name < 1000 || def->cputune.name > 1000000)) { \
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \
> +                       _("Value of cputune '%s' must be in range " \
> +                       "[1000, 1000000]"), #name); \
> +        return -1; \
> +    }
> +
> +#define CPUTUNE_VALIDATE_QUOTA(name) \
> +    if (def->cputune.name > 0 && \
> +        (def->cputune.name < 1000 || \
> +        def->cputune.name > 18446744073709551LL)) { \
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \
> +                       _("Value of cputune '%s' must be in range " \
> +                       "[1000, 18446744073709551]"), #name); \
> +        return -1; \
> +    }
> +

2 macros are fine, maybe even more readable. One thing though, although we
don't enforce this and often forget, the preference for writing preprocesor
macros is by using do-while(0), which has a neat side effect that you can
reliably call the macro as a function. So, I'll convert them before merging.

Reviewed-by: Erik Skultety <eskultet at redhat.com>




More information about the libvir-list mailing list