Re: [Qemu-devel] [PATCH 38/41] block-migration: handle errors with the return codes correctly

[Juan Quintela]

Paolo Bonzini <address@hidden> wrote:
> Il 21/09/2012 10:47, Juan Quintela ha scritto:
>> Signed-off-by: Juan Quintela <address@hidden>
>> ---
>> @@ -635,18 +639,15 @@ static int block_save_complete(QEMUFile *f, void 
>> *opaque)
>>         all async read completed */
>>      assert(block_mig_state.submitted == 0);
>> 
>
> Not clear what the fixes are, I'll take it as a cleanup.  Then:

We are returning now the errors directly.  Until now, we were 

>
>> -    while (blk_mig_save_dirty_block(f, 0) == 0) {
>> +    while ((ret = blk_mig_save_dirty_block(f, 0)) == 0) {
>>          /* Do nothing */
>>      }
>
> use a do...while instead:
>
>     do {
>         ret = blk_mig_save_dirty_block(f, 0);
>     } while (ret == 0);

I wanted to minimize the changes, but I don't really care, 

>
>>      blk_mig_cleanup();
>> -
>> -    /* report completion */
>> -    qemu_put_be64(f, (100 << BDRV_SECTOR_BITS) | BLK_MIG_FLAG_PROGRESS);
>> -
>> -    ret = qemu_file_get_error(f);
>>      if (ret) {
>>          return ret;
>>      }
>
> Is it correct that we now return 1?  We used to return 0.

If you look at the whole change, old code was:


    while (blk_mig_save_dirty_block(f, 0) != 0) {
        /* Do nothing */
    }
    blk_mig_cleanup();

    /* report completion */
    qemu_put_be64(f, (100 << BDRV_SECTOR_BITS) | BLK_MIG_FLAG_PROGRESS);

    ret = qemu_file_get_error(f);
    if (ret) {
        return ret;
    }

    DPRINTF("Block migration completed\n");

    qemu_put_be64(f, BLK_MIG_FLAG_EOS);

    return 0;

i.e. if there is one error, we return it, otherwise, we return 0.

    while ((ret = blk_mig_save_dirty_block(f, 0)) == 0) {
        /* Do nothing */
    }
    blk_mig_cleanup();
    if (ret) {
        return ret;
    }
    /* report completion */
    qemu_put_be64(f, (100 << BDRV_SECTOR_BITS) | BLK_MIG_FLAG_PROGRESS);

    DPRINTF("Block migration completed\n");

    qemu_put_be64(f, BLK_MIG_FLAG_EOS);

    return 0;

Now, we give exactly the same error, just that we return the error
directly from blk_mig_save_dirty_block() instead of storing it on
QEMUFile and getting it through qemu_file_get_error().

Notice that between old code and the _newer_ code, we have reverted the
return value of blk_mig_save_block to make things more consistent.

Later, Juan.