[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/5] block: Use bdrv_nb_sectors() when sectors,
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 3/5] block: Use bdrv_nb_sectors() when sectors, not bytes are wanted |
Date: |
Wed, 28 May 2014 13:05:57 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Kevin Wolf <address@hidden> writes:
> Am 09.05.2014 um 18:27 hat Markus Armbruster geschrieben:
>> Markus Armbruster <address@hidden> writes:
>>
>> > Instead of bdrv_nb_sectors().
>> >
>> > Aside: a few of these callers don't handle errors. I didn't
>> > investigate whether they should.
>> >
>> > Signed-off-by: Markus Armbruster <address@hidden>
>> > ---
>> [...]
>> > diff --git a/block.c b/block.c
>> > index 44e1f57..1b99cb1 100644
>> > --- a/block.c
>> > +++ b/block.c
>> [...]
>> > @@ -3848,21 +3845,21 @@ static int64_t coroutine_fn
>> > bdrv_co_get_block_status(BlockDriverState *bs,
>> > int64_t sector_num,
>> > int nb_sectors, int
>> > *pnum)
>> > {
>> > - int64_t length;
>> > + int64_t total_sectors;
>> > int64_t n;
>> > int64_t ret, ret2;
>> >
>> > - length = bdrv_getlength(bs);
>> > - if (length < 0) {
>> > - return length;
>> > + total_sectors = bdrv_getlength(bs);
>> > + if (total_sectors < 0) {
>> > + return total_sectors;
>> > }
>> >
>> > - if (sector_num >= (length >> BDRV_SECTOR_BITS)) {
>> > + if (sector_num >= total_sectors) {
>> > *pnum = 0;
>> > return 0;
>> > }
>> >
>> > - n = bs->total_sectors - sector_num;
>> > + n = total_sectors - sector_num;
>> > if (n < nb_sectors) {
>> > nb_sectors = n;
>> > }
>> > @@ -3893,8 +3890,8 @@ static int64_t coroutine_fn
>> > bdrv_co_get_block_status(BlockDriverState *bs,
>> > ret |= BDRV_BLOCK_ZERO;
>> > } else if (bs->backing_hd) {
>> > BlockDriverState *bs2 = bs->backing_hd;
>> > - int64_t length2 = bdrv_getlength(bs2);
>> > - if (length2 >= 0 && sector_num >= (length2 >>
>> > BDRV_SECTOR_BITS)) {
>> > + int64_t nb_sectors2 = bdrv_getlength(bs2);
>> > + if (nb_sectors2 >= 0 && sector_num >= nb_sectors2) {
>> > ret |= BDRV_BLOCK_ZERO;
>> > }
>> > }
>> [...]
>>
>> I neglected to actually replace bdrv_getlength() by bdrv_nb_sectors()
>> here, breaking test 030 (I forgot that make check-block doesn't run all
>> the tests). With that fixed, the tests pass. Full respin wanted?
>
> Yes, please. I think you're aware of it, but in order to avoid
> misunderstandings let me mention that there are two places that need a
> fix here, for both total_sectors and nb_sectors2.
Correct.
> Also please consider splitting this patch so that the functions with
> major changes (bdrv_make_zero, bdrv_co_get_block_status) get separated
> at least.
Okay.