[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH 17/17] block: Make bdrv_is_allocate

From: Eric Blake
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH 17/17] block: Make bdrv_is_allocated_above() byte-based
Date: Wed, 10 May 2017 17:11:47 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0

On 04/24/2017 08:48 PM, Eric Blake wrote:

>>> -            n = psectors_inter;
>>> +             offset + pnum_inter < (intermediate->total_sectors *
>>> +                                    BDRV_SECTOR_SIZE))) {
>> Naive question: not worth using either bdrv_getlength for bytes, or the
>> bdrv_nb_sectors helpers?
> bdrv_getlength(intermediate) should indeed be the same as
> intermediate->total_sectors * BDRV_SECTOR_SIZE (for now - ultimately it
> would be nice to track a byte length rather than a sector length for
> images). I can make that cleanup for v2.

This one's tricky.  Calling bdrv_nb_sectors()/bdrv_getlength() (the two
are identical other than scale) guarantees that you have a sane answer
for a variably-sized BDS, but can fail with -ENOMEDIUM.  Which, given
the position in the 'if' clause, makes it a difficult rewrite to
properly catch.  On the other hand, since we just barely called
bdrv_is_allocated(intermediate), which in turn called
bdrv_co_get_block_status(), and that calls bdrv_nb_sectors(), we are
assured that intermediate->total_sectors has not changed in the meantime.

So my options are to either add a big comment stating why we are safe,
or to use bdrv_getlength() anyways but with proper error checking.  Best
done as a separate patch from the conversion in scale; so I'll do that
for v2.

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]