Re: [PATCH v2 07/10] migration: Make no compression operations into its own structure

[Juan Quintela]

"Dr. David Alan Gilbert" <address@hidden> wrote:
> * Juan Quintela (address@hidden) wrote:
>> It will be used later.
>> 
>> +static int nocomp_recv_pages(MultiFDRecvParams *p, uint32_t used, Error 
>> **errp)
>> +{
>> +    if (p->flags != MULTIFD_FLAG_NOCOMP) {
>> +        error_setg(errp, "multifd %d: flags received %x flags expected %x",
>> +                   p->id, MULTIFD_FLAG_NOCOMP, p->flags);
>
> That looks the wrong way around to me - shouldn't that be
>    p->flags, MULTIFD_FLAG_NOCOMP
> ?

Good catch.
>>  
>>          qemu_mutex_init(&p->mutex);
>>          qemu_sem_init(&p->sem, 0);
>> @@ -1240,8 +1405,12 @@ int multifd_save_setup(Error **errp)
>>          p->packet->version = cpu_to_be32(MULTIFD_VERSION);
>>          p->name = g_strdup_printf("multifdsend_%d", i);
>>          socket_send_channel_create(multifd_new_send_channel_async, p);
>> +        res = multifd_send_state->ops->send_setup(p, errp);
>> +        if (ret == 0) {
>> +            ret = res;
>
> How do you handle the errp's here - I think that if is so that you
> get the 'ret' from the first thread that fails; but I don't think you're
> allowed to call that twice if the first one set it's errp.

You are right.  I was doing the res/ret variable right, and failed with
the other.  Changed the code to two loops:
- Everything that can't fail, done in the 1st loop.
- Everything else on the second loop.  the time that we have one error,
  we just stop the loop.
>> @@ -1448,6 +1622,10 @@ int multifd_load_setup(Error **errp)
>>                        + sizeof(ram_addr_t) * page_count;
>>          p->packet = g_malloc0(p->packet_len);
>>          p->name = g_strdup_printf("multifdrecv_%d", i);
>> +        ret = multifd_recv_state->ops->recv_setup(p, errp);
>> +        if (ret != 0) {
>> +            return ret;
>> +        }
>
> same question as the save case above

Same solution.

Thanks, Juan.