Re: [PATCH v2 10/19] block: introduce fleecing block driver

[Markus Armbruster <armbru@redhat.com>]

Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> Introduce a new driver, that works in pair with copy-before-write to
> improve fleecing.
>
> Without fleecing driver, old fleecing scheme looks as follows:
>
> [guest]
>   |
>   |root
>   v
> [copy-before-write] -----> [temp.qcow2] <--- [nbd export]
>   |                 target  |
>   |file                     |backing
>   v                         |
> [active disk] <-------------+
>
> With fleecing driver, new scheme is:
>
> [guest]
>   |
>   |root
>   v
> [copy-before-write] -----> [fleecing] <--- [nbd export]
>   |                 target  |    |
>   |file                     |    |file
>   v                         |    v
> [active disk]<--source------+  [temp.img]
>
> Benefits of new scheme:
>
> 1. Access control: if remote client try to read data that not covered
>    by original dirty bitmap used on copy-before-write open, client gets
>    -EACCES.
>
> 2. Discard support: if remote client do DISCARD, this additionally to
>    discarding data in temp.img informs block-copy process to not copy
>    these clusters. Next read from discarded area will return -EACCES.
>    This is significant thing: when fleecing user reads data that was
>    not yet copied to temp.img, we can avoid copying it on further guest
>    write.
>
> 3. Synchronisation between client reads and block-copy write is more
>    efficient: it doesn't block intersecting block-copy write during
>    client read.
>
> 4. We don't rely on backing feature: active disk should not be backing
>    of temp image, so we avoid some permission-related difficulties and
>    temp image now is not required to support backing, it may be simple
>    raw image.
>
> Note that now nobody calls fleecing_drv_activate(), so new driver is
> actually unusable. It's a work for the following patch: support
> fleecing block driver in copy-before-write filter driver.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  qapi/block-core.json |  17 ++-
>  block/fleecing.h     |  16 +++
>  block/fleecing-drv.c | 260 +++++++++++++++++++++++++++++++++++++++++++
>  MAINTAINERS          |   1 +
>  block/meson.build    |   1 +
>  5 files changed, 294 insertions(+), 1 deletion(-)
>  create mode 100644 block/fleecing-drv.c
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index c42d23752d..8a333136f5 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2826,13 +2826,14 @@
>  # @blkreplay: Since 4.2
>  # @compress: Since 5.0
>  # @copy-before-write: Since 6.2
> +# @fleecing: Since 6.2
>  #
>  # Since: 2.9
>  ##
>  { 'enum': 'BlockdevDriver',
>    'data': [ 'blkdebug', 'blklogwrites', 'blkreplay', 'blkverify', 'bochs',
>              'cloop', 'compress', 'copy-before-write', 'copy-on-read', 'dmg',
> -            'file', 'ftp', 'ftps', 'gluster',
> +            'file', 'fleecing', 'ftp', 'ftps', 'gluster',
>              {'name': 'host_cdrom', 'if': 'defined(HAVE_HOST_BLOCK_DEVICE)' },
>              {'name': 'host_device', 'if': 'defined(HAVE_HOST_BLOCK_DEVICE)' },
>              'http', 'https', 'iscsi',
> @@ -4077,6 +4078,19 @@
>    'base': 'BlockdevOptionsGenericFormat',
>    'data': { 'target': 'BlockdevRef', '*bitmap': 'BlockDirtyBitmap' } }
>  
> +##
> +# @BlockdevOptionsFleecing:
> +#
> +# Driver that works in pair with copy-before-write to make fleecing scheme.

This is really terse.  Do we explain the driver's intended use anywhere?

I'd suggest s/to make fleecing scheme/to make a fleecing scheme/, except
it doesn't make much sense to me either way :)

> +#
> +# @source: source node of fleecing

We usually say "node name of ...".

> +#
> +# Since: 6.2
> +##
> +{ 'struct': 'BlockdevOptionsFleecing',
> +  'base': 'BlockdevOptionsGenericFormat',
> +  'data': { 'source': 'str' } }
> +
>  ##
>  # @BlockdevOptions:
>  #
> @@ -4133,6 +4147,7 @@
>        'copy-on-read':'BlockdevOptionsCor',
>        'dmg':        'BlockdevOptionsGenericFormat',
>        'file':       'BlockdevOptionsFile',
> +      'fleecing':   'BlockdevOptionsFleecing',
>        'ftp':        'BlockdevOptionsCurlFtp',
>        'ftps':       'BlockdevOptionsCurlFtps',
>        'gluster':    'BlockdevOptionsGluster',

[...]