Re: [PATCH v7 19/47] vmdk: Drop vmdk_co_flush()

[Max Reitz]

On 14.07.20 16:52, Andrey Shinkevich wrote:
> On 25.06.2020 18:21, Max Reitz wrote:
>> Before HEAD^, we needed this because bdrv_co_flush() by itself would
>> only flush bs->file.  With HEAD^, bdrv_co_flush() will flush all
>> children on which a WRITE or WRITE_UNCHANGED permission has been taken.
>> Thus, vmdk no longer needs to do it itself.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   block/vmdk.c | 16 ----------------
>>   1 file changed, 16 deletions(-)
>>
>> diff --git a/block/vmdk.c b/block/vmdk.c
>> index 62da465126..a23890e6ec 100644
>> --- a/block/vmdk.c
>> +++ b/block/vmdk.c
>> @@ -2802,21 +2802,6 @@ static void vmdk_close(BlockDriverState *bs)
>>       error_free(s->migration_blocker);
>>   }
>>   -static coroutine_fn int vmdk_co_flush(BlockDriverState *bs)
>> -{
>> -    BDRVVmdkState *s = bs->opaque;
>> -    int i, err;
>> -    int ret = 0;
>> -
>> -    for (i = 0; i < s->num_extents; i++) {
>> -        err = bdrv_co_flush(s->extents[i].file->bs);
>> -        if (err < 0) {
>> -            ret = err;
>> -        }
>> -    }
>> -    return ret;
>> -}
>> -
>>   static int64_t vmdk_get_allocated_file_size(BlockDriverState *bs)
>>   {
>>       int i;
>> @@ -3075,7 +3060,6 @@ static BlockDriver bdrv_vmdk = {
>>       .bdrv_close                   = vmdk_close,
>>       .bdrv_co_create_opts          = vmdk_co_create_opts,
>>       .bdrv_co_create               = vmdk_co_create,
>> -    .bdrv_co_flush_to_disk        = vmdk_co_flush,
> 
> 
> After HEAD^ applied, wouldn't we get an endless recursion in
> bdrv_co_flush() if the HEAD (this patch) had not been merged into HEAD^?

Hm, how so?  HEAD^ just flushes all children, just like vmdk_co_flush()
does.  So it seems to me just like double the work.  (Which is
unfortunate but shouldn’t be a problem.)

Max

signature.asc
Description: OpenPGP digital signature