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 progress. > > 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 compared > 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). > > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- > 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 = { > }; > > void mirror_start(BlockDriverState *bs, BlockDriverState *target, > - int64_t speed, MirrorSyncMode mode, > + int64_t speed, int64_t granularity, MirrorSyncMode mode, > BlockdevOnError on_source_error, > BlockdevOnError on_target_error, > BlockDriverCompletionFunc *cb, > @@ -338,6 +337,20 @@ void mirror_start(BlockDriverState *bs, BlockDriverState > *target, > { > MirrorBlockJob *s; > > + if (granularity == 0) { > + /* Choose the default granularity based on the target file's cluster > + * size, clamped between 4k and 64k. */ > + BlockDriverInfo bdi; > + if (bdrv_get_info(target, &bdi) >= 0 && bdi.cluster_size != 0) { > + granularity = MAX(4096, bdi.cluster_size); > + granularity = MIN(65536, granularity); You clamp granularity to sane bounds when defaulting... > + } else { > + granularity = 65536; > + } > + } > + > + assert((granularity & (granularity - 1)) == 0); ...but don't do any checking other than power-of-two on user input. Can 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_size? > @@ -1217,6 +1218,17 @@ void qmp_drive_mirror(const char *device, const char > *target, > if (!has_mode) { > mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS; > } > + if (!has_granularity) { > + granularity = 0; > + } > + if (granularity != 0 && (granularity < 512 || granularity > 1048576 * > 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 image, 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 clusters > +# 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') > > +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. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature