Re: [libvirt] [PATCH 08/11] conf: Use domainPostParseData(Alloc|Free) in virDomainDefValidate
On 9/20/18 1:44 PM, Marc Hartmayer wrote:
For the usage of the parameter @parseOpqaue within
virDomainDefValidate it must be ensured that the parameter
@parseOpaque is not NULL. But since there are code paths where
virDomainDefValidate is called with @parseOpaque ==
NULL (e.g. the function call of virDomainDefParseNode with @parseOpqaue ==
NULL leads to this).
To prevent this, domainPostParseDataAlloc is called within
virDomainDefValidate to ensure that @parseOpaque is always set. The
usage of domainPostParseDataAlloc within virDomainDefValidate is
allowed since virDomainDefValidate is only called after
virDomainDefPostParse.
Signed-off-by: Marc Hartmayer <mhartmay(a)linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
src/conf/domain_conf.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
Thus is similar to commit e168bc8a did for virDomainDefPostParse...
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ae7f3ed95faf..4f1c569b5733 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6288,6 +6288,9 @@ virDomainDefValidate(virDomainDefPtr def,
virDomainXMLOptionPtr xmlopt,
void *parseOpaque)
{
+ int ret = -1;
+ bool localParseOpaque = false;
+
struct virDomainDefPostParseDeviceIteratorData data = {
.caps = caps,
.xmlopt = xmlopt,
@@ -6299,6 +6302,17 @@ virDomainDefValidate(virDomainDefPtr def,
if (parseFlags & VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)
return 0;
+ if (!data.parseOpaque &&
+ xmlopt->config.domainPostParseDataAlloc) {
+ ret = xmlopt->config.domainPostParseDataAlloc(def, caps, parseFlags,
+ xmlopt->config.priv,
+ &data.parseOpaque);
So ret = 1 if virQEMUCapsCacheLookup failed in
qemuDomainPostParseDataAlloc; otherwise, ret = 0; Just looks strange
below with the ret == 1 check.
+
+ if (virDomainDefPostParseCheckFailure(def, parseFlags, ret) < 0)
This though has/uses specific @parseFlag options and sets
def->postParseFailed when it may not be true if for some reason of
course the post parse processing never got the parseOpaque value.
Wouldn't it perhaps be problematic in the patch3 scenario where you'd be
passing the priv->qemuCaps when @postParseFailed?
John
+ goto cleanup;
+ localParseOpaque = true;
+ }
+
/* call the domain config callback */
if (xmlopt->config.domainValidateCallback &&
xmlopt->config.domainValidateCallback(def, caps, xmlopt->config.priv,
data.parseOpaque) < 0)
@@ -6313,6 +6327,13 @@ virDomainDefValidate(virDomainDefPtr def,
if (virDomainDefValidateInternal(def) < 0)
return -1;
+ cleanup:
+ if (localParseOpaque && xmlopt->config.domainPostParseDataFree)
+ xmlopt->config.domainPostParseDataFree(data.parseOpaque);
+
+ if (ret == 1)
+ return -1;
+
return 0;
}