On Mon, Jul 17, 2017 at 07:14:44PM +0200, Juan Quintela wrote:
> Peter Xu <pet...@redhat.com> wrote:
> > Abstracted from migrate_set_block_enabled() to allocate
> > MigrationCapabilityStatusList properly.
> >
> > Signed-off-by: Peter Xu <pet...@redhat.com>
> 
> Reviewed-by: Juan Quintela <quint...@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

Reply via email to