On Thu, Jan 30, 2020 at 09:03:00AM +0100, Markus Armbruster wrote:
> Juan Quintela <quint...@redhat.com> writes:
>
> > It will indicate which level use for compression.
> >
> > Signed-off-by: Juan Quintela <quint...@redhat.com>
>
> This is slightly confusing (there is no zlib compression), unless you
> peek at the next patch (which adds zlib compression).
>
> Three ways to make it less confusing:
>
> * Squash the two commits
>
> * Swap them: first add zlib compression with level hardcoded to 1, then
> make the level configurable.
>
> * Have the first commit explain itself better. Something like
>
> multifd: Add multifd-zlib-level parameter
>
> This parameter specifies zlib compression level. The next patch
> will put it to use.
Wouldn't the "normal" best practice for QAPI design be to use a
enum and discriminated union. eg
{ 'enum': 'MigrationCompression',
'data': ['none', 'zlib'] }
{ 'struct': 'MigrationCompressionParamsZLib',
'data': { 'compression-level' } }
{ 'union': 'MigrationCompressionParams',
'base': { 'mode': 'MigrationCompression' },
'discriminator': 'mode',
'data': {
'zlib': 'MigrationCompressionParamsZLib',
}
Of course this is quite different from how migration parameters are
done today. Maybe it makes sense to stick with the flat list of
migration parameters for consistency & ignore normal QAPI design
practice ?
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
