Re: [PATCH v4 07/25] migration: Always report an error in block_save_setup()

["Cédric Le Goater" <clg@redhat.com>]

On 3/8/24 07:59, Peter Xu wrote:
> On Wed, Mar 06, 2024 at 02:34:22PM +0100, Cédric Le Goater wrote:
>> @@ -404,6 +403,10 @@ static int init_blk_migration(QEMUFile *f)
>>           sectors = bdrv_nb_sectors(bs);
>>           if (sectors <= 0) {
> 
> Not directly relevant to this patch, but just to mention that this looks
> suspicious (even if I know nothing about block migration..) - I am not sure
> whether any block drive would return 0 here, if so it looks still like a
> problem if we do the cleanup, ignoring the rest and return a success.

yes and it is not symmetric with block_load() :

                 total_sectors = blk_nb_sectors(blk);
                 if (total_sectors <= 0) {
                     error_report("Error getting length of block device %s",
                                  device_name);
                     return -EINVAL;
                 }


> 
>>               ret = sectors;
>> +            if (ret < 0) {
>> +                error_setg(errp, "Error getting length of block device %s",
>> +                           bdrv_get_device_name(bs));
>> +            }
>>               bdrv_next_cleanup(&it);
>>               goto out;
>>           }
> 

May be Kevin could tell if bdrv_nb_sectors(bs) == 0 should be considered
and error in the save_setup() context ?


Thanks,

C.