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

Re: [libvirt] [libvirt PATCH v2] Introduce virDomainChrDefNew()



On 04/14/2011 11:42 AM, Michal Novotny wrote:
> [snip]
> 
> I've fixed those OOMErrors and the whitespace already for v3 however I
> won't send the v3 of the patch until I get reply to this since some
> other approach may have to be chosen for the port = 0 things. comments
> inline ...
> 
> 
>> Hmm, why exactly are these port = 0 changes needed? Were they breaking
>> some tests?
> 
> 
> Exactly. It was breaking almost all of the xmconfigtests and
> sexpr2xmltests. The reason was that it was never set in the code to
> setup just one serial/parallel port and this is why it was breaking it.
> That's the reason for those changes to port = 0.
> 

Yeah, at first I didn't think they were necessary, but I see now that
they are. Reason being that we only do the port allocation when parsing
XML, not when formatting any random DomainDefPtr like I was assuming. So
it's better to have all the callers manually set a target port if
building a ChrDefPtr by hand. Sorry for the noise.

Thanks,
Cole

> [snip]
> 
>> I think these test files would be better named serial-target-port-auto.
> 
> Good, I'll do that name change for v3.
> 
> Thanks,
> Michal
> 


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