[libvirt] [PATCH 04/11] conf: Use getParseOpaque() in virDomainObjSetDefTransient

[Marc Hartmayer]

On Sat, Sep 29, 2018 at 04:06 AM +0200, John Ferlan <jferlan at redhat.com> wrote:
> On 9/20/18 1:44 PM, Marc Hartmayer wrote:
>> This allows the QEMU driver to use the originally used QEMU
>> capabilities for copying the domain. It avoids the usage of a possible
>> changed QEMU capability as virQEMUCapsCacheLookup might return a
>> different QEMU capability than used before (for example when during
>> the job the QEMU binary has been replaced virQEMUCapsCacheLookupCopy
>> will invalidate the originally used QEMU capability).
>>
>> For other drivers @parseOpqaue will still be NULL, because
>> xmlopt->privateData.getParseOpaque is currently only implemented for
>> the QEMU driver.
>>
>> Signed-off-by: Marc Hartmayer <mhartmay at linux.ibm.com>
>> Reviewed-by: Stefan Zimmermann <stzi at linux.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy at linux.ibm.com>
>> ---
>>  src/conf/domain_conf.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>
> This one I'm not so sure about. One would think the domain def that's
> being copied would have already gone through PostParse, et. al. The
> SetDefTransient is where we're copying the recently Parsed and Validated
> @def, so why would it need the qemuCaps especially since @flags would be
> PARSE_INACTIVE | PARSE_SKIP_VALIDATE in *DefCopy.

As far as I can see virDomainDefParseNode doesn’t skip the
virDomainDefPostParse part (there are the caps needed). Can you please
double check this?

An interesting scenario/test would be to see what happens if the QEMU
binary (and therefore the capabilities) has changed before the
virDomainObjSetDefTransient call.

>
> John
>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 1ee43950ae2d..a3f2fcb0a001 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -3366,6 +3366,7 @@ virDomainObjSetDefTransient(virCapsPtr caps,
>>                              virDomainObjPtr domain)
>>  {
>>      int ret = -1;
>> +    void *parseOpaque = NULL;
>>
>>      if (!domain->persistent)
>>          return 0;
>> @@ -3373,7 +3374,10 @@ virDomainObjSetDefTransient(virCapsPtr caps,
>>      if (domain->newDef)
>>          return 0;
>>
>> -    if (!(domain->newDef = virDomainDefCopy(domain->def, caps, xmlopt, NULL, false)))
>> +    if (xmlopt->privateData.getParseOpaque)
>> +        parseOpaque = xmlopt->privateData.getParseOpaque(domain);
>> +
>> +    if (!(domain->newDef = virDomainDefCopy(domain->def, caps, xmlopt, parseOpaque, false)))
>>          goto out;
>>
>>      ret = 0;
>>
>
--
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