qemu-devel
[Top][All Lists]
Advanced

[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



reply via email to

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