[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 26/36] block/backup-top: drop .active
From: |
Kevin Wolf |
Subject: |
Re: [PATCH v2 26/36] block/backup-top: drop .active |
Date: |
Thu, 4 Feb 2021 13:26:51 +0100 |
Am 27.11.2020 um 15:45 hat Vladimir Sementsov-Ogievskiy geschrieben:
> We don't need this workaround anymore: bdrv_append is already smart
> enough and we can use new bdrv_drop_filter().
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/backup-top.c | 38 +-------------------------------------
> tests/qemu-iotests/283.out | 2 +-
> 2 files changed, 2 insertions(+), 38 deletions(-)
>
> diff --git a/block/backup-top.c b/block/backup-top.c
> index 650ed6195c..84eb73aeb7 100644
> --- a/block/backup-top.c
> +++ b/block/backup-top.c
> @@ -37,7 +37,6 @@
> typedef struct BDRVBackupTopState {
> BlockCopyState *bcs;
> BdrvChild *target;
> - bool active;
> int64_t cluster_size;
> } BDRVBackupTopState;
>
> @@ -127,21 +126,6 @@ static void backup_top_child_perm(BlockDriverState *bs,
> BdrvChild *c,
> uint64_t perm, uint64_t shared,
> uint64_t *nperm, uint64_t *nshared)
> {
> - BDRVBackupTopState *s = bs->opaque;
> -
> - if (!s->active) {
> - /*
> - * The filter node may be in process of bdrv_append(), which firstly
> do
> - * bdrv_set_backing_hd() and then bdrv_replace_node(). This means
> that
> - * we can't unshare BLK_PERM_WRITE during bdrv_append() operation.
> So,
> - * let's require nothing during bdrv_append() and refresh permissions
> - * after it (see bdrv_backup_top_append()).
> - */
> - *nperm = 0;
> - *nshared = BLK_PERM_ALL;
> - return;
> - }
> -
> if (!(role & BDRV_CHILD_FILTERED)) {
> /*
> * Target child
> @@ -229,18 +213,6 @@ BlockDriverState
> *bdrv_backup_top_append(BlockDriverState *source,
> }
> appended = true;
>
> - /*
> - * bdrv_append() finished successfully, now we can require permissions
> - * we want.
> - */
> - state->active = true;
> - bdrv_child_refresh_perms(top, top->backing, &local_err);
bdrv_append() uses bdrv_refresh_perms() for the whole node. Is it doing
unnecessary extra work there and should really do the same as backup-top
did here, i.e. bdrv_child_refresh_perms(bs_new->backing)?
(Really a comment for an earlier patch. This patch itself looks fine.)
Kevin
- Re: [PATCH v2 26/36] block/backup-top: drop .active,
Kevin Wolf <=