[PATCH 0/2] block: Fix Transaction leaks

[PATCH 0/2] block: Fix Transaction leaks

[Kevin Wolf]

These are two follow-up fixes for Vladimir's "block: update graph
permissions update". The bugs were reported by Coverity.

Kevin Wolf (2):
  block: Fix Transaction leak in bdrv_root_attach_child()
  block: Fix Transaction leak in bdrv_reopen_multiple()

 block.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

-- 
2.30.2

[PATCH 1/2] block: Fix Transaction leak in bdrv_root_attach_child()

[Kevin Wolf]

The error path needs to call tran_finalize(), too.

Fixes: CID 1452773
Fixes: 548a74c0dbc858edd1a7ee3045b5f2fe710bd8b1
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 874c22c43e..5c0ced6238 100644
--- a/block.c
+++ b/block.c
@@ -2918,13 +2918,14 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
                                    child_role, perm, shared_perm, opaque,
                                    &child, tran, errp);
     if (ret < 0) {
-        bdrv_unref(child_bs);
-        return NULL;
+        assert(child == NULL);
+        goto out;
     }
 
     ret = bdrv_refresh_perms(child_bs, errp);
-    tran_finalize(tran, ret);
 
+out:
+    tran_finalize(tran, ret);
     bdrv_unref(child_bs);
     return child;
 }
-- 
2.30.2

[PATCH 2/2] block: Fix Transaction leak in bdrv_reopen_multiple()

[Kevin Wolf]

Like other error paths, this one needs to call tran_finalize() and clean
up the BlockReopenQueue, too.

Fixes: CID 1452772
Fixes: 72373e40fbc7e4218061a8211384db362d3e7348
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 5c0ced6238..69615fabd1 100644
--- a/block.c
+++ b/block.c
@@ -4052,7 +4052,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
         ret = bdrv_flush(bs_entry->state.bs);
         if (ret < 0) {
             error_setg_errno(errp, -ret, "Error flushing drive");
-            goto cleanup;
+            goto abort;
         }
     }
 
-- 
2.30.2

Re: [PATCH 2/2] block: Fix Transaction leak in bdrv_reopen_multiple()

[Vladimir Sementsov-Ogievskiy]

03.05.2021 14:05, Kevin Wolf wrote:
> Like other error paths, this one needs to call tran_finalize() and clean
> up the BlockReopenQueue, too.

We don't need the "abort" loop on that path. And clean-up of BlockReopenQueue is at "cleanup:" label.

So I'd prefer Peter's suggestion (my "[PATCH 2/6] block: bdrv_reopen_multiple(): fix leak of tran object")

> 
> Fixes: CID 1452772
> Fixes: 72373e40fbc7e4218061a8211384db362d3e7348
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 5c0ced6238..69615fabd1 100644
> --- a/block.c
> +++ b/block.c
> @@ -4052,7 +4052,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
>           ret = bdrv_flush(bs_entry->state.bs);
>           if (ret < 0) {
>               error_setg_errno(errp, -ret, "Error flushing drive");
> -            goto cleanup;
> +            goto abort;
>           }
>       }
>   
> 




-- 
Best regards,
Vladimir

Re: [PATCH 1/2] block: Fix Transaction leak in bdrv_root_attach_child()

[Vladimir Sementsov-Ogievskiy]

03.05.2021 14:05, Kevin Wolf wrote:
> The error path needs to call tran_finalize(), too.
> 
> Fixes: CID 1452773
> Fixes: 548a74c0dbc858edd1a7ee3045b5f2fe710bd8b1
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 874c22c43e..5c0ced6238 100644
> --- a/block.c
> +++ b/block.c
> @@ -2918,13 +2918,14 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
>                                      child_role, perm, shared_perm, opaque,
>                                      &child, tran, errp);
>       if (ret < 0) {
> -        bdrv_unref(child_bs);
> -        return NULL;
> +        assert(child == NULL);
> +        goto out;
>       }
>   
>       ret = bdrv_refresh_perms(child_bs, errp);
> -    tran_finalize(tran, ret);
>   
> +out:
> +    tran_finalize(tran, ret);
>       bdrv_unref(child_bs);
>       return child;
>   }
> 

I like my additional comment and assertion in "[PATCH 1/6] block: fix leak of tran in bdrv_root_attach_child", still this is OK too:

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

-- 
Best regards,
Vladimir

Re: [PATCH 1/2] block: Fix Transaction leak in bdrv_root_attach_child()

[Max Reitz]

On 03.05.21 13:05, Kevin Wolf wrote:
> The error path needs to call tran_finalize(), too.
> 
> Fixes: CID 1452773
> Fixes: 548a74c0dbc858edd1a7ee3045b5f2fe710bd8b1
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 874c22c43e..5c0ced6238 100644
> --- a/block.c
> +++ b/block.c
> @@ -2918,13 +2918,14 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
>                                      child_role, perm, shared_perm, opaque,
>                                      &child, tran, errp);
>       if (ret < 0) {
> -        bdrv_unref(child_bs);
> -        return NULL;
> +        assert(child == NULL);
> +        goto out;
>       }
>   
>       ret = bdrv_refresh_perms(child_bs, errp);
> -    tran_finalize(tran, ret);
>   
> +out:
> +    tran_finalize(tran, ret);
>       bdrv_unref(child_bs);
>       return child;

Looks OK, so:

Reviewed-by: Max Reitz <mreitz@redhat.com>

However, the function’s description says that it will return NULL on 
error.  But if bdrv_refresh_perms() fails, it will still return a 
non-NULL child.  Is that right?

Re: [PATCH 1/2] block: Fix Transaction leak in bdrv_root_attach_child()

[Vladimir Sementsov-Ogievskiy]

03.05.2021 14:49, Max Reitz wrote:
> On 03.05.21 13:05, Kevin Wolf wrote:
>> The error path needs to call tran_finalize(), too.
>>
>> Fixes: CID 1452773
>> Fixes: 548a74c0dbc858edd1a7ee3045b5f2fe710bd8b1
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>   block.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 874c22c43e..5c0ced6238 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -2918,13 +2918,14 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
>>                                      child_role, perm, shared_perm, opaque,
>>                                      &child, tran, errp);
>>       if (ret < 0) {
>> -        bdrv_unref(child_bs);
>> -        return NULL;
>> +        assert(child == NULL);
>> +        goto out;
>>       }
>>       ret = bdrv_refresh_perms(child_bs, errp);
>> -    tran_finalize(tran, ret);
>> +out:
>> +    tran_finalize(tran, ret);
>>       bdrv_unref(child_bs);
>>       return child;
> 
> Looks OK, so:
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> 
> However, the function’s description says that it will return NULL on error.  But if bdrv_refresh_perms() fails, it will still return a non-NULL child.  Is that right?
> 

No, it's reset to NULL on transaction abort, so code is correct. It's not obvious, and I've added a comment and assertion in my version of this fix "[PATCH 1/6] block: fix leak of tran in bdrv_root_attach_child"

-- 
Best regards,
Vladimir

Re: [PATCH 1/2] block: Fix Transaction leak in bdrv_root_attach_child()

[Max Reitz]

On 03.05.21 13:51, Vladimir Sementsov-Ogievskiy wrote:
> 03.05.2021 14:49, Max Reitz wrote:
>> On 03.05.21 13:05, Kevin Wolf wrote:
>>> The error path needs to call tran_finalize(), too.
>>>
>>> Fixes: CID 1452773
>>> Fixes: 548a74c0dbc858edd1a7ee3045b5f2fe710bd8b1
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>> ---
>>>   block.c | 7 ++++---
>>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/block.c b/block.c
>>> index 874c22c43e..5c0ced6238 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -2918,13 +2918,14 @@ BdrvChild 
>>> *bdrv_root_attach_child(BlockDriverState *child_bs,
>>>                                      child_role, perm, shared_perm, 
>>> opaque,
>>>                                      &child, tran, errp);
>>>       if (ret < 0) {
>>> -        bdrv_unref(child_bs);
>>> -        return NULL;
>>> +        assert(child == NULL);
>>> +        goto out;
>>>       }
>>>       ret = bdrv_refresh_perms(child_bs, errp);
>>> -    tran_finalize(tran, ret);
>>> +out:
>>> +    tran_finalize(tran, ret);
>>>       bdrv_unref(child_bs);
>>>       return child;
>>
>> Looks OK, so:
>>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>>
>> However, the function’s description says that it will return NULL on 
>> error.  But if bdrv_refresh_perms() fails, it will still return a 
>> non-NULL child.  Is that right?
>>
> 
> No, it's reset to NULL on transaction abort, so code is correct. It's 
> not obvious, and I've added a comment and assertion in my version of 
> this fix "[PATCH 1/6] block: fix leak of tran in bdrv_root_attach_child"

The fact that the transaction keeps the pointer to this local variable 
around is a bit horrifying, but well.

Max

Re: [PATCH 1/2] block: Fix Transaction leak in bdrv_root_attach_child()

[Vladimir Sementsov-Ogievskiy]

03.05.2021 14:53, Max Reitz wrote:
> On 03.05.21 13:51, Vladimir Sementsov-Ogievskiy wrote:
>> 03.05.2021 14:49, Max Reitz wrote:
>>> On 03.05.21 13:05, Kevin Wolf wrote:
>>>> The error path needs to call tran_finalize(), too.
>>>>
>>>> Fixes: CID 1452773
>>>> Fixes: 548a74c0dbc858edd1a7ee3045b5f2fe710bd8b1
>>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>>> ---
>>>>   block.c | 7 ++++---
>>>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/block.c b/block.c
>>>> index 874c22c43e..5c0ced6238 100644
>>>> --- a/block.c
>>>> +++ b/block.c
>>>> @@ -2918,13 +2918,14 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
>>>>                                      child_role, perm, shared_perm, opaque,
>>>>                                      &child, tran, errp);
>>>>       if (ret < 0) {
>>>> -        bdrv_unref(child_bs);
>>>> -        return NULL;
>>>> +        assert(child == NULL);
>>>> +        goto out;
>>>>       }
>>>>       ret = bdrv_refresh_perms(child_bs, errp);
>>>> -    tran_finalize(tran, ret);
>>>> +out:
>>>> +    tran_finalize(tran, ret);
>>>>       bdrv_unref(child_bs);
>>>>       return child;
>>>
>>> Looks OK, so:
>>>
>>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>>>
>>> However, the function’s description says that it will return NULL on error.  But if bdrv_refresh_perms() fails, it will still return a non-NULL child.  Is that right?
>>>
>>
>> No, it's reset to NULL on transaction abort, so code is correct. It's not obvious, and I've added a comment and assertion in my version of this fix "[PATCH 1/6] block: fix leak of tran in bdrv_root_attach_child"
> 
> The fact that the transaction keeps the pointer to this local variable around is a bit horrifying, but well.
> 

Yes this looks overcomplicated here. But it's useful for bdrv_set_backing_noperm where we would have to add a separate entry to @tran to rollback bs->backing change. Probably, it's better to refactor this thing. Or at least document that out-pointer argument goes into transaction, and should be valid up to transaction finalization.



-- 
Best regards,
Vladimir

Re: [PATCH 1/2] block: Fix Transaction leak in bdrv_root_attach_child()

[Vladimir Sementsov-Ogievskiy]

03.05.2021 15:14, Vladimir Sementsov-Ogievskiy wrote:
> 03.05.2021 14:53, Max Reitz wrote:
>> On 03.05.21 13:51, Vladimir Sementsov-Ogievskiy wrote:
>>> 03.05.2021 14:49, Max Reitz wrote:
>>>> On 03.05.21 13:05, Kevin Wolf wrote:
>>>>> The error path needs to call tran_finalize(), too.
>>>>>
>>>>> Fixes: CID 1452773
>>>>> Fixes: 548a74c0dbc858edd1a7ee3045b5f2fe710bd8b1
>>>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>>>> ---
>>>>>   block.c | 7 ++++---
>>>>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/block.c b/block.c
>>>>> index 874c22c43e..5c0ced6238 100644
>>>>> --- a/block.c
>>>>> +++ b/block.c
>>>>> @@ -2918,13 +2918,14 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
>>>>>                                      child_role, perm, shared_perm, opaque,
>>>>>                                      &child, tran, errp);
>>>>>       if (ret < 0) {
>>>>> -        bdrv_unref(child_bs);
>>>>> -        return NULL;
>>>>> +        assert(child == NULL);
>>>>> +        goto out;
>>>>>       }
>>>>>       ret = bdrv_refresh_perms(child_bs, errp);
>>>>> -    tran_finalize(tran, ret);
>>>>> +out:
>>>>> +    tran_finalize(tran, ret);
>>>>>       bdrv_unref(child_bs);
>>>>>       return child;
>>>>
>>>> Looks OK, so:
>>>>
>>>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>>>>
>>>> However, the function’s description says that it will return NULL on error.  But if bdrv_refresh_perms() fails, it will still return a non-NULL child.  Is that right?
>>>>
>>>
>>> No, it's reset to NULL on transaction abort, so code is correct. It's not obvious, and I've added a comment and assertion in my version of this fix "[PATCH 1/6] block: fix leak of tran in bdrv_root_attach_child"
>>
>> The fact that the transaction keeps the pointer to this local variable around is a bit horrifying, but well.
>>
> 
> Yes this looks overcomplicated here. But it's useful for bdrv_set_backing_noperm where we would have to add a separate entry to @tran to rollback bs->backing change. Probably, it's better to refactor this thing. Or at least document that out-pointer argument goes into transaction, and should be valid up to transaction finalization.
> 
> 
> 

documentation done:

   [PATCH 7/6] block: document child argument of bdrv_attach_child_common()

-- 
Best regards,
Vladimir

Re: [PATCH 2/2] block: Fix Transaction leak in bdrv_reopen_multiple()

[Kevin Wolf]

Am 03.05.2021 um 13:40 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 03.05.2021 14:05, Kevin Wolf wrote:
> > Like other error paths, this one needs to call tran_finalize() and clean
> > up the BlockReopenQueue, too.
> 
> We don't need the "abort" loop on that path. And clean-up of
> BlockReopenQueue is at "cleanup:" label.

The cleanup of the BlockReopenQueue itself is there, but not of all
fields in it. Specifically, these lines are needed:

    qobject_unref(bs_entry->state.explicit_options);
    qobject_unref(bs_entry->state.options);

The references are taken in bdrv_reopen_queue_child() and either used in
commit or released on abort. Doing nothing with them leaks them.

> So I'd prefer Peter's suggestion (my "[PATCH 2/6] block:
> bdrv_reopen_multiple(): fix leak of tran object")

I don't like it because I think every call that doesn't end in a commit,
should jump to the abort label to make sure that everything that remains
unused because of this is correctly cleaned up.

Kevin

Re: [PATCH 2/2] block: Fix Transaction leak in bdrv_reopen_multiple()

[Vladimir Sementsov-Ogievskiy]

03.05.2021 15:41, Kevin Wolf wrote:
> Am 03.05.2021 um 13:40 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> 03.05.2021 14:05, Kevin Wolf wrote:
>>> Like other error paths, this one needs to call tran_finalize() and clean
>>> up the BlockReopenQueue, too.
>>
>> We don't need the "abort" loop on that path. And clean-up of
>> BlockReopenQueue is at "cleanup:" label.
> 
> The cleanup of the BlockReopenQueue itself is there, but not of all
> fields in it. Specifically, these lines are needed:
> 
>      qobject_unref(bs_entry->state.explicit_options);
>      qobject_unref(bs_entry->state.options);
> 
> The references are taken in bdrv_reopen_queue_child() and either used in
> commit or released on abort. Doing nothing with them leaks them.

Oops. Somehow I decided they are set in _prepare.

> 
>> So I'd prefer Peter's suggestion (my "[PATCH 2/6] block:
>> bdrv_reopen_multiple(): fix leak of tran object")
> 
> I don't like it because I think every call that doesn't end in a commit,
> should jump to the abort label to make sure that everything that remains
> unused because of this is correctly cleaned up.
> 


agree now..

Still, don't we miss these two qobject_unref() calls on success path?

-- 
Best regards,
Vladimir

Re: [PATCH 2/2] block: Fix Transaction leak in bdrv_reopen_multiple()

[Kevin Wolf]

Am 03.05.2021 um 15:09 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 03.05.2021 15:41, Kevin Wolf wrote:
> > Am 03.05.2021 um 13:40 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > 03.05.2021 14:05, Kevin Wolf wrote:
> > > > Like other error paths, this one needs to call tran_finalize() and clean
> > > > up the BlockReopenQueue, too.
> > > 
> > > We don't need the "abort" loop on that path. And clean-up of
> > > BlockReopenQueue is at "cleanup:" label.
> > 
> > The cleanup of the BlockReopenQueue itself is there, but not of all
> > fields in it. Specifically, these lines are needed:
> > 
> >      qobject_unref(bs_entry->state.explicit_options);
> >      qobject_unref(bs_entry->state.options);
> > 
> > The references are taken in bdrv_reopen_queue_child() and either used in
> > commit or released on abort. Doing nothing with them leaks them.
> 
> Oops. Somehow I decided they are set in _prepare.
> 
> > 
> > > So I'd prefer Peter's suggestion (my "[PATCH 2/6] block:
> > > bdrv_reopen_multiple(): fix leak of tran object")
> > 
> > I don't like it because I think every call that doesn't end in a commit,
> > should jump to the abort label to make sure that everything that remains
> > unused because of this is correctly cleaned up.
> > 
> 
> 
> agree now..
> 
> Still, don't we miss these two qobject_unref() calls on success path?

No, they are put to use in bdrv_reopen_commit():

    qobject_unref(bs->explicit_options);
    qobject_unref(bs->options);

    bs->explicit_options   = reopen_state->explicit_options;
    bs->options            = reopen_state->options;

Kevin

Re: [PATCH 2/2] block: Fix Transaction leak in bdrv_reopen_multiple()

[Vladimir Sementsov-Ogievskiy]

03.05.2021 14:05, Kevin Wolf wrote:
> Like other error paths, this one needs to call tran_finalize() and clean
> up the BlockReopenQueue, too.
> 
> Fixes: CID 1452772
> Fixes: 72373e40fbc7e4218061a8211384db362d3e7348
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 5c0ced6238..69615fabd1 100644
> --- a/block.c
> +++ b/block.c
> @@ -4052,7 +4052,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
>           ret = bdrv_flush(bs_entry->state.bs);
>           if (ret < 0) {
>               error_setg_errno(errp, -ret, "Error flushing drive");
> -            goto cleanup;
> +            goto abort;
>           }
>       }
>   
> 


Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

-- 
Best regards,
Vladimir

Re: [PATCH 2/2] block: Fix Transaction leak in bdrv_reopen_multiple()

[Vladimir Sementsov-Ogievskiy]

03.05.2021 17:33, Kevin Wolf wrote:
> Am 03.05.2021 um 15:09 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> 03.05.2021 15:41, Kevin Wolf wrote:
>>> Am 03.05.2021 um 13:40 hat Vladimir Sementsov-Ogievskiy geschrieben:
>>>> 03.05.2021 14:05, Kevin Wolf wrote:
>>>>> Like other error paths, this one needs to call tran_finalize() and clean
>>>>> up the BlockReopenQueue, too.
>>>>
>>>> We don't need the "abort" loop on that path. And clean-up of
>>>> BlockReopenQueue is at "cleanup:" label.
>>>
>>> The cleanup of the BlockReopenQueue itself is there, but not of all
>>> fields in it. Specifically, these lines are needed:
>>>
>>>       qobject_unref(bs_entry->state.explicit_options);
>>>       qobject_unref(bs_entry->state.options);
>>>
>>> The references are taken in bdrv_reopen_queue_child() and either used in
>>> commit or released on abort. Doing nothing with them leaks them.
>>
>> Oops. Somehow I decided they are set in _prepare.
>>
>>>
>>>> So I'd prefer Peter's suggestion (my "[PATCH 2/6] block:
>>>> bdrv_reopen_multiple(): fix leak of tran object")
>>>
>>> I don't like it because I think every call that doesn't end in a commit,
>>> should jump to the abort label to make sure that everything that remains
>>> unused because of this is correctly cleaned up.
>>>
>>
>>
>> agree now..
>>
>> Still, don't we miss these two qobject_unref() calls on success path?
> 
> No, they are put to use in bdrv_reopen_commit():
> 
>      qobject_unref(bs->explicit_options);
>      qobject_unref(bs->options);
> 
>      bs->explicit_options   = reopen_state->explicit_options;
>      bs->options            = reopen_state->options;
> 
> Kevin
> 

OK then. I should have checked myself :\

-- 
Best regards,
Vladimir

Re: [PATCH 0/2] block: Fix Transaction leaks

[Vladimir Sementsov-Ogievskiy]

03.05.2021 14:05, Kevin Wolf wrote:
> These are two follow-up fixes for Vladimir's "block: update graph
> permissions update". The bugs were reported by Coverity.
> 
> Kevin Wolf (2):
>    block: Fix Transaction leak in bdrv_root_attach_child()
>    block: Fix Transaction leak in bdrv_reopen_multiple()
> 
>   block.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 

I'll rebase my "[PATCH 0/6] block permission updated follow-up" on top of this.

-- 
Best regards,
Vladimir

Re: [PATCH 0/2] block: Fix Transaction leaks

[Kevin Wolf]

Am 04.05.2021 um 08:56 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 03.05.2021 14:05, Kevin Wolf wrote:
> > These are two follow-up fixes for Vladimir's "block: update graph
> > permissions update". The bugs were reported by Coverity.
> > 
> > Kevin Wolf (2):
> >    block: Fix Transaction leak in bdrv_root_attach_child()
> >    block: Fix Transaction leak in bdrv_reopen_multiple()
> > 
> >   block.c | 9 +++++----
> >   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> I'll rebase my "[PATCH 0/6] block permission updated follow-up" on top
> of this.

Ok, thanks. I've applied this to my block branch now.

Kevin