On Wed, Feb 29, 2012 at 1:37 PM, Paolo Bonzini <pbonz...@redhat.com> wrote:
> From: Marcelo Tosatti <mtosa...@redhat.com>
>
> Mirrored writes are used by live block copy.
>
> The blkmirror driver is for internal use only, because it requires
> bdrv_append to set up a backing_hd for it. It relies on a quirk
> of bdrv_append, which leaves the old image open for writes.
>
> The source is hardcoded as the backing_hd for the destination, so that
> copy-on-write functions properly. Since the source is not yet available
> at the time blkmirror_open is called, the backing_hd is set later.
>
> Signed-off-by: Marcelo Tosatti <mtosa...@redhat.com>
> Signed-off-by: Federico Simoncelli <fsimo...@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> This version of the driver is almost entirely rewritten to
> use bs->backing_hd and bs->file. This is necessary in order
> to share as much code as possible with group snapshots.
>
> Makefile.objs | 2 +-
> block/blkmirror.c | 153
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
> docs/blkmirror.txt | 16 ++++++
> 3 files changed, 170 insertions(+), 1 deletions(-)
> create mode 100644 block/blkmirror.c
> create mode 100644 docs/blkmirror.txt
Mostly happy here, I just recommend tweaking the name of this block
driver and documenting clearly that this is not a general-purpose
mirroring driver, given that it points image B's backing file at image
A's backing file. I see this driver as internal functionality and
it's fairly easy for users to misuse it and be surprised by the
results.
> +static int blkmirror_co_writev(BlockDriverState *bs,
> + int64_t sector_num, int nb_sectors,
> + QEMUIOVector *qiov)
> +{
> + int ret;
> +
> + /* bs->backing_hd is set after initialization. */
> + bs->file->backing_hd = bs->backing_hd;
> +
> + ret = bdrv_co_writev(bs->backing_hd, sector_num, nb_sectors, qiov);
> + if (ret >= 0) {
> + ret = bdrv_co_writev(bs->file, sector_num, nb_sectors, qiov);
> + }
> +
> + return ret;
> +}
Have you done performance tests? It seems suboptimal to use
.bdrv_co_writev() and perform writes sequentially, even with
cache=unsafe.
