Re: [Qemu-devel] [PATCH 06/17] bochs: Implement .bdrv_co_preadv() interface

[Kevin Wolf]

Am 27.04.2016 um 16:06 hat Stefan Hajnoczi geschrieben:
> On Wed, Apr 27, 2016 at 11:52:36AM +0200, Kevin Wolf wrote:
> > Signed-off-by: Kevin Wolf <address@hidden>
> > ---
> >  block/bochs.c | 46 +++++++++++++++++++++++++++++-----------------
> >  1 file changed, 29 insertions(+), 17 deletions(-)
> > 
> > diff --git a/block/bochs.c b/block/bochs.c
> > index af8b7ab..d148454 100644
> > --- a/block/bochs.c
> > +++ b/block/bochs.c
> > @@ -104,6 +104,7 @@ static int bochs_open(BlockDriverState *bs, QDict 
> > *options, int flags,
> >      int ret;
> >  
> >      bs->read_only = 1; // no write support yet
> > +    bs->request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O 
> > supported */
> 
> Can this be the default in block.c?  Drivers that have other alignment
> characteristics can set it explicitly but most drivers will want
> BDRV_SECTOR_SIZE so it's nice to make it common code.

I absolutely don't expect BDRV_SECTOR_SIZE to be what most drivers will
want. It's basically a sign that a driver is either a protocol that
imposes restrictions (like O_DIRECT on raw-posix) or it is broken and we
don't care enough about the emulation overhead to fix it (like this one).

The expected value for most block drivers is 1.

> >      while (nb_sectors > 0) {
> >          int64_t block_offset = seek_to_sector(bs, sector_num);
> >          if (block_offset < 0) {
> >              return block_offset;
> 
> s->lock must be unlocked.

I can't believe I messed this up in so many places. Thanks for catching
this, I'll have to go through all patches and check the locking.

Kevin

pgpMDJnqB76e3.pgp
Description: PGP signature