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