Re: [Qemu-devel] [PATCH v10 14/14, resend] rdma: add setup time accounting to QMP statistics

[Paolo Bonzini <pbonzini@redhat.com>]

Il 24/06/2013 15:55, Michael R. Hines ha scritto:
>>>
>>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>> Please stop inventing Reviewed-by tags, or I will stop reviewing your
>> patches.
>>
>> Paolo
> 
> Inventing? I don't understand.
> 
> I accumulated all of those tags from everybody - copy and pasted
> directly from everyone when they gave them to me.\
> 
> What exactly is wrong here?

I didn't review this patch in particular, or at least it was changed
very heavily since I last reviewed it.

I certainly haven't reviewed this:

>>> +static void migrate_set_state(MigrationState *s, int old_state, int new_state)
>>>   {
>>> -    if (__sync_val_compare_and_swap(&s->state, MIG_STATE_ACTIVE,
>>> +    if (__sync_val_compare_and_swap(&s->state, old_state,
>>>                                       new_state) == new_state) { 

and I wouldn't have approved it in fact.  In this shape, the patch
should be split in at least 3 or 4 pieces.

In v9 I asked to not apply patch 14 of v9 (I don't remember if you had
already the R-b there); in v10 you rewrote patch 14 and I pointed out
that the Reviewed-by tag was coming out of thin air; now you resent the
series and left the wrong tag.  That's why I started to be a bit grumpy.

By now you must have learnt that I want changes outside migration-rdma.c
to be as a) minimal b) separate c) well-described as possible.

What is _exactly_ the reason why you're changing the state machine?
What is the change exactly, in fact?

Paolo