Re: [Qemu-devel] [PATCH 5/6] Add blkmirror block driver

[Paolo Bonzini <pbonzini@redhat.com>]

Il 29/02/2012 15:37, Stefan Hajnoczi ha scritto:
>>  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.

I was thinking of adding something like no_user later.

And of course docs/blkmirror.txt is totally obsolete, I'll merge it into
block/blkmirror.c.

>> +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.

No, not yet.  I can add back AIO here.

Paolo