[libvirt] [PATCH 06/11] conf: Get rid of virDomainDeviceDefPostParseOne

[Marc Hartmayer]

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