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

[Cédric Le Goater]

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.