On 12/1/20 8:58 PM, Daniel Henrique Barboza wrote:
On 12/1/20 3:46 PM, Michal Privoznik wrote:
On 11/24/20 8:20 PM, Daniel Henrique Barboza wrote:
Move 'labelsize' validation to virDomainMemoryDefPostParse().
Signed-off-by: Daniel Henrique Barboza <danielhb413@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.
You mean these callbacks?
---- domain_conf.h ----
/* validation callbacks */ virDomainDefValidateCallback domainValidateCallback; virDomainDeviceDefValidateCallback deviceValidateCallback;
I mean virDomainDefValidate() and more specifically virDomainDefValidateInternal(). Driver specific callbacks are out of question - exactly for the reason you pointed out. Michal