[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v12 1/3] block: introduce compress filter driver
From: |
Max Reitz |
Subject: |
Re: [PATCH v12 1/3] block: introduce compress filter driver |
Date: |
Fri, 20 Dec 2019 15:52:48 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 |
On 02.12.19 13:15, 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 <address@hidden>
> Signed-off-by: Andrey Shinkevich <address@hidden>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
> block/Makefile.objs | 1 +
> block/filter-compress.c | 168
> ++++++++++++++++++++++++++++++++++++++++++++++++
> qapi/block-core.json | 10 +--
> 3 files changed, 175 insertions(+), 4 deletions(-)
> create mode 100644 block/filter-compress.c
[...]
> diff --git a/block/filter-compress.c b/block/filter-compress.c
> new file mode 100644
> index 0000000..4d756ea
> --- /dev/null
> +++ b/block/filter-compress.c
> @@ -0,0 +1,168 @@
[...]
> +static int compress_open(BlockDriverState *bs, QDict *options, int flags,
> + Error **errp)
> +{
> + bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file, false,
> + errp);
> + if (!bs->file) {
> + return -EINVAL;
> + }
> +
> + if (!bs->file->bs->drv || !block_driver_can_compress(bs->file->bs->drv))
> {
> + error_setg(errp,
> + "Compression is not supported for underlying format: %s",
> + bdrv_get_format_name(bs->file->bs));
bdrv_get_format_name() returns NULL if bs->file->bs->drv is NULL. I’m
sure g_strdup_vprintf() handles %s with a NULL string gracefully in
practice, but I can’t find that specified anywhere. So even though I’m
well aware I’m being a bit stupid about a minor edge case, I’m hesitant
to accept this patch as-is.
Obviously the solution can be as simple as bdrv_get_format_name(...) ?:
"(no format)".
Well, actually, I can be a bit less stupid about it and just propose
merging that change in myself. Would that be OK for you?
(The rest looks good to me.)
Max
signature.asc
Description: OpenPGP digital signature
- [PATCH v12 0/3] qcow2: advanced compression options, Andrey Shinkevich, 2019/12/02
- [PATCH v12 3/3] tests/qemu-iotests: add case to write compressed data of multiple clusters, Andrey Shinkevich, 2019/12/02
- [PATCH v12 1/3] block: introduce compress filter driver, Andrey Shinkevich, 2019/12/02
- Re: [PATCH v12 1/3] block: introduce compress filter driver,
Max Reitz <=
- [PATCH v12 2/3] qcow2: Allow writing compressed data of multiple clusters, Andrey Shinkevich, 2019/12/02
- Re: [PATCH v12 0/3] qcow2: advanced compression options, Andrey Shinkevich, 2019/12/18
- Re: [PATCH v12 0/3] qcow2: advanced compression options, Max Reitz, 2019/12/20