Re: [Qemu-devel] [PATCH v2 12/17] block: Convert bdrv_write() to BdrvChild

[Kevin Wolf]

Am 29.06.2016 um 17:22 hat Max Reitz geschrieben:
> On 29.06.2016 14:11, Kevin Wolf wrote:
> > Signed-off-by: Kevin Wolf <address@hidden>
> > Acked-by: Stefan Hajnoczi <address@hidden>
> > ---
> > 
> > This patch contains non-trivial fixes, so I think it's worth sending out a 
> > v2
> > for it even though I already applied the series. I added a coroutine entry
> > wrapper qcow(2)_write that can be used from .bdrv_write_compressed. These
> > wrappers will soon disappear again when .bdrv_write_compressed is changed 
> > into
> > .bdrv_co_pwritev_compressed (Pavel Butsykin's backup compression series).
> > 
> >  block/io.c             |  5 +++--
> >  block/qcow.c           | 45 ++++++++++++++++++++++++++++++++++++++++++++-
> >  block/qcow2-cluster.c  |  2 +-
> >  block/qcow2-refcount.c |  2 +-
> >  block/qcow2.c          | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
> >  block/vdi.c            |  4 ++--
> >  block/vvfat.c          |  5 ++---
> >  include/block/block.h  |  2 +-
> >  8 files changed, 100 insertions(+), 12 deletions(-)
> 
> Reviewed-by: Max Reitz <address@hidden>
> 
> [...]
> 
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index 0178931..cd9c27b 100644
> > --- a/block/qcow2.c
> > +++ b/block/qcow2.c
> > @@ -2533,6 +2533,51 @@ static int qcow2_truncate(BlockDriverState *bs, 
> > int64_t offset)
> >      return 0;
> >  }
> >  
> > +typedef struct Qcow2WriteCo {
> > +    BlockDriverState *bs;
> > +    int64_t sector_num;
> > +    const uint8_t *buf;
> > +    int nb_sectors;
> > +    int ret;
> > +} Qcow2WriteCo;
> > +
> > +static void qcow2_write_co_entry(void *opaque)
> > +{
> > +    Qcow2WriteCo *co = opaque;
> > +    QEMUIOVector qiov;
> > +    uint64_t offset = co->sector_num * BDRV_SECTOR_SIZE;
> > +    uint64_t bytes = co->nb_sectors * BDRV_SECTOR_SIZE;
> 
> It doesn't make much sense to make this a uint64_t, and I'm afraid
> Coverity will complain about it... It's not wrong, though, but an int
> would have been more "honest".

Hm, just copied from vmdk... Anyway, you right that we don't really need
uint64_t here because of BDRV_REQUEST_MAX_SECTORS, but BDRV_SECTOR_SIZE
is unsigned long long, so at least this is a proper 64 bit calculation
and Coverity should stay silent.

Kevin

pgpHINQnXOe5S.pgp
Description: PGP signature