qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 7/7] block: apply COR-filter to block-stream jobs


From: Andrey Shinkevich
Subject: Re: [PATCH 7/7] block: apply COR-filter to block-stream jobs
Date: Mon, 27 Apr 2020 04:08:33 +0000

1. The mirror technology instantiates its own BLK, sets 0/BLK_PERM_ALL for it on exit and then, via subsequent call to bdrv_child_refresh_perms(), calls bdrv_child_try_set_perm(). We don't have the extra BLK for our filter in stream and do call the same function directly.

2. The function remove_filter() can't be used in the stream job on exit because we should remove the filter and change a backing file in the same critical 'drain' section.

3. Due to the specifics above, I suggest that we make insert/remove filter as an interface when there gets another user of it. 

Andrey


From: Vladimir Sementsov-Ogievskiy <address@hidden>
Sent: Tuesday, April 21, 2020 3:58 PM
To: Andrey Shinkevich <address@hidden>; address@hidden <address@hidden>
Cc: address@hidden <address@hidden>; address@hidden <address@hidden>; address@hidden <address@hidden>; address@hidden <address@hidden>; address@hidden <address@hidden>; address@hidden <address@hidden>; address@hidden <address@hidden>; Denis Lunev <address@hidden>
Subject: Re: [PATCH 7/7] block: apply COR-filter to block-stream jobs
 
20.04.2020 21:36, Andrey Shinkevich wrote:
> The patch completes the series with the COR-filter insertion to any
> block-stream operation. It also makes changes to the iotests 030, 141
> and 245.
>
> Signed-off-by: Andrey Shinkevich <address@hidden>
> ---
>   block/stream.c             | 151 +++++++++++++++++++++++++++++++++++++++------
>   tests/qemu-iotests/030     |   6 +-
>   tests/qemu-iotests/141.out |   2 +-
>   tests/qemu-iotests/245     |   5 +-
>   4 files changed, 141 insertions(+), 23 deletions(-)
>
> diff --git a/block/stream.c b/block/stream.c
> index fab7923..af14ba8 100644
> --- a/block/stream.c
> +++ b/block/stream.c


[..]

> +static int stream_exit(Job *job, bool abort)
> +{
> +    StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
> +    BlockJob *bjob = &s->common;
> +    BlockDriverState *target_bs = s->target_bs;
> +    int ret = 0;
> +
> +    if (s->chain_frozen) {
> +        bdrv_unfreeze_chain(s->target_bs, s->bottom_cow_node);
> +        s->chain_frozen = false;
> +    }
> +
> +    /* Retain the BDS until we complete the graph change. */
> +    bdrv_ref(target_bs);
> +    /* Hold a guest back from writing while permissions are being reset. */
> +    bdrv_drained_begin(target_bs);
> +    /* Drop permissions before the graph change. */
> +    bdrv_child_try_set_perm(bdrv_filtered_rw_child(s->cor_filter_bs),
> +                            0, BLK_PERM_ALL, &error_abort);

Hmm. I don't remember what's wrong with it, but something is. Neither mirror nor backup do this now,
instead they use some flag, that permissions are not needed anymore and call bdrv_child_refresh_perms()

Also, strange that you have insert_filter function, but don't use it here.

Also, could we keep add/remove filter api in block/copy-on-read.c, like it is done for backup-top ?

--
Best regards,
Vladimir

reply via email to

[Prev in Thread] Current Thread [Next in Thread]