| Message ID | 20201203172311.68232-2-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: Fix block_resize deadlock with iothreads | expand |
03.12.2020 20:23, Kevin Wolf wrote: > The only thing that happens after the 'out:' label is blk_unref(blk). > However, blk = NULL in all of the error cases, so instead of jumping to > 'out:', we can just return directly. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > blockdev.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index fe6fb5dc1d..229d2cce1b 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -2468,17 +2468,17 @@ void coroutine_fn qmp_block_resize(bool has_device, const char *device, > > if (size < 0) { > error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "size", "a >0 size"); > - goto out; > + return; > } > > if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) { > error_setg(errp, QERR_DEVICE_IN_USE, device); > - goto out; > + return; > } > > blk = blk_new_with_bs(bs, BLK_PERM_RESIZE, BLK_PERM_ALL, errp); > if (!blk) { > - goto out; > + return; > } > > bdrv_drained_begin(bs); > @@ -2487,7 +2487,6 @@ void coroutine_fn qmp_block_resize(bool has_device, const char *device, > bdrv_co_leave(bs, old_ctx); > bdrv_drained_end(bs); > > -out: > bdrv_co_lock(bs); > blk_unref(blk); > bdrv_co_unlock(bs); > Initialization of blk to NULL becomes redundant with this patch, so may be dropped too. Anyway: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Am 08.12.2020 um 15:15 hat Vladimir Sementsov-Ogievskiy geschrieben: > 03.12.2020 20:23, Kevin Wolf wrote: > > The only thing that happens after the 'out:' label is blk_unref(blk). > > However, blk = NULL in all of the error cases, so instead of jumping to > > 'out:', we can just return directly. > > > > Cc: qemu-stable@nongnu.org > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > blockdev.c | 7 +++---- > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/blockdev.c b/blockdev.c > > index fe6fb5dc1d..229d2cce1b 100644 > > --- a/blockdev.c > > +++ b/blockdev.c > > @@ -2468,17 +2468,17 @@ void coroutine_fn qmp_block_resize(bool has_device, const char *device, > > if (size < 0) { > > error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "size", "a >0 size"); > > - goto out; > > + return; > > } > > if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) { > > error_setg(errp, QERR_DEVICE_IN_USE, device); > > - goto out; > > + return; > > } > > blk = blk_new_with_bs(bs, BLK_PERM_RESIZE, BLK_PERM_ALL, errp); > > if (!blk) { > > - goto out; > > + return; > > } > > bdrv_drained_begin(bs); > > @@ -2487,7 +2487,6 @@ void coroutine_fn qmp_block_resize(bool has_device, const char *device, > > bdrv_co_leave(bs, old_ctx); > > bdrv_drained_end(bs); > > -out: > > bdrv_co_lock(bs); > > blk_unref(blk); > > bdrv_co_unlock(bs); > > > > Initialization of blk to NULL becomes redundant with this patch, so > may be dropped too. Good catch, I'll change this while applying. Kevin
diff --git a/blockdev.c b/blockdev.c index fe6fb5dc1d..229d2cce1b 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2468,17 +2468,17 @@ void coroutine_fn qmp_block_resize(bool has_device, const char *device, if (size < 0) { error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "size", "a >0 size"); - goto out; + return; } if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) { error_setg(errp, QERR_DEVICE_IN_USE, device); - goto out; + return; } blk = blk_new_with_bs(bs, BLK_PERM_RESIZE, BLK_PERM_ALL, errp); if (!blk) { - goto out; + return; } bdrv_drained_begin(bs); @@ -2487,7 +2487,6 @@ void coroutine_fn qmp_block_resize(bool has_device, const char *device, bdrv_co_leave(bs, old_ctx); bdrv_drained_end(bs); -out: bdrv_co_lock(bs); blk_unref(blk); bdrv_co_unlock(bs);
The only thing that happens after the 'out:' label is blk_unref(blk). However, blk = NULL in all of the error cases, so instead of jumping to 'out:', we can just return directly. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- blockdev.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)