[v1] Correct two minor QMP interface design flaws

[Qemu-devel] [PATCH for-2.10 08/10] migration: Add TODO comments on duplication of QAPI_CLONE()

Posted by Markus Armbruster 8 years, 6 months ago

qmp_query_migrate_parameters() and qmp_migrate_set_parameters()
effectively duplicate QAPI_CLONE() inline.  Add suitable TODO
comments.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 migration/migration.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/migration/migration.c b/migration/migration.c
index bae9808..d0a1d13 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -427,6 +427,7 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
     MigrationParameters *params;
     MigrationState *s = migrate_get_current();
 
+    /* TODO use QAPI_CLONE() instead of duplicating it inline */
     params = g_malloc0(sizeof(*params));
     params->has_compress_level = true;
     params->compress_level = s->parameters.compress_level;
@@ -703,6 +704,7 @@ void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
                     "is invalid, it should be positive");
     }
 
+    /* TODO use QAPI_CLONE() instead of duplicating it inline */
     if (params->has_compress_level) {
         s->parameters.compress_level = params->compress_level;
     }
-- 
2.7.5

Re: [Qemu-devel] [PATCH for-2.10 08/10] migration: Add TODO comments on duplication of QAPI_CLONE()

Posted by Daniel P. Berrange 8 years, 6 months ago

On Tue, Jul 18, 2017 at 03:41:24PM +0200, Markus Armbruster wrote:
> qmp_query_migrate_parameters() and qmp_migrate_set_parameters()
> effectively duplicate QAPI_CLONE() inline.  Add suitable TODO
> comments.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  migration/migration.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>

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 :|

Re: [Qemu-devel] [PATCH for-2.10 08/10] migration: Add TODO comments on duplication of QAPI_CLONE()

Posted by Eric Blake 8 years, 6 months ago

On 07/18/2017 08:41 AM, Markus Armbruster wrote:
> qmp_query_migrate_parameters() and qmp_migrate_set_parameters()
> effectively duplicate QAPI_CLONE() inline.  Add suitable TODO
> comments.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  migration/migration.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org