On Tue, Jan 15, 2019 at 10:03:53AM -0600, Eric Blake wrote: > On 1/15/19 4:24 AM, Dr. David Alan Gilbert wrote: > > > I think the problem is that > > migrate_params_check checks a MigrationParameters > > > > while the QMP command gives us a MigrateSetParameters; but we also use > > migrate_params_check for the global check you added (8b0b29dc) which is > > against migrationParameters; so that's why migrate_params_check takes > > a MigrationParameters. > > > > It's horrible we've got stuff duped so much. > > Indeed. > > > > > However, I don't like this fix because if someone later was to add > > a test for tls parameters to migrate_params_check, then they would be > > confused why the hostname/creds weren't checked. > > So while we have migrate_params_test_apply, it should cover all > > parameters. > > > > I think a cleaner check would be to write a MigrateParameters_free > > that free'd any strings, and call that in qmp_migrate_set_parameters > > on both exit paths. > > We already have it; it's named qapi_free_MigrationParameters(), > generated in qapi-types-migration.h. Yes this seems better. Then IIUC patch 3 can be simplified as well with it. I'm doing one step back and reading below thread for more context that I've missed: https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg04526.html Do we have chance/plan to remove these duplication for QEMU 4.0? Thanks, > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3226 > Virtualization: qemu.org | libvirt.org > Regards, -- Peter Xu
