On Sat, Sep 29, 2018 at 05:34 PM +0200, John Ferlan <jferlan@xxxxxxxxxx> wrote: > On 9/20/18 1:44 PM, Marc Hartmayer wrote: >> Add @parseOpaque argument to virDomainDefValidate and >> virDomainDefValidateCallback, but don't use it for now since it's not >> ensured that it's always a non-NULL value. >> >> Signed-off-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxx> >> Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx> >> --- >> src/conf/domain_conf.c | 11 +++++++---- >> src/conf/domain_conf.h | 6 ++++-- >> src/qemu/qemu_domain.c | 3 ++- >> src/qemu/qemu_process.c | 2 +- >> src/vz/vz_driver.c | 3 ++- >> 5 files changed, 16 insertions(+), 9 deletions(-) >> > > I like this idea especially since the Validate paths are the ones where > qemuCaps seem to be most useful. > > Couple of nits below > > John > >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index e61f04ea2271..ae7f3ed95faf 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -6271,6 +6271,7 @@ virDomainDefValidateInternal(const virDomainDef *def) >> * @caps: driver capabilities object >> * @parseFlags: virDomainDefParseFlags >> * @xmlopt: XML parser option object >> + * @parseOpaque: opaque data and it might be NULL (for QEMU driver it's qemuCaps) >> * >> * This validation function is designed to take checks of globally invalid >> * configurations that the parser needs to accept so that VMs don't vanish upon >> @@ -6284,12 +6285,14 @@ int >> virDomainDefValidate(virDomainDefPtr def, >> virCapsPtr caps, >> unsigned int parseFlags, >> - virDomainXMLOptionPtr xmlopt) >> + virDomainXMLOptionPtr xmlopt, >> + void *parseOpaque) >> { >> struct virDomainDefPostParseDeviceIteratorData data = { >> .caps = caps, >> .xmlopt = xmlopt, >> .parseFlags = parseFlags, >> + .parseOpaque = parseOpaque, >> }; >> >> /* validate configuration only in certain places */ >> @@ -6298,7 +6301,7 @@ virDomainDefValidate(virDomainDefPtr def, >> >> /* call the domain config callback */ >> if (xmlopt->config.domainValidateCallback && >> - xmlopt->config.domainValidateCallback(def, caps, xmlopt->config.priv) < 0) >> + xmlopt->config.domainValidateCallback(def, caps, xmlopt->config.priv, data.parseOpaque) < 0) >> return -1; >> >> /* iterate the devices */ >> @@ -21063,7 +21066,7 @@ virDomainObjParseXML(xmlDocPtr xml, >> goto error; >> >> /* valdiate configuration */ > > May as well fix the typo above *validate Will change. > >> - if (virDomainDefValidate(obj->def, caps, flags, xmlopt) < 0) >> + if (virDomainDefValidate(obj->def, caps, flags, xmlopt, parseOpaque) < 0) >> goto error; >> >> return obj; >> @@ -21154,7 +21157,7 @@ virDomainDefParseNode(xmlDocPtr xml, >> goto cleanup; >> >> /* valdiate configuration */ > > Consistency is the key ;-) Yep :D > >> - if (virDomainDefValidate(def, caps, flags, xmlopt) < 0) >> + if (virDomainDefValidate(def, caps, flags, xmlopt, parseOpaque) < 0) >> goto cleanup; >> […snip…] >> static int >> vzDomainDefValidate(const virDomainDef *def, >> virCapsPtr caps ATTRIBUTE_UNUSED, >> - void *opaque) >> + void *opaque, >> + void *parserOpaque ATTRIBUTE_UNUSED) > > nit: @parseOpaque Okay. > >> { >> if (vzCheckUnsupportedControllers(def, opaque) < 0) >> return -1; >> > Thanks for the review! -- 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list