[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: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 3/5] block: Use bdrv_nb_sectors() when sectors, not bytes are wanted |
Date: |
Wed, 28 May 2014 12:03:21 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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.
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.
Kevin