[Qemu-devel] [PATCH 1/2] block: Set backing_hd to NULL after deleting it

[Qemu-devel] [PATCH 1/2] block: Set backing_hd to NULL after deleting it

[Stefan Hajnoczi]

It is safer to set backing_hd to NULL after deleting it so that any use
after deletion is obvious during development.  Happy segfaulting!

This patch should be applied after Kevin Wolf's "vmdk: Convert to
bdrv_open" so that vmdk does not segfault on close.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/block.c b/block.c
index 12cf434..e7e74d9 100644
--- a/block.c
+++ b/block.c
@@ -523,8 +523,10 @@ unlink_and_fail:
 void bdrv_close(BlockDriverState *bs)
 {
     if (bs->drv) {
-        if (bs->backing_hd)
+        if (bs->backing_hd) {
             bdrv_delete(bs->backing_hd);
+            bs->backing_hd = NULL;
+        }
         bs->drv->bdrv_close(bs);
         qemu_free(bs->opaque);
 #ifdef _WIN32
-- 
1.7.0

[Qemu-devel] [PATCH 2/2] qcow2: Avoid shadowing variable in alloc_clusters_noref()

[Stefan Hajnoczi]

The i loop iterator is shadowed by the next free cluster index.  Both
using the variable name 'i' makes the code harder to read.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block/qcow2-refcount.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 47c9978..088d96d 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -554,8 +554,8 @@ static int64_t alloc_clusters_noref(BlockDriverState *bs, int64_t size)
     nb_clusters = size_to_clusters(s, size);
 retry:
     for(i = 0; i < nb_clusters; i++) {
-        int64_t i = s->free_cluster_index++;
-        if (get_refcount(bs, i) != 0)
+        int64_t next_cluster_index = s->free_cluster_index++;
+        if (get_refcount(bs, next_cluster_index) != 0)
             goto retry;
     }
 #ifdef DEBUG_ALLOC2
-- 
1.7.0

Re: [Qemu-devel] [PATCH 1/2] block: Set backing_hd to NULL after deleting it

[Stefan Hajnoczi]

On Sat, Apr 17, 2010 at 10:49 AM, Stefan Hajnoczi
<stefanha@linux.vnet.ibm.com> wrote:
> This patch should be applied after Kevin Wolf's "vmdk: Convert to
> bdrv_open" so that vmdk does not segfault on close.

This is a mistake, vmdk will check backing_hd != NULL so it is safe.

Stefan

[Qemu-devel] Re: [PATCH 1/2] block: Set backing_hd to NULL after deleting it

[Kevin Wolf]

Am 17.04.2010 11:49, schrieb Stefan Hajnoczi:
> It is safer to set backing_hd to NULL after deleting it so that any use
> after deletion is obvious during development.  Happy segfaulting!
> 
> This patch should be applied after Kevin Wolf's "vmdk: Convert to
> bdrv_open" so that vmdk does not segfault on close.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Series looks good, applied both patches to the block branch.

Kevin