Re: [Qemu-devel] [PATCH v3 10/10] block: Avoid bdrv_get_geometry() where errors should be detected

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

> Benoît Canet <address@hidden> writes:
>
>> The Friday 30 May 2014 à 20:13:51 (+0200), Markus Armbruster wrote :
>>> bdrv_get_geometry() hides errors.  Use bdrv_nb_sectors() or
>>> bdrv_getlength() instead where that's obviously inappropriate.
[...]
>>> diff --git a/qemu-img.c b/qemu-img.c
>>> index 2cb56c5..fca58bc 100644
>>> --- a/qemu-img.c
>>> +++ b/qemu-img.c
>>> @@ -959,7 +959,6 @@ static int img_compare(int argc, char **argv)
>>>      int64_t sector_num = 0;
>>>      int64_t nb_sectors;
>>>      int c, pnum;
>>> -    uint64_t bs_sectors;
>>>      uint64_t progress_base;
>>>  
>>>      for (;;) {
>>> @@ -1021,10 +1020,18 @@ static int img_compare(int argc, char **argv)
>>>  
>>>      buf1 = qemu_blockalign(bs1, IO_BUF_SIZE);
>>>      buf2 = qemu_blockalign(bs2, IO_BUF_SIZE);
>>> -    bdrv_get_geometry(bs1, &bs_sectors);
>>> -    total_sectors1 = bs_sectors;
>>> -    bdrv_get_geometry(bs2, &bs_sectors);
>>> -    total_sectors2 = bs_sectors;
>>> +    total_sectors1 = bdrv_nb_sectors(bs1);
>>> +    if (total_sectors1 < 0) {
>>> +        error_report("Can't get size of %s: %s",
>>> +                     filename1, strerror(-total_sectors1));
>>> +        goto out;
>>> +    }
>>> +    total_sectors2 = bdrv_nb_sectors(bs2);
>>> +    if (total_sectors2 < 0) {
>>> +        error_report("Can't get size of %s: %s",
>>> +                     filename2, strerror(-total_sectors2));
>>> +        goto out;
>>> +    }
>> Would setting ret be useful beforie goto out ? 
>
> Yes, to a value > 1.  But which one?  We use 2, 3 and 4, but their
> documented only as ">1 - Error occured".

All right, I'm picking the value we use for read failures: 4.

>>>      total_sectors = MIN(total_sectors1, total_sectors2);
>>>      progress_base = MAX(total_sectors1, total_sectors2);
>>>  
> [...]
>
> Thanks!