Re: [PATCH 03/12] domain: add implicit controllers from post parse

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/08/2016 07:05 AM, Peter Krempa wrote:
> On Thu, Jan 07, 2016 at 22:49:57 -0500, Cole Robinson wrote:
>> Seems like the natural fit, since we are already adding other XML bits
>> in the PostParse routine.
>>
>> Previously AddImplicitControllers was only called at the end of XML
>> parsing, meaning code that builds a DomainDef by hand had to manually
>> call it. Adding it for those sites causes some test suite churn.
>> ---
> 
> [...]
> 
>>  57 files changed, 59 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 2570f94..5b9dab9 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -3854,6 +3854,9 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
>>      if (virDomainDefPostParseTimer(def) < 0)
>>          return -1;
>>  
>> +    if (virDomainDefAddImplicitControllers(def) < 0)
>> +        return -1;
>> +
> 
> Moving it here makes this called twice in case you use
> qemuParseCommandLine or virVMXParseConfig.
> 

Right, the calling twice should be fine since this operation needs to be
idempotent as it's called every time we read the XML off disk for example.

Probably an opportunity for me cleanup but dropping some call sites causes
test suite churn that I didn't feel like dealing with at this point.

>>      /* clean up possibly duplicated metadata entries */
>>      virDomainDefMetadataSanitize(def);
>>  
> 
> The changes to the test suite look good to me but a more XEN
> knowledgeable person could comment on this fact possibly so that we are
> sure.
> 

ccing jfehlig

To expand a bit, I think the changes should be safe, it's only adding bits to
the output XML that xl/xm/sexpr code already needs to handle, since they will
show up in any XML defined by the user.

Thanks,
Cole

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]