[Qemu-devel] [PATCH] Fix: remove bogus QDECREF()

[Qemu-devel] [PATCH] Fix: remove bogus QDECREF()

[Benoît Canet]

This extra QDECREF() was causing quorum_close() to crash while doing
bdrv_unref().

As the optional options qdict should belong to the block layer so do not use
QDECREF() on it.

I tested the following inti and qemu exit sequences.

tested regular snapshots on device=virtio0 ok
tested command line quorum ok
tested command line quorum + snapshot transaction ok
tested quorum open by reference ok
tested quorum open by reference + snapshot transaction ok
tested quorum open at once ok
tested quorum open at once + snapshot transaction ok

Benoît Canet (1):
  blockdev: Fix wrong usage of QDECREF causing snapshoted quorum to
    crash on close.

 blockdev.c | 2 --
 1 file changed, 2 deletions(-)

-- 
1.8.3.2

[Qemu-devel] [PATCH] blockdev: Fix wrong usage of QDECREF causing snapshoted quorum to crash on close.

[Benoît Canet]

As bdrv_open() documentation states:
"The reference to the QDict belongs to the block layer
 * after the call (even on failure), so if the caller intends to reuse the
 * dictionary, it needs to use QINCREF() before calling bdrv_open."

the optional options dict will not be reused after bdrv_open() and should
belong to the block layer so remove the extra QDECREF(options).

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 blockdev.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 5cd50f3..7fbce6c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1316,8 +1316,6 @@ static void external_snapshot_prepare(BlkTransactionState *common,
     if (ret != 0) {
         error_propagate(errp, local_err);
     }
-
-    QDECREF(options);
 }
 
 static void external_snapshot_commit(BlkTransactionState *common)
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH] blockdev: Fix wrong usage of QDECREF causing snapshoted quorum to crash on close.

[Kevin Wolf]

Am 13.02.2014 um 17:22 hat Benoît Canet geschrieben:
> As bdrv_open() documentation states:
> "The reference to the QDict belongs to the block layer
>  * after the call (even on failure), so if the caller intends to reuse the
>  * dictionary, it needs to use QINCREF() before calling bdrv_open."
> 
> the optional options dict will not be reused after bdrv_open() and should
> belong to the block layer so remove the extra QDECREF(options).
> 
> Signed-off-by: Benoit Canet <benoit@irqsave.net>

Ouch, yes. Sorry for not catching this in the review.

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [Qemu-devel] [PATCH] blockdev: Fix wrong usage of QDECREF causing snapshoted quorum to crash on close.

[Stefan Hajnoczi]

On Thu, Feb 13, 2014 at 05:22:33PM +0100, Benoît Canet wrote:
> As bdrv_open() documentation states:
> "The reference to the QDict belongs to the block layer
>  * after the call (even on failure), so if the caller intends to reuse the
>  * dictionary, it needs to use QINCREF() before calling bdrv_open."
> 
> the optional options dict will not be reused after bdrv_open() and should
> belong to the block layer so remove the extra QDECREF(options).
> 
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
>  blockdev.c | 2 --
>  1 file changed, 2 deletions(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan