On 28.02.2017 13:54, Kevin Wolf wrote: > The mirror block job is mainly used for two different scenarios: > Mirroring to an otherwise unused, independent target node, or for active > commit where the target node is part of the backing chain of the source. > > Similarly to the commit block job patch, we need to insert a new filter > node to keep the permissions correct during active commit. > > Note that one change this implies is that job->blk points to > mirror_top_bs as its root now, and mirror_top_bs (rather than the actual > source node) contains the bs->job pointer. This requires qemu-img commit > to get the job by name now rather than just taking bs->job. > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > --- > block/mirror.c | 216 > ++++++++++++++++++++++++++++++++++++++------- > qemu-img.c | 6 +- > tests/qemu-iotests/141 | 2 +- > tests/qemu-iotests/141.out | 4 +- > 4 files changed, 190 insertions(+), 38 deletions(-)
BTW, seeing this commit made me remember that the additional functions I
requested for the dummy block driver (flush etc.) were not meant to be
limited to mirror; the commit node would need get_block_status, too, of
course (not necessarily flush, discard, or write_zeroes, because the
commit node doesn't allow writing anyway). Can be fixed during freeze,
though.
> diff --git a/block/mirror.c b/block/mirror.c
> index beaac6f..a2f22e1 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
[...]
> @@ -983,6 +1005,77 @@ static const BlockJobDriver commit_active_job_driver = {
> .drain = mirror_drain,
> };
>
> +static int coroutine_fn bdrv_mirror_top_preadv(BlockDriverState *bs,
> + uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
> +{
> + return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
> +}
> +
> +static int coroutine_fn bdrv_mirror_top_pwritev(BlockDriverState *bs,
> + uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
> +{
> + return bdrv_co_pwritev(bs->backing, offset, bytes, qiov, flags);
> +}
> +
> +static int coroutine_fn bdrv_mirror_top_flush(BlockDriverState *bs)
> +{
> + return bdrv_co_flush(bs->backing->bs);
> +}
> +
> +static int64_t coroutine_fn bdrv_mirror_top_get_block_status(
> + BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
> + BlockDriverState **file)
> +{
> + *pnum = nb_sectors;
> + *file = bs->backing->bs;
> + return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
BDRV_BLOCK_RAW makes bdrv_co_get_block_status() fall through to
bs->file, though.
I'm willing to turn two blind eyes on this, though, and consider that to
be a bug in bdrv_co_get_block_status(); it should fall through to *file.
OTOH, as I said before, the bs->backing without bs->file thing also
breaks bdrv_refresh_filename(). We could fix that, too, by falling
through to bs->backing if there is no bs->file, but that would be a
hack. The other solution is to implement .bdrv_refresh_filename(), of
course.
Since I only have two eyes I can turn blind, I can't really give an R-b, so:
Acked-by: Max Reitz <mre...@redhat.com>
> + (sector_num << BDRV_SECTOR_BITS);
> +}
signature.asc
Description: OpenPGP digital signature
