Re: [Qemu-devel] [PATCH 3/5] block: Simplify bdrv_reopen_abort()

[Alberto Garcia]

On Tue 14 Aug 2018 11:07:42 AM CEST, Kevin Wolf wrote:
> Am 29.06.2018 um 13:37 hat Alberto Garcia geschrieben:
>> If a bdrv_reopen_multiple() call fails, then the explicit_options
>> QDict has to be deleted for every entry in the reopen queue. This must
>> happen regardless of whether that entry's bdrv_reopen_prepare() call
>> succeeded or not.
>> 
>> This patch simplifies the cleanup code a bit.
>> 
>> Signed-off-by: Alberto Garcia <address@hidden>
>> ---
>>  block.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>> 
>> diff --git a/block.c b/block.c
>> index 5aaed424b9..48e8f4814c 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3060,9 +3060,10 @@ int bdrv_reopen_multiple(AioContext *ctx, 
>> BlockReopenQueue *bs_queue, Error **er
>>  
>>  cleanup:
>>      QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
>> -        if (ret && bs_entry->prepared) {
>> -            bdrv_reopen_abort(&bs_entry->state);
>> -        } else if (ret) {
>> +        if (ret) {
>> +            if (bs_entry->prepared) {
>> +                bdrv_reopen_abort(&bs_entry->state);
>> +            }
>>              qobject_unref(bs_entry->state.explicit_options);
>>          }
>>          qobject_unref(bs_entry->state.options);
>> @@ -3351,8 +3352,6 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
>>          drv->bdrv_reopen_abort(reopen_state);
>>      }
>>  
>> -    qobject_unref(reopen_state->explicit_options);
>> -
>>      bdrv_abort_perm_update(reopen_state->bs);
>>  }
>
> I think this is only correct if we make bdrv_reopen_prepare/commit/abort
> private. At the moment they are public interfaces, so we must have
> thought that some callers might want to integrate them into other
> transactions.

But that's not a problem as long as the new callers unref
state.explicit_options when bdrv_reopen_prepare() fails. They need to do
it with state.options anyway.

Berto