[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v12 1/3] block: introduce compress filter driver
From: |
Andrey Shinkevich |
Subject: |
Re: [PATCH v12 1/3] block: introduce compress filter driver |
Date: |
Fri, 20 Dec 2019 15:11:39 +0000 |
On 20/12/2019 17:52, Max Reitz wrote:
> 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?
Yes, please.
Thank you, Max.
Andrey
>
> (The rest looks good to me.)
>
> Max
>
--
With the best regards,
Andrey Shinkevich
- [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
- [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