Re: [PATCH v7 1/3] block: introduce compress filter driver

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3003 bytes --]

On 13.11.19 19:43, Andrey Shinkevich wrote:
> Allow writing all the data compressed through the filter driver.
> The written data will be aligned by the cluster size.
> Based on the QEMU current implementation, that data can be written to
> unallocated clusters only. May be used for a backup job.
> 
> Suggested-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>  block/Makefile.objs     |   1 +
>  block/filter-compress.c | 201 ++++++++++++++++++++++++++++++++++++++++++++++++
>  qapi/block-core.json    |  10 ++-
>  3 files changed, 208 insertions(+), 4 deletions(-)
>  create mode 100644 block/filter-compress.c
> 
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index e394fe0..330529b 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -43,6 +43,7 @@ block-obj-y += crypto.o
>  
>  block-obj-y += aio_task.o
>  block-obj-y += backup-top.o
> +block-obj-y += filter-compress.o
>  
>  common-obj-y += stream.o
>  
> diff --git a/block/filter-compress.c b/block/filter-compress.c
> new file mode 100644
> index 0000000..64b1ee5
> --- /dev/null
> +++ b/block/filter-compress.c
> @@ -0,0 +1,201 @@
> +/*
> + * Compress filter block driver
> + *
> + * Copyright (c) 2019 Virtuozzo International GmbH
> + *
> + * Author:
> + *   Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> + *   (based on block/copy-on-read.c by Max Reitz)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 or
> + * (at your option) any later version of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "block/block_int.h"
> +#include "qemu/module.h"
> +
> +
> +static int compress_open(BlockDriverState *bs, QDict *options, int flags,
> +                         Error **errp)
> +{
> +    bs->backing = bdrv_open_child(NULL, options, "file", bs, &child_file, false,
> +                                  errp);

Please don’t attach something that the QAPI schema calls “file” as
bs->backing.

Yes, attaching it as bs->file would break backing chains.  That’s a bug
in the block layer.  I’ve been working on a fix for a long time.

Please don’t introduce more weirdness just because we have a bug in the
block layer.

(Note that I’d strongly oppose calling the child “backing” in the QAPI
schema, as this would go against what all other user-creatable filters do.)

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]