[PATCH] error message in virDomainDiskDefIotuneParse shifted to virDomainDefPostParseCheck

[Peter Krempa]

On Thu, Mar 24, 2022 at 21:39:56 +0530, Jamm02 wrote:
> From: Moteen Shah <moteenshah.02 at gmail.com>
> 
> domain_conf.c: all the option collision total and... error messages in
> virDomainDiskDefIotuneParse shifted to new function virDomainDefPostParseCheck

I don't quite understand what you mean by this commit message. Is it
meant to signal that it's just pure code movement?

Additionally we put the impacted file in the summary line so eg.

conf: Move validation from virDomainDiskDefIotuneParse into the validation callback

Also per https://libvirt.org/hacking.html#developer-certificate-of-origin

Your submission is missing a certification of compliance with the
"Developer certificate of origin".

> ---
>  src/conf/domain_conf.c | 86 ++++++++++++++++++++++--------------------
>  1 file changed, 45 insertions(+), 41 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 153954a0b0..f2480f37f6 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6229,6 +6229,50 @@ virDomainDefPostParseCheckFailure(virDomainDef *def,
>      return 0;
>  }
>  
> +static int
> +virDomainDefPostParseCheck(virDomainDiskDef *def)

This function is really miss-named since it operates on a
"virDomainDiskDef" object it should contain that in the name as
"virDomainDef" is a separate object

Also I suppose you were told to move this to "post parse checks" but
what was meant is the already existing set of functions in this case in
src/conf/domain_validate.c since this code is just validation of the
input.

Just extracting this here makes not that much sense by itself.

> +{
> +    if ((def->blkdeviotune.total_bytes_sec &&
> +         def->blkdeviotune.read_bytes_sec) ||
> +        (def->blkdeviotune.total_bytes_sec &&
> +         def->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;
> +    }