On Sat, Sep 29, 2018 at 04:09 AM +0200, John Ferlan <jferlan@redhat.com> wrote:
On 9/20/18 1:44 PM, Marc Hartmayer wrote:
Move the domainPostParseDataAlloc/Free calls to virDomainDeviceDefParse. As an consequence virDomainDeviceDefPostParseOne is superfluous and can therefore be removed.
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/conf/domain_conf.c | 37 +++++++++++-------------------------- 1 file changed, 11 insertions(+), 26 deletions(-)
I'm not against this per se; however, I should not that the code was specifically extracted in commit e168bc8a.
There are the following three functions: virDomainDeviceDefParse virDomainDeviceDefPostParse virDomainDeviceDefPostParseOne Peter introduced the function “virDomainDeviceDefPostParseOne” to avoid the allocation of private data across the callbacks. This is absolutely fine. What I’ve done is, I moved the domainPostParseDataAlloc/Free calls to virDomainDeviceDefParse instead of having an extra wrapper function (virDomainDeviceDefPostParse/One) for that. With this change I can reuse the QEMU caps for the virDomainDeviceDefValidate call in virDomainDeviceDefParse as well. For safety I added Peter to CC.
John
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3c307d325a89..e61f04ea2271 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4900,31 +4900,6 @@ virDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, return 0; }
-static int -virDomainDeviceDefPostParseOne(virDomainDeviceDefPtr dev, - const virDomainDef *def, - virCapsPtr caps, - unsigned int flags, - virDomainXMLOptionPtr xmlopt) -{ - void *parseOpaque = NULL; - int ret; - - if (xmlopt->config.domainPostParseDataAlloc) { - if (xmlopt->config.domainPostParseDataAlloc(def, caps, flags, - xmlopt->config.priv, - &parseOpaque) < 0) - return -1; - } - - ret = virDomainDeviceDefPostParse(dev, def, caps, flags, xmlopt, parseOpaque); - - if (parseOpaque && xmlopt->config.domainPostParseDataFree) - xmlopt->config.domainPostParseDataFree(parseOpaque); - - return ret; -} -
struct virDomainDefPostParseDeviceIteratorData { virCapsPtr caps; @@ -16066,6 +16041,7 @@ virDomainDeviceDefParse(const char *xmlStr, { xmlDocPtr xml; xmlNodePtr node; + void *parseOpaque = NULL; xmlXPathContextPtr ctxt = NULL; virDomainDeviceDefPtr dev = NULL; char *netprefix; @@ -16222,8 +16198,15 @@ virDomainDeviceDefParse(const char *xmlStr, break; }
+ if (xmlopt->config.domainPostParseDataAlloc) { + if (xmlopt->config.domainPostParseDataAlloc(def, caps, flags, + xmlopt->config.priv, + &parseOpaque) < 0) + goto error; + } + /* callback to fill driver specific device aspects */ - if (virDomainDeviceDefPostParseOne(dev, def, caps, flags, xmlopt) < 0) + if (virDomainDeviceDefPostParse(dev, def, caps, flags, xmlopt, parseOpaque) < 0) goto error;
/* validate the configuration */ @@ -16231,6 +16214,8 @@ virDomainDeviceDefParse(const char *xmlStr, goto error;
cleanup: + if (parseOpaque && xmlopt->config.domainPostParseDataFree) + xmlopt->config.domainPostParseDataFree(parseOpaque); xmlFreeDoc(xml); xmlXPathFreeContext(ctxt); return dev;
-- Kind regards / Beste Grüße Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294