Re: [Qemu-block] [PATCH v2 3/7] qcow2: Make perform_cow() call do_perform_cow() twice

[Alberto Garcia]

On Wed 07 Jun 2017 11:43:34 PM CEST, Eric Blake wrote:
>>  block/qcow2-cluster.c | 38 +++++++++++++++++++++++---------------
>>  1 file changed, 23 insertions(+), 15 deletions(-)
>
>>      qemu_co_mutex_unlock(&s->lock);
>> -    ret = do_perform_cow(bs, m->offset, m->alloc_offset, r->offset, 
>> r->nb_bytes);
>> +    ret = do_perform_cow(bs, m->offset, m->alloc_offset,
>> +                         start->offset, start->nb_bytes);
>> +    if (ret < 0) {
>> +        goto fail;
>> +    }
>> +
>> +    ret = do_perform_cow(bs, m->offset, m->alloc_offset,
>> +                         end->offset, end->nb_bytes);
>> +
>> +fail:
>
> Since you reach this label even on success, it feels a bit awkward. I
> probably would have avoided the goto and used fewer lines by
> refactoring the logic as:
>
> ret = do_perform_cow(..., start->...);
> if (ret >= 0) {
>     ret = do_perform_cow(..., end->...);
> }

I actually wrote this code without any gotos the way you mention, and it
works fine in this patch, but as I was making more changes I ended up
with a quite large piece of code that needed to add "if (!ret)" to
almost every if statement, so I didn't quite like the final result.

I'm open to reconsider it, but take a look first at the code after the
last patch and you'll see that it would not be as neat as it appears
now.

Berto