Re: [Qemu-devel] [PATCH 10/20] mirror: add buf-size argument to drive-mirror

[Paolo Bonzini <pbonzini@redhat.com>]

> On 12/12/2012 06:46 AM, Paolo Bonzini wrote:
> > This makes sense when the next commit starts using the extra buffer
> > space
> > to perform many I/O operations asynchronously.
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  block/mirror.c         |  6 +++---
> >  block_int.h            |  5 +++--
> >  blockdev.c             |  9 ++++++++-
> >  hmp.c                  |  2 +-
> >  qapi-schema.json       |  5 ++++-
> >  qmp-commands.hx        |  4 +++-
> >  tests/qemu-iotests/041 | 31 +++++++++++++++++++++++++++++++
> >  7 files changed, 53 insertions(+), 9 deletions(-)
> > 
> 
> > @@ -447,7 +447,7 @@ void mirror_start(BlockDriverState *bs,
> > BlockDriverState *target,
> >      s->target = target;
> >      s->mode = mode;
> >      s->granularity = granularity;
> > -    s->buf_size = granularity;
> > +    s->buf_size = MAX(buf_size, granularity);
> 
> So you silently clamp the buffer size if the user gives something
> larger than granularity.

I silently make it large if it gives something *smaller*.  The job
always has to copy one granularity-sized chunk.

> > +++ b/qapi-schema.json
> > @@ -1641,6 +1641,9 @@
> >  #               are smaller than that, else the cluster size.
> >   Must be a
> >  #               power of 2 between 512 and 64M.
> >  #
> > +# @buf-size: #optional maximum amount of data in flight from
> > source to
> > +#            target.
> 
> Mention that it was added in 1.4.  Also, is it worth mentioning
> reasonable bounds (such as granularity), and whether it must be a
> power of two?

It is arbitrary, though making it too big makes little sense.

> >  - "granularity": granularity of the dirty bitmap (json-int,
> >  optional)
> > +- "buf_size": maximum amount of data in flight from source to
> > target
> > +  (json-int, default 10M)
> 
> Oh, so unlike granularity, this one does NOT have to be a power of 2.
> But why is the default 10M if you clamp it to granularity, which
> defaults to 64k?

I don't clamp it, I make it _at least_ as big as the granularity.

Paolo