mbox series

[v10,0/9] Apply COR-filter to the block-stream permanently

Message ID 1601383109-110988-1-git-send-email-andrey.shinkevich@virtuozzo.com
Headers show
Series Apply COR-filter to the block-stream permanently | expand

Message

Zhijian Li (Fujitsu)" via Sept. 29, 2020, 12:38 p.m. UTC
Despite the patch "freeze link to base node..." has been removed from the
series in the current version 9, the iotest case test_stream_parallel does
not pass after the COR-filter is inserted into the backing chain. As the
test case may not be initialized, it does not make a sense and was removed
again.
The check with bdrv_is_allocated_above() takes place in the COR-filter and
in the block-stream job both. An optimization of the block-stream job based
on the filter functionality may be made in a separate series.

v10:
  02: The missed new file block/copy-on-read.h added
v9:
  02: Refactored.
  04: Base node name is used instead of the file name.
  05: New implementation based on Max' review.
  06: New.
  07: New. The patch "freeze link to base node..." was deleted.
  08: New.
  09: The filter node options are initialized.

The v8 Message-Id:
<1598633579-221780-1-git-send-email-andrey.shinkevich@virtuozzo.com>

Andrey Shinkevich (9):
  copy-on-read: Support preadv/pwritev_part functions
  copy-on-read: add filter append/drop functions
  qapi: add filter-node-name to block-stream
  copy-on-read: pass base node name to COR driver
  copy-on-read: limit guest COR activity to base in COR driver
  copy-on-read: skip non-guest reads if no copy needed
  stream: skip filters when writing backing file name to QCOW2 header
  block: remove unused backing-file name parameter
  block: apply COR-filter to block-stream jobs

 block/copy-on-read.c           | 165 ++++++++++++++++++++++++++++++++++++++---
 block/copy-on-read.h           |  35 +++++++++
 block/io.c                     |   2 +-
 block/monitor/block-hmp-cmds.c |   6 +-
 block/stream.c                 | 112 +++++++++++++++++-----------
 blockdev.c                     |  21 +-----
 include/block/block_int.h      |   9 ++-
 qapi/block-core.json           |  23 ++----
 tests/qemu-iotests/030         |  51 ++-----------
 tests/qemu-iotests/030.out     |   4 +-
 tests/qemu-iotests/141.out     |   2 +-
 tests/qemu-iotests/245         |  19 +++--
 12 files changed, 302 insertions(+), 147 deletions(-)
 create mode 100644 block/copy-on-read.h

Comments

Vladimir Sementsov-Ogievskiy Oct. 5, 2020, 2:50 p.m. UTC | #1
29.09.2020 15:38, Andrey Shinkevich wrote:
> To limit the guest's COR operations by the base node in the backing

Better to drop "guest's " word. We don't to limit the guest in any

> chain during stream job, pass the base node name to the copy-on-read
> driver. The rest of the functionality will be implemented in the patch
> that follows.
> 

Oops. Seems we want bottom-node, not base, in according with stream job

> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   block/copy-on-read.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/block/copy-on-read.c b/block/copy-on-read.c
> index 3c8231f..e04092f 100644
> --- a/block/copy-on-read.c
> +++ b/block/copy-on-read.c
> @@ -24,19 +24,23 @@
>   #include "block/block_int.h"
>   #include "qemu/module.h"
>   #include "qapi/error.h"
> +#include "qapi/qmp/qerror.h"
>   #include "qapi/qmp/qdict.h"
>   #include "block/copy-on-read.h"
>   
>   
>   typedef struct BDRVStateCOR {
>       bool active;
> +    BlockDriverState *base_bs;
>   } BDRVStateCOR;
>   
>   
>   static int cor_open(BlockDriverState *bs, QDict *options, int flags,
>                       Error **errp)
>   {
> +    BlockDriverState *base_bs = NULL;
>       BDRVStateCOR *state = bs->opaque;
> +    const char *base_node = qdict_get_try_str(options, "base");
>   
>       bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds,
>                                  BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
> @@ -52,7 +56,16 @@ static int cor_open(BlockDriverState *bs, QDict *options, int flags,
>           ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
>               bs->file->bs->supported_zero_flags);
>   
> +    if (base_node) {
> +        qdict_del(options, "base");
> +        base_bs = bdrv_lookup_bs(NULL, base_node, errp);
> +        if (!base_bs) {
> +            error_setg(errp, QERR_BASE_NOT_FOUND, base_node);
> +            return -EINVAL;
> +        }
> +    }
>       state->active = true;
> +    state->base_bs = base_bs;
>   
>       /*
>        * We don't need to call bdrv_child_refresh_perms() now as the permissions
>