On 12/05/2015 13:48, Fam Zheng wrote:
> + if (!bdrv_is_allocated_above(source, NULL, sector_num,
> + nb_sectors, &pnum)) {
> + op->nb_sectors = pnum;
> + if (s->source_may_unmap) {
Can you avoid this check by introducing bdrv_get_block_status_above? Then:
- if BDRV_ZERO, you use bdrv_aio_write_zeroes
- if BDRV_ALLOCATED, you use bdrv_aio_readv
- else you use bdrv_aio_discard
> + /*
> + * Source unallocated sectors have zero data. We can't discard
> + * target even if s->target_may_unmap, because the discard
> + * granularity may be different.
> + */
> + bdrv_aio_write_zeroes(s->target, sector_num, op->nb_sectors,
> + s->target_may_unmap ? BDRV_REQ_MAY_UNMAP :
> 0,
You can set the flag unconditionally. But it's probably better to make
this a drive-mirror argument instead of checking the target's
BlockDriverInfo.
Paolo
> + mirror_write_complete,
> + op);
> + } else {
> + /*
> + * Source has irrelevant data in unmapped sectors, it's safe to
> + * discard target.
> + * */
> + bdrv_aio_discard(s->target, sector_num, op->nb_sectors,
> + mirror_write_complete, op);
> + }
> + } else {
> + bdrv_aio_readv(source, sector_num, &op->qiov, nb_sectors,
> + mirror_read_complete, op);
> + }
> return delay_ns;
> }
>
> @@ -399,6 +428,22 @@ static void coroutine_fn mirror_run(void *opaque)
> length = DIV_ROUND_UP(s->bdev_length, s->granularity);
> s->in_flight_bitmap = bitmap_new(length);
>
> + ret = bdrv_get_info(bs, &bdi);
> + if (ret < 0) {
> + /* Safe side. */
> + s->source_may_unmap = true;
> + } else {
> + s->source_may_unmap = bdi.can_write_zeroes_with_unmap;
> + }
> +
> + ret = bdrv_get_info(s->target, &bdi);
> + if (ret < 0) {
> + /* Safe side. */
> + s->target_may_unmap = false;
> + } else {
> + s->target_may_unmap = bdi.can_write_zeroes_with_unmap;
> + }
> +
