Daniel P. Berrangé <berra...@redhat.com> wrote: > 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', > }
How is this translate into HMP? Markus says to start over, so lets see the dependencies: Announce: Allawys there announce-initial announce-max announce-rounds announce-step Osd compression (deprecated) compress-level compress-threads compress-wait-thread decompress-threads cpu-throttles-initial cpu-throottle-incroment max-cpu-throotle tls-creds tls-hostname tls-auth Real params max-bandwidth downtime-limit colo x-checkpoint-delay block-incremental multifd-channels xbzrle-cache-size max-postcopy-bandwidth New things: - multifd method - multifd-zlib-level - multifd-zstd-level What is a good way to define them? Why do I ask, because the current method is as bad as it can be. To add a new parameter: - for qapi, add it in three places (as Markus said) - go to hmp-cmds.c and do things by hand - qemu_migrate_set_parameters - migrate_params_check - migrate_params_apply (last three functions are almost identical in structure, not in content). So, if you can give me something that is _easier_ of maintaining, I am all ears. Later, Juan. > > 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