On Sat, Sep 29, 2018 at 04:09 AM +0200, John Ferlan <jferlan@xxxxxxxxxx> 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@xxxxxxxxxxxxx>
>> Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>
>> ---
>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
