[PATCH 01/21] domain_conf.c: move NVDIMM 'labelsize' check to post parse

Michal Privoznik mprivozn at redhat.com
Tue Dec 1 18:46:23 UTC 2020


On 11/24/20 8:20 PM, Daniel Henrique Barboza wrote:
> Move 'labelsize' validation to virDomainMemoryDefPostParse().
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
> ---
>   src/conf/domain_conf.c | 43 +++++++++++++++++++++---------------------
>   1 file changed, 22 insertions(+), 21 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b1534dcc1e..5e5905f483 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -5363,15 +5363,28 @@ static int
>   virDomainMemoryDefPostParse(virDomainMemoryDefPtr mem,
>                               const virDomainDef *def)
>   {
> -    /* Although only the QEMU driver implements PPC64 support, this
> -     * code is related to the platform specification (PAPR), i.e. it
> -     * is hypervisor agnostic, and any future PPC64 hypervisor driver
> -     * will have the same restriction.
> -     */
> -    if (ARCH_IS_PPC64(def->os.arch) &&
> -        mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM &&
> -        virDomainNVDimmAlignSizePseries(mem) < 0)
> -        return -1;
> +    if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
> +        if (mem->labelsize && mem->labelsize < 128) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("nvdimm label must be at least 128KiB"));
> +            return -1;
> +        }
> +
> +        if (mem->labelsize >= mem->size) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("label size must be smaller than NVDIMM size"));
> +            return -1;
> +        }
> +
> +        /* Although only the QEMU driver implements PPC64 support, this
> +         * code is related to the platform specification (PAPR), i.e. it
> +         * is hypervisor agnostic, and any future PPC64 hypervisor driver
> +         * will have the same restriction.
> +         */
> +        if (ARCH_IS_PPC64(def->os.arch) &&
> +            virDomainNVDimmAlignSizePseries(mem) < 0)
> +            return -1;
> +    }

For this and the rest of patches - shouldn't changes like this go into 
validator callback? I view post parse callbacks as "fill missing values" 
not a place to check if configuration makes sense/is valid.

Michal




More information about the libvir-list mailing list