[Qemu-devel] [PATCH v2 10/11] migration: provide migrate_cap_add()

Peter Xu posted 11 patches 8 years, 6 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v2 10/11] migration: provide migrate_cap_add()
Posted by Peter Xu 8 years, 6 months ago
Abstracted from migrate_set_block_enabled() to allocate
MigrationCapabilityStatusList properly.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/migration.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 52db5e7..300f84d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -833,14 +833,27 @@ void migrate_set_state(int *state, int old_state, int new_state)
     }
 }
 
-void migrate_set_block_enabled(bool value, Error **errp)
+static MigrationCapabilityStatusList *migrate_cap_add(
+    MigrationCapabilityStatusList *head,
+    MigrationCapability index,
+    bool state)
 {
     MigrationCapabilityStatusList *cap;
 
     cap = g_new0(MigrationCapabilityStatusList, 1);
     cap->value = g_new0(MigrationCapabilityStatus, 1);
-    cap->value->capability = MIGRATION_CAPABILITY_BLOCK;
-    cap->value->state = value;
+    cap->value->capability = index;
+    cap->value->state = state;
+    cap->next = head;
+
+    return cap;
+}
+
+void migrate_set_block_enabled(bool value, Error **errp)
+{
+    MigrationCapabilityStatusList *cap;
+
+    cap = migrate_cap_add(NULL, MIGRATION_CAPABILITY_BLOCK, value);
     qmp_migrate_set_capabilities(cap, errp);
     qapi_free_MigrationCapabilityStatusList(cap);
 }
-- 
2.7.4


Re: [Qemu-devel] [PATCH v2 10/11] migration: provide migrate_cap_add()
Posted by Juan Quintela 8 years, 6 months ago
Peter Xu <peterx@redhat.com> wrote:
> Abstracted from migrate_set_block_enabled() to allocate
> MigrationCapabilityStatusList properly.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>


Nitpick

> -void migrate_set_block_enabled(bool value, Error **errp)
> +static MigrationCapabilityStatusList *migrate_cap_add(
> +    MigrationCapabilityStatusList *head,

We have a parameter called head

> +    MigrationCapability index,
> +    bool state)
>  {
>      MigrationCapabilityStatusList *cap;
>  
>      cap = g_new0(MigrationCapabilityStatusList, 1);
>      cap->value = g_new0(MigrationCapabilityStatus, 1);
> -    cap->value->capability = MIGRATION_CAPABILITY_BLOCK;
> -    cap->value->state = value;
> +    cap->value->capability = index;
> +    cap->value->state = state;
> +    cap->next = head;
> +
> +    return cap;


But we don't do the *head = cap?

Pelhaps is better to call it "next" or "list"?

Re: [Qemu-devel] [PATCH v2 10/11] migration: provide migrate_cap_add()
Posted by Peter Xu 8 years, 6 months ago
On Mon, Jul 17, 2017 at 07:14:44PM +0200, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
> > Abstracted from migrate_set_block_enabled() to allocate
> > MigrationCapabilityStatusList properly.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> 
> 
> Nitpick
> 
> > -void migrate_set_block_enabled(bool value, Error **errp)
> > +static MigrationCapabilityStatusList *migrate_cap_add(
> > +    MigrationCapabilityStatusList *head,
> 
> We have a parameter called head
> 
> > +    MigrationCapability index,
> > +    bool state)
> >  {
> >      MigrationCapabilityStatusList *cap;
> >  
> >      cap = g_new0(MigrationCapabilityStatusList, 1);
> >      cap->value = g_new0(MigrationCapabilityStatus, 1);
> > -    cap->value->capability = MIGRATION_CAPABILITY_BLOCK;
> > -    cap->value->state = value;
> > +    cap->value->capability = index;
> > +    cap->value->state = state;
> > +    cap->next = head;
> > +
> > +    return cap;
> 
> 
> But we don't do the *head = cap?
> 
> Pelhaps is better to call it "next" or "list"?

It's the (old) head, but sure. :)

Thanks,

-- 
Peter Xu