Re: [Qemu-devel] [PATCH block v1 2/2] block: m25p80: Support read only bdrvs.

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1409 bytes --]

On Thu, Jun 19, 2014 at 11:08:42AM +0200, Paolo Bonzini wrote:
> >static void bdrv_sync_complete(void *opaque, int ret)
> >{
> >    /* do nothing. Masters do not directly interact with the backing store,
> >     * only the working copy so no mutexing required.
> >     */
> >}
> >
> >static void flash_sync_page(Flash *s, int page)
> >{
> >    if (s->bdrv) {
> >        int bdrv_sector, nb_sectors;
> >        QEMUIOVector iov;
> >
> >        bdrv_sector = (page * s->pi->page_size) / BDRV_SECTOR_SIZE;
> >        nb_sectors = DIV_ROUND_UP(s->pi->page_size, BDRV_SECTOR_SIZE);
> >        qemu_iovec_init(&iov, 1);
> >        qemu_iovec_add(&iov, s->storage + bdrv_sector * BDRV_SECTOR_SIZE,
> >                                                nb_sectors * BDRV_SECTOR_SIZE);
> >        bdrv_aio_writev(s->bdrv, bdrv_sector, &iov, nb_sectors,
> >                                                bdrv_sync_complete, NULL);
> >    }
> >}
> 
> Using AIO is a good idea, but you could have overlapping writes here if you
> get close calls to flash_sync_page.  It can be bad.
> 
> Serializing can be done in fancy manners, but it can be as easy as adding
> bdrv_drain(s->bdrv) before the bdrv_aio_writev.

Since this issue was present before the patch and not caused by this
series I'm keeping it in the block queue.

It would be nice to address this in a separate series.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]