[libvirt] [PATCH 07/11] conf: Extend virDomainDefValidate(Callback) for parseOpaque

[Marc Hartmayer]

On Sat, Sep 29, 2018 at 05:34 PM +0200, John Ferlan <jferlan at redhat.com> 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 at linux.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy at linux.ibm.com>
>> ---
>>  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