Re: [Qemu-devel] [PATCH v2 08/12] mirror: allow customizing the granularity

[Kevin Wolf <kwolf@redhat.com>]

Am 16.01.2013 18:31, schrieb Paolo Bonzini:
> 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.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

> @@ -72,19 +70,19 @@ static int coroutine_fn mirror_iteration(MirrorBlockJob *s,
>       * is very large, we need to do COW ourselves.  The first time a cluster is
>       * copied, copy it entirely.
>       *
> -     * Because both BDRV_SECTORS_PER_DIRTY_CHUNK and the cluster size are
> -     * powers of two, the number of sectors to copy cannot exceed one cluster.
> +     * Because both the granularity and the cluster size are powers of two, the
> +     * number of sectors to copy cannot exceed one cluster.
>       */
>      sector_num = s->sector_num;
> -    nb_sectors = BDRV_SECTORS_PER_DIRTY_CHUNK;
> -    cluster_num = sector_num / BDRV_SECTORS_PER_DIRTY_CHUNK;
> +    nb_sectors_chunk = nb_sectors = s->granularity >> BDRV_SECTOR_BITS;

When you respin anyway: sectors_per_chunk would be a clearer name, I
think. I'd also avoid double assignments, but the coding style document
doesn't seem to forbid it.


> @@ -962,6 +963,7 @@ Arguments:
>    file/device (NewImageMode, optional, default 'absolute-paths')
>  - "speed": maximum speed of the streaming job, in bytes per second
>    (json-int)
> +- "granularity": granularity of the dirty bitmap, in bytes (json-int, optional)
>  - "sync": what parts of the disk image should be copied to the destination;
>    possibilities include "full" for all the disk, "top" for only the sectors
>    allocated in the topmost image, or "none" to only replicate new I/O
> @@ -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.

This doesn't match the code which has an upper limit of 64k for the
granularity, even with larger cluster sizes.

Kevin