[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 32/44] mirror: Use real permissions in mirror
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v3 32/44] mirror: Use real permissions in mirror/active commit block job |
Date: |
Tue, 28 Feb 2017 16:50:47 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 |
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 <address@hidden>
> ---
> 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 <address@hidden>
> + (sector_num << BDRV_SECTOR_BITS);
> +}
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v3 25/44] commit: Use real permissions in commit block job, (continued)
- [Qemu-devel] [PATCH v3 24/44] blockjob: Add permissions to block_job_add_bdrv(), Kevin Wolf, 2017/02/28
- [Qemu-devel] [PATCH v3 27/44] backup: Use real permissions in backup block job, Kevin Wolf, 2017/02/28
- [Qemu-devel] [PATCH v3 28/44] block: Fix pending requests check in bdrv_append(), Kevin Wolf, 2017/02/28
- [Qemu-devel] [PATCH v3 29/44] block: BdrvChildRole.attach/detach() callbacks, Kevin Wolf, 2017/02/28
- [Qemu-devel] [PATCH v3 30/44] block: Allow backing file links in change_parent_backing_link(), Kevin Wolf, 2017/02/28
- [Qemu-devel] [PATCH v3 31/44] blockjob: Factor out block_job_remove_all_bdrv(), Kevin Wolf, 2017/02/28
- [Qemu-devel] [PATCH v3 33/44] stream: Use real permissions in streaming block job, Kevin Wolf, 2017/02/28
- [Qemu-devel] [PATCH v3 32/44] mirror: Use real permissions in mirror/active commit block job, Kevin Wolf, 2017/02/28
- Re: [Qemu-devel] [PATCH v3 32/44] mirror: Use real permissions in mirror/active commit block job,
Max Reitz <=
- [Qemu-devel] [PATCH v3 35/44] commit: Add filter-node-name to block-commit, Kevin Wolf, 2017/02/28
- [Qemu-devel] [PATCH v3 36/44] hmp: Request permissions in qemu-io, Kevin Wolf, 2017/02/28
- [Qemu-devel] [PATCH v3 34/44] mirror: Add filter-node-name to blockdev-mirror, Kevin Wolf, 2017/02/28
- [Qemu-devel] [PATCH v3 37/44] migration/block: Use real permissions, Kevin Wolf, 2017/02/28
- [Qemu-devel] [PATCH v3 38/44] nbd/server: Use real permissions for NBD exports, Kevin Wolf, 2017/02/28
- [Qemu-devel] [PATCH v3 39/44] tests: Remove FIXME comments, Kevin Wolf, 2017/02/28
- [Qemu-devel] [PATCH v3 41/44] block: Assertions for write permissions, Kevin Wolf, 2017/02/28