block/export: Fix crash on error after iothread conflict

[PATCH 0/2] block/export: Fix crash on error after iothread conflict

Max Reitz posted 2 patches 3 years ago

Download series mbox

Test checkpatch passed

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210422145335.65814-1-mreitz@redhat.com

Maintainers: Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>

block/export/export.c      |  4 ++++
tests/qemu-iotests/307     | 15 +++++++++++++++
tests/qemu-iotests/307.out |  8 ++++++++
3 files changed, 27 insertions(+)

Expand all Fold all

[PATCH 0/2] block/export: Fix crash on error after iothread conflict

Posted by Max Reitz 3 years ago

Hi,

By passing the @iothread option to block-export-add, the new export can
be moved to the given iothread.  This may conflict with an existing
parent of the node in question.  How this conflict is resolved, depends
on @fixed-iothread: If that option is true, the error is fatal and
block-export-add fails.  If it is false, the error is ignored and the
node stays in its original iothread.

However, in the implementation, the ignored error is still in *errp, and
so if a second error occurs afterwards and tries to put something into
*errp, that will fail an assertion.

To really ignore the error, we have to free it and clear *errp (with an
ERRP_GUARD()).

Patch 1 is the fix, patch 2 a regression test.


Max Reitz (2):
  block/export: Free ignored Error
  iotests/307: Test iothread conflict for exports

 block/export/export.c      |  4 ++++
 tests/qemu-iotests/307     | 15 +++++++++++++++
 tests/qemu-iotests/307.out |  8 ++++++++
 3 files changed, 27 insertions(+)

-- 
2.30.2

Re: [PATCH 0/2] block/export: Fix crash on error after iothread conflict

Posted by Stefan Hajnoczi 3 years ago

On Thu, Apr 22, 2021 at 04:53:33PM +0200, Max Reitz wrote:
> Hi,
> 
> By passing the @iothread option to block-export-add, the new export can
> be moved to the given iothread.  This may conflict with an existing
> parent of the node in question.  How this conflict is resolved, depends
> on @fixed-iothread: If that option is true, the error is fatal and
> block-export-add fails.  If it is false, the error is ignored and the
> node stays in its original iothread.
> 
> However, in the implementation, the ignored error is still in *errp, and
> so if a second error occurs afterwards and tries to put something into
> *errp, that will fail an assertion.
> 
> To really ignore the error, we have to free it and clear *errp (with an
> ERRP_GUARD()).
> 
> Patch 1 is the fix, patch 2 a regression test.
> 
> 
> Max Reitz (2):
>   block/export: Free ignored Error
>   iotests/307: Test iothread conflict for exports
> 
>  block/export/export.c      |  4 ++++
>  tests/qemu-iotests/307     | 15 +++++++++++++++
>  tests/qemu-iotests/307.out |  8 ++++++++
>  3 files changed, 27 insertions(+)
> 
> -- 
> 2.30.2
> 

Thanks for fixing this!

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>