From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58014) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TjdJs-00066c-0j for qemu-devel@nongnu.org; Fri, 14 Dec 2012 17:01:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TjdJn-0005Mw-CG for qemu-devel@nongnu.org; Fri, 14 Dec 2012 17:01:23 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55082) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TjdJn-0005MS-4O for qemu-devel@nongnu.org; Fri, 14 Dec 2012 17:01:19 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qBEM1DLr006422 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 14 Dec 2012 17:01:13 -0500 Message-ID: <50CBA1A0.7020302@redhat.com> Date: Fri, 14 Dec 2012 15:01:04 -0700 From: Eric Blake MIME-Version: 1.0 References: <1355319999-30627-1-git-send-email-pbonzini@redhat.com> <1355319999-30627-9-git-send-email-pbonzini@redhat.com> In-Reply-To: <1355319999-30627-9-git-send-email-pbonzini@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------enig6B9A8A412090BD7FF884C670" Subject: Re: [Qemu-devel] [PATCH 08/20] mirror: allow customizing the granularity List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: kwolf@redhat.com, jcody@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig6B9A8A412090BD7FF884C670 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 12/12/2012 06:46 AM, Paolo Bonzini wrote: > The desired granularity may be very different depending on the kind of > operation (e.g. continuous replication vs. collapse-to-raw) and whether= > the VM is expected to perform lots of I/O while mirroring is in progres= s. >=20 > Allow the user to customize it, while providing a sane default so that > in general there will be no extra allocated space in the target compare= d > to the source. Might be worth mentioning that this configuration still has to be done prior to starting the job (ie. it's a one-time initialization, not something that is run-time tweakable). >=20 > Signed-off-by: Paolo Bonzini > --- > block/mirror.c | 50 ++++++++++++++++++++++++++++++++----------------= -- > block_int.h | 3 ++- > blockdev.c | 15 ++++++++++++++- > hmp.c | 2 +- > qapi-schema.json | 8 +++++++- > qmp-commands.hx | 8 +++++++- > 6 files changed, 63 insertions(+), 23 deletions(-) > @@ -330,7 +329,7 @@ static BlockJobType mirror_job_type =3D { > }; > =20 > void mirror_start(BlockDriverState *bs, BlockDriverState *target, > - int64_t speed, MirrorSyncMode mode, > + int64_t speed, int64_t granularity, MirrorSyncMode m= ode, > BlockdevOnError on_source_error, > BlockdevOnError on_target_error, > BlockDriverCompletionFunc *cb, > @@ -338,6 +337,20 @@ void mirror_start(BlockDriverState *bs, BlockDrive= rState *target, > { > MirrorBlockJob *s; > =20 > + if (granularity =3D=3D 0) { > + /* Choose the default granularity based on the target file's c= luster > + * size, clamped between 4k and 64k. */ > + BlockDriverInfo bdi; > + if (bdrv_get_info(target, &bdi) >=3D 0 && bdi.cluster_size !=3D= 0) { > + granularity =3D MAX(4096, bdi.cluster_size); > + granularity =3D MIN(65536, granularity); You clamp granularity to sane bounds when defaulting... > + } else { > + granularity =3D 65536; > + } > + } > + > + assert((granularity & (granularity - 1)) =3D=3D 0); =2E..but don't do any checking other than power-of-two on user input. Ca= n the user request a granularity that makes no sense, such as something less than 512 or huge like 1G? Or for that matter, is it even a problem if the user requests a granularity larger than the target bdi.cluster_siz= e? > @@ -1217,6 +1218,17 @@ void qmp_drive_mirror(const char *device, const = char *target, > if (!has_mode) { > mode =3D NEW_IMAGE_MODE_ABSOLUTE_PATHS; > } > + if (!has_granularity) { > + granularity =3D 0; > + } > + if (granularity !=3D 0 && (granularity < 512 || granularity > 1048= 576 * 64)) { That answers part of my question - you clamp between 512 and 64M, which is a much wider range than the defaults you end up choosing. > +++ b/qapi-schema.json > @@ -1636,6 +1636,11 @@ > # (all the disk, only the sectors allocated in the topmost imag= e, or > # only new I/O). > # > +# @granularity: #optional granularity of the dirty bitmap, default is = 64K > +# if the image format doesn't have clusters, 4K if the c= lusters > +# are smaller than that, else the cluster size. Must be= a > +# power of 2 between 512 and 64M. Maybe mention that this attribute was added in 1.4. (Hmm, now I have to decide how to expose this attribute via libvirt.) > @@ -971,6 +973,10 @@ Arguments: > - "on-target-error": the action to take on an error on the target > (BlockdevOnError, default 'report') > =20 > +The default value of the granularity is, if the image format defines > +a cluster size, the cluster size or 4096, whichever is larger. If it > +does not define a cluster size, the default value of the granularity > +is 65536. That doesn't quite cover the fact that you clamp granularity to 64k if the image format has a cluster size larger than 64k. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --------------enig6B9A8A412090BD7FF884C670 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with undefined - http://www.enigmail.net/ iQEcBAEBCAAGBQJQy6GhAAoJEKeha0olJ0NqpggIAKFfdwzRujYFO3s7nQqFTBbE t5aW33//Ra8RrFd5qP7TnsPxZxbNWrMF/wx3FJDobno6g0Zq9ej0B33n+NcmcAlF z9DOP7Sp85PoEEh6OYy7+p7AzckX9B6H+M72XJBBcc44ADKSo9UzKWGF3gAZQxZn sDuHRTXC86ZlFLSRtpmAvrcBqvL3mJs5dY0fKjJdTvBdbD0lhc6i3pnU4kBDe42c 6sF8iTbR1dkV9orVkJ70zc+XvsXBBFXS9QLfYWhI69wIohAKv0OU9y+CxKBMPm2U PmGdoIfNW0xeenep5MdXnc50VAjnPcw27ts5Lu0aDUtfi69s/A/Y63lffu2WcqM= =ho5B -----END PGP SIGNATURE----- --------------enig6B9A8A412090BD7FF884C670--