[v9] qcow2: cluster space preallocation

[Qemu-devel] [PATCH v9 1/9] mirror: inherit supported write/zero flags

Posted by Anton Nefedov 7 years, 5 months ago

Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
 block/mirror.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/block/mirror.c b/block/mirror.c
index 820f512..a22ddef 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1098,6 +1098,15 @@ static BlockDriver bdrv_mirror_top = {
     .bdrv_child_perm            = bdrv_mirror_top_child_perm,
 };
 
+static void mirror_top_set_supported_flags(BlockDriverState *bs)
+{
+    bs->supported_write_flags = BDRV_REQ_FUA &
+        bs->backing->bs->supported_write_flags;
+    bs->supported_zero_flags =
+        (BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) &
+        bs->backing->bs->supported_zero_flags;
+}
+
 static void mirror_start_job(const char *job_id, BlockDriverState *bs,
                              int creation_flags, BlockDriverState *target,
                              const char *replaces, int64_t speed,
@@ -1163,6 +1172,8 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs,
         return;
     }
 
+    mirror_top_set_supported_flags(mirror_top_bs);
+
     /* Make sure that the source is not resized while the job is running */
     s = block_job_create(job_id, driver, NULL, mirror_top_bs,
                          BLK_PERM_CONSISTENT_READ,
-- 
2.7.4

Re: [Qemu-devel] [PATCH v9 1/9] mirror: inherit supported write/zero flags

Posted by Eric Blake 7 years, 5 months ago

On 05/08/2018 09:58 AM, Anton Nefedov wrote:
> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> ---
>   block/mirror.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

That said,

> 
> diff --git a/block/mirror.c b/block/mirror.c
> index 820f512..a22ddef 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -1098,6 +1098,15 @@ static BlockDriver bdrv_mirror_top = {
>       .bdrv_child_perm            = bdrv_mirror_top_child_perm,
>   };
>   
> +static void mirror_top_set_supported_flags(BlockDriverState *bs)
> +{
> +    bs->supported_write_flags = BDRV_REQ_FUA &
> +        bs->backing->bs->supported_write_flags;
> +    bs->supported_zero_flags =
> +        (BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) &
> +        bs->backing->bs->supported_zero_flags;
> +}
> +

This is a pretty short static function...

>   static void mirror_start_job(const char *job_id, BlockDriverState *bs,
>                                int creation_flags, BlockDriverState *target,
>                                const char *replaces, int64_t speed,
> @@ -1163,6 +1172,8 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs,
>           return;
>       }
>   
> +    mirror_top_set_supported_flags(mirror_top_bs);

...with exactly one caller.  Wouldn't it be easier to just inline it?

> +
>       /* Make sure that the source is not resized while the job is running */
>       s = block_job_create(job_id, driver, NULL, mirror_top_bs,
>                            BLK_PERM_CONSISTENT_READ,
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v9 1/9] mirror: inherit supported write/zero flags

Posted by Anton Nefedov 7 years, 5 months ago


On 15/5/2018 5:35 PM, Eric Blake wrote:
> On 05/08/2018 09:58 AM, Anton Nefedov wrote:
>> diff --git a/block/mirror.c b/block/mirror.c
>> index 820f512..a22ddef 100644
>> --- a/block/mirror.c
>> +++ b/block/mirror.c
>> @@ -1098,6 +1098,15 @@ static BlockDriver bdrv_mirror_top = {
>>       .bdrv_child_perm            = bdrv_mirror_top_child_perm,
>>   };
>> +static void mirror_top_set_supported_flags(BlockDriverState *bs)
>> +{
>> +    bs->supported_write_flags = BDRV_REQ_FUA &
>> +        bs->backing->bs->supported_write_flags;
>> +    bs->supported_zero_flags =
>> +        (BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) &
>> +        bs->backing->bs->supported_zero_flags;
>> +}
>> +
> 
> This is a pretty short static function...
> 
>>   static void mirror_start_job(const char *job_id, BlockDriverState *bs,
>>                                int creation_flags, BlockDriverState 
>> *target,
>>                                const char *replaces, int64_t speed,
>> @@ -1163,6 +1172,8 @@ static void mirror_start_job(const char *job_id, 
>> BlockDriverState *bs,
>>           return;
>>       }
>> +    mirror_top_set_supported_flags(mirror_top_bs);
> 
> ...with exactly one caller.  Wouldn't it be easier to just inline it?
> 

idk, I felt mirror_start_job() was quite massive already, even
considering there are just a few new lines.