Re: [libvirt] [PATCH v4 1/5] migration: add compress method option
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