From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50966) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tjnk8-0002WU-33 for qemu-devel@nongnu.org; Sat, 15 Dec 2012 04:09:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tjnk3-0001I4-TJ for qemu-devel@nongnu.org; Sat, 15 Dec 2012 04:09:12 -0500 Received: from mx3-phx2.redhat.com ([209.132.183.24]:33545) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tjnk3-0001I0-LY for qemu-devel@nongnu.org; Sat, 15 Dec 2012 04:09:07 -0500 Date: Sat, 15 Dec 2012 04:09:06 -0500 (EST) From: Paolo Bonzini Message-ID: <867170921.24992648.1355562546865.JavaMail.root@redhat.com> In-Reply-To: <50CBA6B3.2040801@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 10/20] mirror: add buf-size argument to drive-mirror List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: kwolf@redhat.com, jcody@redhat.com, qemu-devel@nongnu.org, stefanha@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 > > --- > > 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