Re: [Qemu-block] [Qemu-devel] [PATCH 5/9] mirror: improve performance of mirroring of empty disk

[Stefan Hajnoczi]

On Wed, Jun 15, 2016 at 01:37:18PM +0300, Denis V. Lunev wrote:
> On 06/15/2016 12:19 PM, Stefan Hajnoczi wrote:
> > On Tue, Jun 14, 2016 at 09:20:47PM -0600, Eric Blake wrote:
> > > On 06/14/2016 09:25 AM, Denis V. Lunev wrote:
> > > > We should not take into account zero blocks for delay calculations.
> > > > They are not read and thus IO throttling is not required. In the
> > > > other case VM migration with 16 Tb QCOW2 disk with 4 Gb of data takes
> > > > days.
> > > > 
> > > > Signed-off-by: Denis V. Lunev <address@hidden>
> > > > Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> > > > CC: Stefan Hajnoczi <address@hidden>
> > > > CC: Fam Zheng <address@hidden>
> > > > CC: Kevin Wolf <address@hidden>
> > > > CC: Max Reitz <address@hidden>
> > > > CC: Jeff Cody <address@hidden>
> > > > CC: Eric Blake <address@hidden>
> > > > ---
> > > >   block/mirror.c | 7 +++++--
> > > >   1 file changed, 5 insertions(+), 2 deletions(-)
> > > Seems reasonable, but I'll let others more familiar with throttling give
> > > the final say.
> > There is a bounce buffer fallback when !drv->bdrv_co_pwrite_zeroes.  In
> > that case we need to account for the bytes transferred.  I don't see
> > where the patch takes this into account.
> Interesting point.
> 
> I think we are charging for IO performed. Thus with the
> absence of the callback we should charge for io_sectors/2
> The write will be full scale, there is no reading.
> 
> Correct?

io_sectors currently only accounts for bytes written, not bytes read.

Therefore, I think we need:

/* Don't charge for efficient zero writes */
if (drv->bdrv_co_pwrite_zeroes) {
    io_sectors = 0;
}

Stefan

signature.asc
Description: PGP signature