[libvirt] [PATCH v4 1/5] migration: add compress method option

[Nikolay Shirokovskiy]

On 31.03.2016 09:53, Jiri Denemark wrote:
> On Thu, Mar 24, 2016 at 11:08:31 +0300, Nikolay Shirokovskiy wrote:
>>> The backward compatibility code will be limited to Parse/Dump parameters
>>> and the rest of the code will just see a nice struct of compression
>>> parameters without having to worry about how it was called. And dealing
>>> with it in *Dump is easy, just set don't output any parameters if only
>>> XBZRLE method was selected and no compression parameter is set.
>>
>> Then I should never pass NULL value of compression pointer anywhere.
>> That is I should fix all the places where this is done in current version.
>> Below is the complete list of functions where NULL is passed:
>>
>> qemuDomainMigratePrepare2
>> qemuDomainMigratePrepare3
>> qemuDomainMigratePerform
>> qemuDomainMigratePerform3
> 
> None of these accepts any compression parameters (except for the flag
> itself) so you could use a simple struct with only xbzrle set (or not
> depending on the flag) and place it on the stack, but the code would
> look cleaner if you just called the parse/free function too even though
> they wouldn't do much without any typed parameters passed in.
> 
>> qemuMigrationPrepareTunnel
>> doPeer2PeerMigrate2
>> qemuMigrationPerformJob
>>
>> I need to add code to parse/free in every of this places. This is
>> my concern. Except this I totally agree that trying to put
>> backward compatibility issues in one place like parse/dump 
>> functions is good. But on this way I still need to touch
>> a lot of places in less trivial way (in comparsion to
>> don't mixing flags / compression parameters).
> 
> Right, but you touch various entry points and the internal code that
> actually does something interesting doesn't need to care about the
> origin of compression parameters. That said, I didn't really try
> changing the code to see how it looks like but having a single structure
> describing compression methods and parameters no matter how they were
> specified by a user seems cleaner to me.
> 
> Jirka
> 

Thanx for clarification!

Nikolay