[Qemu-devel] [PATCH v5 0/8] Implement reference count for BlockDriverState [resend]

[Qemu-devel] [PATCH v5 0/8] Implement reference count for BlockDriverState [resend]

[Fam Zheng]

[resend to the correct list]

BlockDriverState lifecycle management is needed by future features such as
image fleecing and blockdev-add. This series adds reference count to
BlockDriverState.

The first two patches clean up two odd BlockDriverState use cases, so all code
uses bdrv_new() to create BlockDriverState instance.

Then implemented bdrv_ref() and bdrv_unref() to operate on refcnt: Initially,
refcnt is 1, which means bdrv_unref is effectively a bdrv_delete() here. So
patch 04 has a search and replace to convert bdrv_delete to bdrv_unref, before
bdrv_ref is used anywhere. 05~08 patches calls bdrv_ref for device attach,
block-migration and nbd.

The rule is: Either bdrv_ref() or bdrv_new() must have a matching
bdrv_unref() call, and the last matching bdrv_unref deletes the bs.

v4:
    08: Added, let block job use BDS reference.
    02: Fix leak of bs.opaque

v3:
    03: Removed unnecessary bdrv_close() call.

v2:
    05: Removed: "block: use BlockDriverState refcnt for device attach/detach"
    07: Fix xen_disk blk_disconnect() as it depended on device attach refcnt.


Fam Zheng (8):
  vvfat: use bdrv_new() to allocate BlockDriverState
  iscsi: use bdrv_new() instead of stack structure
  block: implement reference count for BlockDriverState
  block: make bdrv_delete() static
  migration: omit drive ref as we have bdrv_ref now
  xen_disk: simplify blk_disconnect with refcnt
  nbd: use BlockDriverState refcnt
  block: use BDS ref for block jobs

 block-migration.c         |  4 +--
 block.c                   | 44 +++++++++++++++++++++++--------
 block/backup.c            |  2 +-
 block/blkverify.c         |  4 +--
 block/cow.c               |  2 +-
 block/iscsi.c             | 16 ++++++-----
 block/mirror.c            |  2 +-
 block/qcow.c              |  2 +-
 block/qcow2.c             |  2 +-
 block/qed.c               |  2 +-
 block/sheepdog.c          |  6 ++---
 block/snapshot.c          |  2 +-
 block/stream.c            |  2 +-
 block/vmdk.c              | 10 +++----
 block/vvfat.c             |  6 ++---
 blockdev-nbd.c            | 10 +------
 blockdev.c                | 67 +++++------------------------------------------
 blockjob.c                |  3 +++
 hw/block/xen_disk.c       | 13 +++++----
 include/block/block.h     |  3 ++-
 include/block/block_int.h |  1 +
 nbd.c                     |  5 ++++
 qemu-img.c                | 26 +++++++++---------
 qemu-io.c                 |  6 ++---
 24 files changed, 106 insertions(+), 134 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v5 1/8] vvfat: use bdrv_new() to allocate BlockDriverState

[Fam Zheng]

we need bdrv_new() to properly initialize BDS, don't allocate memory
manually.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vvfat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index cd3b8ed..a827d91 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2943,7 +2943,7 @@ static int enable_write_target(BDRVVVFATState *s)
     unlink(s->qcow_filename);
 #endif
 
-    s->bs->backing_hd = calloc(sizeof(BlockDriverState), 1);
+    s->bs->backing_hd = bdrv_new("");
     s->bs->backing_hd->drv = &vvfat_write_target;
     s->bs->backing_hd->opaque = g_malloc(sizeof(void*));
     *(void**)s->bs->backing_hd->opaque = s;
-- 
1.8.3.1

[Qemu-devel] [PATCH v5 2/8] iscsi: use bdrv_new() instead of stack structure

[Fam Zheng]

BlockDriverState structure needs bdrv_new() to initialize refcnt, don't
allocate a local structure variable and memset to 0, becasue with coming
refcnt implementation, bdrv_unref will crash if bs->refcnt not
initialized to 1.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/iscsi.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index e7c1c2b..123f058 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1249,11 +1249,11 @@ static int iscsi_create(const char *filename, QEMUOptionParameter *options)
 {
     int ret = 0;
     int64_t total_size = 0;
-    BlockDriverState bs;
+    BlockDriverState *bs;
     IscsiLun *iscsilun = NULL;
     QDict *bs_options;
 
-    memset(&bs, 0, sizeof(BlockDriverState));
+    bs = bdrv_new("");
 
     /* Read out options */
     while (options && options->name) {
@@ -1263,12 +1263,12 @@ static int iscsi_create(const char *filename, QEMUOptionParameter *options)
         options++;
     }
 
-    bs.opaque = g_malloc0(sizeof(struct IscsiLun));
-    iscsilun = bs.opaque;
+    bs->opaque = g_malloc0(sizeof(struct IscsiLun));
+    iscsilun = bs->opaque;
 
     bs_options = qdict_new();
     qdict_put(bs_options, "filename", qstring_from_str(filename));
-    ret = iscsi_open(&bs, bs_options, 0);
+    ret = iscsi_open(bs, bs_options, 0);
     QDECREF(bs_options);
 
     if (ret != 0) {
@@ -1282,7 +1282,7 @@ static int iscsi_create(const char *filename, QEMUOptionParameter *options)
         ret = -ENODEV;
         goto out;
     }
-    if (bs.total_sectors < total_size) {
+    if (bs->total_sectors < total_size) {
         ret = -ENOSPC;
         goto out;
     }
@@ -1292,7 +1292,9 @@ out:
     if (iscsilun->iscsi != NULL) {
         iscsi_destroy_context(iscsilun->iscsi);
     }
-    g_free(bs.opaque);
+    g_free(bs->opaque);
+    bs->opaque = NULL;
+    bdrv_delete(bs);
     return ret;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v5 3/8] block: implement reference count for BlockDriverState

[Fam Zheng]

Introduce bdrv_ref/bdrv_unref to manage the lifecycle of
BlockDriverState. They are unused for now but will used to replace
bdrv_delete() later.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c                   | 21 +++++++++++++++++++++
 include/block/block.h     |  2 ++
 include/block/block_int.h |  1 +
 3 files changed, 24 insertions(+)

diff --git a/block.c b/block.c
index 01b66d8..fea6904 100644
--- a/block.c
+++ b/block.c
@@ -306,6 +306,7 @@ BlockDriverState *bdrv_new(const char *device_name)
     bdrv_iostatus_disable(bs);
     notifier_list_init(&bs->close_notifiers);
     notifier_with_return_list_init(&bs->before_write_notifiers);
+    bs->refcnt = 1;
 
     return bs;
 }
@@ -1517,6 +1518,9 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
     /* dirty bitmap */
     bs_dest->dirty_bitmap       = bs_src->dirty_bitmap;
 
+    /* reference count */
+    bs_dest->refcnt             = bs_src->refcnt;
+
     /* job */
     bs_dest->in_use             = bs_src->in_use;
     bs_dest->job                = bs_src->job;
@@ -4391,6 +4395,23 @@ int64_t bdrv_get_dirty_count(BlockDriverState *bs)
     }
 }
 
+/* Get a reference to bs */
+void bdrv_ref(BlockDriverState *bs)
+{
+    bs->refcnt++;
+}
+
+/* Release a previously grabbed reference to bs.
+ * If after releasing, reference count is zero, the BlockDriverState is
+ * deleted. */
+void bdrv_unref(BlockDriverState *bs)
+{
+    assert(bs->refcnt > 0);
+    if (--bs->refcnt == 0) {
+        bdrv_delete(bs);
+    }
+}
+
 void bdrv_set_in_use(BlockDriverState *bs, int in_use)
 {
     assert(bs->in_use != in_use);
diff --git a/include/block/block.h b/include/block/block.h
index 742fce5..b33ef62 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -356,6 +356,8 @@ int64_t bdrv_get_dirty_count(BlockDriverState *bs);
 void bdrv_enable_copy_on_read(BlockDriverState *bs);
 void bdrv_disable_copy_on_read(BlockDriverState *bs);
 
+void bdrv_ref(BlockDriverState *bs);
+void bdrv_unref(BlockDriverState *bs);
 void bdrv_set_in_use(BlockDriverState *bs, int in_use);
 int bdrv_in_use(BlockDriverState *bs);
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index e45f2a0..1f85cfb 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -294,6 +294,7 @@ struct BlockDriverState {
     BlockDeviceIoStatus iostatus;
     char device_name[32];
     HBitmap *dirty_bitmap;
+    int refcnt;
     int in_use; /* users other than guest access, eg. block migration */
     QTAILQ_ENTRY(BlockDriverState) list;
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v5 4/8] block: make bdrv_delete() static

[Fam Zheng]

Manage BlockDriverState lifecycle with refcnt, so bdrv_delete() is no
longer public and should be called by bdrv_unref() if refcnt is
decreased to 0.

This is an identical change because effectively, there's no multiple
reference of BDS now: no caller of bdrv_ref() yet, only bdrv_new() sets
bs->refcnt to 1, so all bdrv_unref() now actually delete the BDS.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c               | 23 ++++++++++++-----------
 block/backup.c        |  2 +-
 block/blkverify.c     |  4 ++--
 block/cow.c           |  2 +-
 block/iscsi.c         |  2 +-
 block/mirror.c        |  2 +-
 block/qcow.c          |  2 +-
 block/qcow2.c         |  2 +-
 block/qed.c           |  2 +-
 block/sheepdog.c      |  6 +++---
 block/snapshot.c      |  2 +-
 block/stream.c        |  2 +-
 block/vmdk.c          | 10 +++++-----
 block/vvfat.c         |  4 ++--
 blockdev.c            | 14 +++++++-------
 hw/block/xen_disk.c   |  4 ++--
 include/block/block.h |  1 -
 qemu-img.c            | 26 +++++++++++++-------------
 qemu-io.c             |  6 +++---
 19 files changed, 58 insertions(+), 58 deletions(-)

diff --git a/block.c b/block.c
index fea6904..73e9abd 100644
--- a/block.c
+++ b/block.c
@@ -877,7 +877,7 @@ fail:
     if (!bs->drv) {
         QDECREF(bs->options);
     }
-    bdrv_delete(bs);
+    bdrv_unref(bs);
     return ret;
 }
 
@@ -928,7 +928,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
                     *backing_filename ? backing_filename : NULL, options,
                     back_flags, back_drv);
     if (ret < 0) {
-        bdrv_delete(bs->backing_hd);
+        bdrv_unref(bs->backing_hd);
         bs->backing_hd = NULL;
         bs->open_flags |= BDRV_O_NO_BACKING;
         return ret;
@@ -1003,12 +1003,12 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
         bs1 = bdrv_new("");
         ret = bdrv_open(bs1, filename, NULL, 0, drv);
         if (ret < 0) {
-            bdrv_delete(bs1);
+            bdrv_unref(bs1);
             goto fail;
         }
         total_size = bdrv_getlength(bs1) & BDRV_SECTOR_MASK;
 
-        bdrv_delete(bs1);
+        bdrv_unref(bs1);
 
         ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
         if (ret < 0) {
@@ -1082,7 +1082,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
     }
 
     if (bs->file != file) {
-        bdrv_delete(file);
+        bdrv_unref(file);
         file = NULL;
     }
 
@@ -1122,7 +1122,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
 
 unlink_and_fail:
     if (file != NULL) {
-        bdrv_delete(file);
+        bdrv_unref(file);
     }
     if (bs->is_temporary) {
         unlink(filename);
@@ -1383,7 +1383,7 @@ void bdrv_close(BlockDriverState *bs)
 
     if (bs->drv) {
         if (bs->backing_hd) {
-            bdrv_delete(bs->backing_hd);
+            bdrv_unref(bs->backing_hd);
             bs->backing_hd = NULL;
         }
         bs->drv->bdrv_close(bs);
@@ -1407,7 +1407,7 @@ void bdrv_close(BlockDriverState *bs)
         bs->options = NULL;
 
         if (bs->file != NULL) {
-            bdrv_delete(bs->file);
+            bdrv_unref(bs->file);
             bs->file = NULL;
         }
     }
@@ -1604,11 +1604,12 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)
             bs_new->drv ? bs_new->drv->format_name : "");
 }
 
-void bdrv_delete(BlockDriverState *bs)
+static void bdrv_delete(BlockDriverState *bs)
 {
     assert(!bs->dev);
     assert(!bs->job);
     assert(!bs->in_use);
+    assert(!bs->refcnt);
 
     /* remove from list, if necessary */
     bdrv_make_anon(bs);
@@ -2124,7 +2125,7 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top,
     QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_delete, entry, next) {
         /* so that bdrv_close() does not recursively close the chain */
         intermediate_state->bs->backing_hd = NULL;
-        bdrv_delete(intermediate_state->bs);
+        bdrv_unref(intermediate_state->bs);
     }
     ret = 0;
 
@@ -4625,7 +4626,7 @@ out:
     free_option_parameters(param);
 
     if (bs) {
-        bdrv_delete(bs);
+        bdrv_unref(bs);
     }
 }
 
diff --git a/block/backup.c b/block/backup.c
index 6ae8a05..586d941 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -338,7 +338,7 @@ static void coroutine_fn backup_run(void *opaque)
     hbitmap_free(job->bitmap);
 
     bdrv_iostatus_disable(target);
-    bdrv_delete(target);
+    bdrv_unref(target);
 
     block_job_completed(&job->common, ret);
 }
diff --git a/block/blkverify.c b/block/blkverify.c
index 1d58cc3..c4e961e 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -155,7 +155,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags)
     s->test_file = bdrv_new("");
     ret = bdrv_open(s->test_file, filename, NULL, flags, NULL);
     if (ret < 0) {
-        bdrv_delete(s->test_file);
+        bdrv_unref(s->test_file);
         s->test_file = NULL;
         goto fail;
     }
@@ -169,7 +169,7 @@ static void blkverify_close(BlockDriverState *bs)
 {
     BDRVBlkverifyState *s = bs->opaque;
 
-    bdrv_delete(s->test_file);
+    bdrv_unref(s->test_file);
     s->test_file = NULL;
 }
 
diff --git a/block/cow.c b/block/cow.c
index 1cc2e89..767639c 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -314,7 +314,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
     }
 
 exit:
-    bdrv_delete(cow_bs);
+    bdrv_unref(cow_bs);
     return ret;
 }
 
diff --git a/block/iscsi.c b/block/iscsi.c
index 123f058..df3a52d 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1294,7 +1294,7 @@ out:
     }
     g_free(bs->opaque);
     bs->opaque = NULL;
-    bdrv_delete(bs);
+    bdrv_unref(bs);
     return ret;
 }
 
diff --git a/block/mirror.c b/block/mirror.c
index bed4a7e..6662f55 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -480,7 +480,7 @@ immediate_exit:
         bdrv_swap(s->target, s->common.bs);
     }
     bdrv_close(s->target);
-    bdrv_delete(s->target);
+    bdrv_unref(s->target);
     block_job_completed(&s->common, ret);
 }
 
diff --git a/block/qcow.c b/block/qcow.c
index 5239bd6..6b891ac 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -751,7 +751,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options)
     g_free(tmp);
     ret = 0;
 exit:
-    bdrv_delete(qcow_bs);
+    bdrv_unref(qcow_bs);
     return ret;
 }
 
diff --git a/block/qcow2.c b/block/qcow2.c
index 3376901..c236505 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1390,7 +1390,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
 
     ret = 0;
 out:
-    bdrv_delete(bs);
+    bdrv_unref(bs);
     return ret;
 }
 
diff --git a/block/qed.c b/block/qed.c
index f767b05..0e1b767 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -599,7 +599,7 @@ static int qed_create(const char *filename, uint32_t cluster_size,
     ret = 0; /* success */
 out:
     g_free(l1_table);
-    bdrv_delete(bs);
+    bdrv_unref(bs);
     return ret;
 }
 
diff --git a/block/sheepdog.c b/block/sheepdog.c
index afe0533..9aebeb6 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1447,7 +1447,7 @@ static int sd_prealloc(const char *filename)
     }
 out:
     if (bs) {
-        bdrv_delete(bs);
+        bdrv_unref(bs);
     }
     g_free(buf);
 
@@ -1526,13 +1526,13 @@ static int sd_create(const char *filename, QEMUOptionParameter *options)
 
         if (!is_snapshot(&s->inode)) {
             error_report("cannot clone from a non snapshot vdi");
-            bdrv_delete(bs);
+            bdrv_unref(bs);
             ret = -EINVAL;
             goto out;
         }
 
         base_vid = s->inode.vdi_id;
-        bdrv_delete(bs);
+        bdrv_unref(bs);
     }
 
     ret = do_sd_create(s, vdi, vdi_size, base_vid, &vid, 0);
diff --git a/block/snapshot.c b/block/snapshot.c
index 6c6d9de..8f61cc0 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -99,7 +99,7 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
         ret = bdrv_snapshot_goto(bs->file, snapshot_id);
         open_ret = drv->bdrv_open(bs, NULL, bs->open_flags);
         if (open_ret < 0) {
-            bdrv_delete(bs->file);
+            bdrv_unref(bs->file);
             bs->drv = NULL;
             return open_ret;
         }
diff --git a/block/stream.c b/block/stream.c
index 7fe9e48..97cc14b 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -68,7 +68,7 @@ static void close_unused_images(BlockDriverState *top, BlockDriverState *base,
         unused = intermediate;
         intermediate = intermediate->backing_hd;
         unused->backing_hd = NULL;
-        bdrv_delete(unused);
+        bdrv_unref(unused);
     }
     top->backing_hd = base;
 }
diff --git a/block/vmdk.c b/block/vmdk.c
index 346bb5c..fa3bd1c 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -216,7 +216,7 @@ static void vmdk_free_extents(BlockDriverState *bs)
         g_free(e->l2_cache);
         g_free(e->l1_backup_table);
         if (e->file != bs->file) {
-            bdrv_delete(e->file);
+            bdrv_unref(e->file);
         }
     }
     g_free(s->extents);
@@ -745,7 +745,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
             /* SPARSE extent */
             ret = vmdk_open_sparse(bs, extent_file, bs->open_flags);
             if (ret) {
-                bdrv_delete(extent_file);
+                bdrv_unref(extent_file);
                 return ret;
             }
         } else {
@@ -1634,15 +1634,15 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
         BlockDriverState *bs = bdrv_new("");
         ret = bdrv_open(bs, backing_file, NULL, 0, NULL);
         if (ret != 0) {
-            bdrv_delete(bs);
+            bdrv_unref(bs);
             return ret;
         }
         if (strcmp(bs->drv->format_name, "vmdk")) {
-            bdrv_delete(bs);
+            bdrv_unref(bs);
             return -EINVAL;
         }
         parent_cid = vmdk_read_cid(bs, 0);
-        bdrv_delete(bs);
+        bdrv_unref(bs);
         snprintf(parent_desc_line, sizeof(parent_desc_line),
                 "parentFileNameHint=\"%s\"", backing_file);
     }
diff --git a/block/vvfat.c b/block/vvfat.c
index a827d91..2178a13 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2894,7 +2894,7 @@ static int write_target_commit(BlockDriverState *bs, int64_t sector_num,
 
 static void write_target_close(BlockDriverState *bs) {
     BDRVVVFATState* s = *((BDRVVVFATState**) bs->opaque);
-    bdrv_delete(s->qcow);
+    bdrv_unref(s->qcow);
     g_free(s->qcow_filename);
 }
 
@@ -2935,7 +2935,7 @@ static int enable_write_target(BDRVVVFATState *s)
     ret = bdrv_open(s->qcow, s->qcow_filename, NULL,
             BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow);
     if (ret < 0) {
-        bdrv_delete(s->qcow);
+        bdrv_unref(s->qcow);
         goto err;
     }
 
diff --git a/blockdev.c b/blockdev.c
index 41b0a49..5b86c9d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -213,7 +213,7 @@ static void bdrv_format_print(void *opaque, const char *name)
 static void drive_uninit(DriveInfo *dinfo)
 {
     qemu_opts_del(dinfo->opts);
-    bdrv_delete(dinfo->bdrv);
+    bdrv_unref(dinfo->bdrv);
     g_free(dinfo->id);
     QTAILQ_REMOVE(&drives, dinfo, next);
     g_free(dinfo->serial);
@@ -724,7 +724,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
 err:
     qemu_opts_del(opts);
     QDECREF(bs_opts);
-    bdrv_delete(dinfo->bdrv);
+    bdrv_unref(dinfo->bdrv);
     g_free(dinfo->id);
     QTAILQ_REMOVE(&drives, dinfo, next);
     g_free(dinfo);
@@ -994,7 +994,7 @@ static void external_snapshot_abort(BlkTransactionState *common)
     ExternalSnapshotState *state =
                              DO_UPCAST(ExternalSnapshotState, common, common);
     if (state->new_bs) {
-        bdrv_delete(state->new_bs);
+        bdrv_unref(state->new_bs);
     }
 }
 
@@ -1597,7 +1597,7 @@ void qmp_drive_backup(const char *device, const char *target,
     target_bs = bdrv_new("");
     ret = bdrv_open(target_bs, target, NULL, flags, drv);
     if (ret < 0) {
-        bdrv_delete(target_bs);
+        bdrv_unref(target_bs);
         error_setg_file_open(errp, -ret, target);
         return;
     }
@@ -1605,7 +1605,7 @@ void qmp_drive_backup(const char *device, const char *target,
     backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error,
                  block_job_cb, bs, &local_err);
     if (local_err != NULL) {
-        bdrv_delete(target_bs);
+        bdrv_unref(target_bs);
         error_propagate(errp, local_err);
         return;
     }
@@ -1737,7 +1737,7 @@ void qmp_drive_mirror(const char *device, const char *target,
     target_bs = bdrv_new("");
     ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv);
     if (ret < 0) {
-        bdrv_delete(target_bs);
+        bdrv_unref(target_bs);
         error_setg_file_open(errp, -ret, target);
         return;
     }
@@ -1746,7 +1746,7 @@ void qmp_drive_mirror(const char *device, const char *target,
                  on_source_error, on_target_error,
                  block_job_cb, bs, &local_err);
     if (local_err != NULL) {
-        bdrv_delete(target_bs);
+        bdrv_unref(target_bs);
         error_propagate(errp, local_err);
         return;
     }
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 727f433..8bfa04e 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -813,7 +813,7 @@ static int blk_connect(struct XenDevice *xendev)
                                                            readonly);
             if (bdrv_open(blkdev->bs,
                           blkdev->filename, NULL, qflags, drv) != 0) {
-                bdrv_delete(blkdev->bs);
+                bdrv_unref(blkdev->bs);
                 blkdev->bs = NULL;
             }
         }
@@ -926,7 +926,7 @@ static void blk_disconnect(struct XenDevice *xendev)
             /* close/delete only if we created it ourself */
             bdrv_close(blkdev->bs);
             bdrv_detach_dev(blkdev->bs, blkdev);
-            bdrv_delete(blkdev->bs);
+            bdrv_unref(blkdev->bs);
         }
         blkdev->bs = NULL;
     }
diff --git a/include/block/block.h b/include/block/block.h
index b33ef62..1929355 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -123,7 +123,6 @@ BlockDriverState *bdrv_new(const char *device_name);
 void bdrv_make_anon(BlockDriverState *bs);
 void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
 void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
-void bdrv_delete(BlockDriverState *bs);
 int bdrv_parse_cache_flags(const char *mode, int *flags);
 int bdrv_parse_discard_flags(const char *mode, int *flags);
 int bdrv_file_open(BlockDriverState **pbs, const char *filename,
diff --git a/qemu-img.c b/qemu-img.c
index b9a848d..3eaf1f1 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -298,7 +298,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
     return bs;
 fail:
     if (bs) {
-        bdrv_delete(bs);
+        bdrv_unref(bs);
     }
     return NULL;
 }
@@ -652,7 +652,7 @@ static int img_check(int argc, char **argv)
 
 fail:
     qapi_free_ImageCheck(check);
-    bdrv_delete(bs);
+    bdrv_unref(bs);
 
     return ret;
 }
@@ -722,7 +722,7 @@ static int img_commit(int argc, char **argv)
         break;
     }
 
-    bdrv_delete(bs);
+    bdrv_unref(bs);
     if (ret) {
         return 1;
     }
@@ -1104,11 +1104,11 @@ static int img_compare(int argc, char **argv)
     ret = 0;
 
 out:
-    bdrv_delete(bs2);
+    bdrv_unref(bs2);
     qemu_vfree(buf1);
     qemu_vfree(buf2);
 out2:
-    bdrv_delete(bs1);
+    bdrv_unref(bs1);
 out3:
     qemu_progress_end();
     return ret;
@@ -1538,12 +1538,12 @@ out:
     free_option_parameters(param);
     qemu_vfree(buf);
     if (out_bs) {
-        bdrv_delete(out_bs);
+        bdrv_unref(out_bs);
     }
     if (bs) {
         for (bs_i = 0; bs_i < bs_n; bs_i++) {
             if (bs[bs_i]) {
-                bdrv_delete(bs[bs_i]);
+                bdrv_unref(bs[bs_i]);
             }
         }
         g_free(bs);
@@ -1681,7 +1681,7 @@ static ImageInfoList *collect_image_info_list(const char *filename,
         *last = elem;
         last = &elem->next;
 
-        bdrv_delete(bs);
+        bdrv_unref(bs);
 
         filename = fmt = NULL;
         if (chain) {
@@ -1895,7 +1895,7 @@ static int img_snapshot(int argc, char **argv)
     }
 
     /* Cleanup */
-    bdrv_delete(bs);
+    bdrv_unref(bs);
     if (ret) {
         return 1;
     }
@@ -2170,14 +2170,14 @@ out:
     /* Cleanup */
     if (!unsafe) {
         if (bs_old_backing != NULL) {
-            bdrv_delete(bs_old_backing);
+            bdrv_unref(bs_old_backing);
         }
         if (bs_new_backing != NULL) {
-            bdrv_delete(bs_new_backing);
+            bdrv_unref(bs_new_backing);
         }
     }
 
-    bdrv_delete(bs);
+    bdrv_unref(bs);
     if (ret) {
         return 1;
     }
@@ -2300,7 +2300,7 @@ static int img_resize(int argc, char **argv)
     }
 out:
     if (bs) {
-        bdrv_delete(bs);
+        bdrv_unref(bs);
     }
     if (ret) {
         return 1;
diff --git a/qemu-io.c b/qemu-io.c
index d54dc86..71f4ff1 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -32,7 +32,7 @@ static char **cmdline;
 
 static int close_f(BlockDriverState *bs, int argc, char **argv)
 {
-    bdrv_delete(bs);
+    bdrv_unref(bs);
     qemuio_bs = NULL;
     return 0;
 }
@@ -61,7 +61,7 @@ static int openfile(char *name, int flags, int growable)
 
         if (bdrv_open(qemuio_bs, name, NULL, flags, NULL) < 0) {
             fprintf(stderr, "%s: can't open device %s\n", progname, name);
-            bdrv_delete(qemuio_bs);
+            bdrv_unref(qemuio_bs);
             qemuio_bs = NULL;
             return 1;
         }
@@ -422,7 +422,7 @@ int main(int argc, char **argv)
     bdrv_drain_all();
 
     if (qemuio_bs) {
-        bdrv_delete(qemuio_bs);
+        bdrv_unref(qemuio_bs);
     }
     return 0;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v5 5/8] migration: omit drive ref as we have bdrv_ref now

[Fam Zheng]

block-migration.c does not actually use DriveInfo anywhere.  Hence it's
safe to drive ref code, we really only care about referencing BDS.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block-migration.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index f803f20..daf9ec1 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -336,8 +336,8 @@ static void init_blk_migration_it(void *opaque, BlockDriverState *bs)
         bmds->completed_sectors = 0;
         bmds->shared_base = block_mig_state.shared_base;
         alloc_aio_bitmap(bmds);
-        drive_get_ref(drive_get_by_blockdev(bs));
         bdrv_set_in_use(bs, 1);
+        bdrv_ref(bs);
 
         block_mig_state.total_sector_sum += sectors;
 
@@ -575,7 +575,7 @@ static void blk_mig_cleanup(void)
     while ((bmds = QSIMPLEQ_FIRST(&block_mig_state.bmds_list)) != NULL) {
         QSIMPLEQ_REMOVE_HEAD(&block_mig_state.bmds_list, entry);
         bdrv_set_in_use(bmds->bs, 0);
-        drive_put_ref(drive_get_by_blockdev(bmds->bs));
+        bdrv_unref(bmds->bs);
         g_free(bmds->aio_bitmap);
         g_free(bmds);
     }
-- 
1.8.3.1

[Qemu-devel] [PATCH v5 6/8] xen_disk: simplify blk_disconnect with refcnt

[Fam Zheng]

We call bdrv_attach_dev when initializing whether or not bs is created
locally, so call bdrv_detach_dev and let the refcnt handle the
lifecycle.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/block/xen_disk.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 8bfa04e..668cc06 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -824,6 +824,9 @@ static int blk_connect(struct XenDevice *xendev)
         /* setup via qemu cmdline -> already setup for us */
         xen_be_printf(&blkdev->xendev, 2, "get configured bdrv (cmdline setup)\n");
         blkdev->bs = blkdev->dinfo->bdrv;
+        /* blkdev->bs is not create by us, we get a reference
+         * so we can bdrv_unref() unconditionally */
+        bdrv_ref(blkdev->bs);
     }
     bdrv_attach_dev_nofail(blkdev->bs, blkdev);
     blkdev->file_size = bdrv_getlength(blkdev->bs);
@@ -922,12 +925,8 @@ static void blk_disconnect(struct XenDevice *xendev)
     struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
 
     if (blkdev->bs) {
-        if (!blkdev->dinfo) {
-            /* close/delete only if we created it ourself */
-            bdrv_close(blkdev->bs);
-            bdrv_detach_dev(blkdev->bs, blkdev);
-            bdrv_unref(blkdev->bs);
-        }
+        bdrv_detach_dev(blkdev->bs, blkdev);
+        bdrv_unref(blkdev->bs);
         blkdev->bs = NULL;
     }
     xen_be_unbind_evtchn(&blkdev->xendev);
-- 
1.8.3.1

[Qemu-devel] [PATCH v5 7/8] nbd: use BlockDriverState refcnt

[Fam Zheng]

Previously, nbd calls drive_get_ref() on the drive of bs. A BDS doesn't
always have associated dinfo, which nbd doesn't care either. We already
have BDS ref count, so use it to make it safe for a BDS w/o blockdev.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 blockdev-nbd.c | 10 +---------
 nbd.c          |  5 +++++
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 95f10c8..922cf56 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -69,12 +69,6 @@ static void nbd_close_notifier(Notifier *n, void *data)
     g_free(cn);
 }
 
-static void nbd_server_put_ref(NBDExport *exp)
-{
-    BlockDriverState *bs = nbd_export_get_blockdev(exp);
-    drive_put_ref(drive_get_by_blockdev(bs));
-}
-
 void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
                         Error **errp)
 {
@@ -105,11 +99,9 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
         writable = false;
     }
 
-    exp = nbd_export_new(bs, 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY,
-                         nbd_server_put_ref);
+    exp = nbd_export_new(bs, 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY, NULL);
 
     nbd_export_set_name(exp, device);
-    drive_get_ref(drive_get_by_blockdev(bs));
 
     n = g_malloc0(sizeof(NBDCloseNotifier));
     n->n.notify = nbd_close_notifier;
diff --git a/nbd.c b/nbd.c
index 2606403..f258cdd 100644
--- a/nbd.c
+++ b/nbd.c
@@ -881,6 +881,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
     exp->nbdflags = nbdflags;
     exp->size = size == -1 ? bdrv_getlength(bs) : size;
     exp->close = close;
+    bdrv_ref(bs);
     return exp;
 }
 
@@ -927,6 +928,10 @@ void nbd_export_close(NBDExport *exp)
     }
     nbd_export_set_name(exp, NULL);
     nbd_export_put(exp);
+    if (exp->bs) {
+        bdrv_unref(exp->bs);
+        exp->bs = NULL;
+    }
 }
 
 void nbd_export_get(NBDExport *exp)
-- 
1.8.3.1

[Qemu-devel] [PATCH v5 8/8] block: use BDS ref for block jobs

[Fam Zheng]

Block jobs used drive_get_ref(drive_get_by_blockdev(bs)) to avoid BDS
being deleted. Now we have BDS reference count, and block jobs don't
care about dinfo, so replace them to get cleaner code. It is also the
safe way when BDS has no drive info.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 blockdev.c | 53 -----------------------------------------------------
 blockjob.c |  3 +++
 2 files changed, 3 insertions(+), 53 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 5b86c9d..f6961e1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -233,37 +233,6 @@ void drive_get_ref(DriveInfo *dinfo)
     dinfo->refcount++;
 }
 
-typedef struct {
-    QEMUBH *bh;
-    DriveInfo *dinfo;
-} DrivePutRefBH;
-
-static void drive_put_ref_bh(void *opaque)
-{
-    DrivePutRefBH *s = opaque;
-
-    drive_put_ref(s->dinfo);
-    qemu_bh_delete(s->bh);
-    g_free(s);
-}
-
-/*
- * Release a drive reference in a BH
- *
- * It is not possible to use drive_put_ref() from a callback function when the
- * callers still need the drive.  In such cases we schedule a BH to release the
- * reference.
- */
-static void drive_put_ref_bh_schedule(DriveInfo *dinfo)
-{
-    DrivePutRefBH *s;
-
-    s = g_new(DrivePutRefBH, 1);
-    s->bh = qemu_bh_new(drive_put_ref_bh, s);
-    s->dinfo = dinfo;
-    qemu_bh_schedule(s->bh);
-}
-
 static int parse_block_error_action(const char *buf, bool is_read)
 {
     if (!strcmp(buf, "ignore")) {
@@ -1394,8 +1363,6 @@ static void block_job_cb(void *opaque, int ret)
         monitor_protocol_event(QEVENT_BLOCK_JOB_COMPLETED, obj);
     }
     qobject_decref(obj);
-
-    drive_put_ref_bh_schedule(drive_get_by_blockdev(bs));
 }
 
 void qmp_block_stream(const char *device, bool has_base,
@@ -1431,12 +1398,6 @@ void qmp_block_stream(const char *device, bool has_base,
         error_propagate(errp, local_err);
         return;
     }
-
-    /* Grab a reference so hotplug does not delete the BlockDriverState from
-     * underneath us.
-     */
-    drive_get_ref(drive_get_by_blockdev(bs));
-
     trace_qmp_block_stream(bs, bs->job);
 }
 
@@ -1493,10 +1454,6 @@ void qmp_block_commit(const char *device,
         error_propagate(errp, local_err);
         return;
     }
-    /* Grab a reference so hotplug does not delete the BlockDriverState from
-     * underneath us.
-     */
-    drive_get_ref(drive_get_by_blockdev(bs));
 }
 
 void qmp_drive_backup(const char *device, const char *target,
@@ -1609,11 +1566,6 @@ void qmp_drive_backup(const char *device, const char *target,
         error_propagate(errp, local_err);
         return;
     }
-
-    /* Grab a reference so hotplug does not delete the BlockDriverState from
-     * underneath us.
-     */
-    drive_get_ref(drive_get_by_blockdev(bs));
 }
 
 #define DEFAULT_MIRROR_BUF_SIZE   (10 << 20)
@@ -1750,11 +1702,6 @@ void qmp_drive_mirror(const char *device, const char *target,
         error_propagate(errp, local_err);
         return;
     }
-
-    /* Grab a reference so hotplug does not delete the BlockDriverState from
-     * underneath us.
-     */
-    drive_get_ref(drive_get_by_blockdev(bs));
 }
 
 static BlockJob *find_block_job(const char *device)
diff --git a/blockjob.c b/blockjob.c
index ca80df1..832d558 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -46,6 +46,7 @@ void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs,
         return NULL;
     }
     bdrv_set_in_use(bs, 1);
+    bdrv_ref(bs);
 
     job = g_malloc0(job_type->instance_size);
     job->job_type      = job_type;
@@ -80,6 +81,8 @@ void block_job_completed(BlockJob *job, int ret)
     bs->job = NULL;
     g_free(job);
     bdrv_set_in_use(bs, 0);
+    bdrv_unref(bs);
+    job->bs = NULL;
 }
 
 void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v5 0/8] Implement reference count for BlockDriverState [resend]

[Stefan Hajnoczi]

On Fri, Aug 09, 2013 at 06:01:53PM +0800, Fam Zheng wrote:
> [resend to the correct list]
> 
> BlockDriverState lifecycle management is needed by future features such as
> image fleecing and blockdev-add. This series adds reference count to
> BlockDriverState.
> 
> The first two patches clean up two odd BlockDriverState use cases, so all code
> uses bdrv_new() to create BlockDriverState instance.
> 
> Then implemented bdrv_ref() and bdrv_unref() to operate on refcnt: Initially,
> refcnt is 1, which means bdrv_unref is effectively a bdrv_delete() here. So
> patch 04 has a search and replace to convert bdrv_delete to bdrv_unref, before
> bdrv_ref is used anywhere. 05~08 patches calls bdrv_ref for device attach,
> block-migration and nbd.
> 
> The rule is: Either bdrv_ref() or bdrv_new() must have a matching
> bdrv_unref() call, and the last matching bdrv_unref deletes the bs.
> 
> v4:
>     08: Added, let block job use BDS reference.
>     02: Fix leak of bs.opaque
> 
> v3:
>     03: Removed unnecessary bdrv_close() call.
> 
> v2:
>     05: Removed: "block: use BlockDriverState refcnt for device attach/detach"
>     07: Fix xen_disk blk_disconnect() as it depended on device attach refcnt.

Sorry, can't merge this because it breaks qemu-iotests 041 and 055:

  $ ./check -qcow2 055 041

Please always run qemu-iotests before submitting patches.

Stefan

Re: [Qemu-devel] [PATCH v5 0/8] Implement reference count for BlockDriverState [resend]

[Fam Zheng]

On Thu, 08/22 13:38, Stefan Hajnoczi wrote:
> On Fri, Aug 09, 2013 at 06:01:53PM +0800, Fam Zheng wrote:
> > [resend to the correct list]
> > 
> > BlockDriverState lifecycle management is needed by future features such as
> > image fleecing and blockdev-add. This series adds reference count to
> > BlockDriverState.
> > 
> > The first two patches clean up two odd BlockDriverState use cases, so all code
> > uses bdrv_new() to create BlockDriverState instance.
> > 
> > Then implemented bdrv_ref() and bdrv_unref() to operate on refcnt: Initially,
> > refcnt is 1, which means bdrv_unref is effectively a bdrv_delete() here. So
> > patch 04 has a search and replace to convert bdrv_delete to bdrv_unref, before
> > bdrv_ref is used anywhere. 05~08 patches calls bdrv_ref for device attach,
> > block-migration and nbd.
> > 
> > The rule is: Either bdrv_ref() or bdrv_new() must have a matching
> > bdrv_unref() call, and the last matching bdrv_unref deletes the bs.
> > 
> > v4:
> >     08: Added, let block job use BDS reference.
> >     02: Fix leak of bs.opaque
> > 
> > v3:
> >     03: Removed unnecessary bdrv_close() call.
> > 
> > v2:
> >     05: Removed: "block: use BlockDriverState refcnt for device attach/detach"
> >     07: Fix xen_disk blk_disconnect() as it depended on device attach refcnt.
> 
> Sorry, can't merge this because it breaks qemu-iotests 041 and 055:
> 
>   $ ./check -qcow2 055 041

041 works for me, so I will post v6 to fix 055. If 041 still not working
then, I'll look into it.
> 
> Please always run qemu-iotests before submitting patches.
> 
OK.

Thanks

Fam

Re: [Qemu-devel] [PATCH v5 0/8] Implement reference count for BlockDriverState [resend]

[Wenchao Xia]

于 2013-8-22 19:38, Stefan Hajnoczi 写道:
> On Fri, Aug 09, 2013 at 06:01:53PM +0800, Fam Zheng wrote:
>> [resend to the correct list]
>>
>> BlockDriverState lifecycle management is needed by future features such as
>> image fleecing and blockdev-add. This series adds reference count to
>> BlockDriverState.
>>
>> The first two patches clean up two odd BlockDriverState use cases, so all code
>> uses bdrv_new() to create BlockDriverState instance.
>>
>> Then implemented bdrv_ref() and bdrv_unref() to operate on refcnt: Initially,
>> refcnt is 1, which means bdrv_unref is effectively a bdrv_delete() here. So
>> patch 04 has a search and replace to convert bdrv_delete to bdrv_unref, before
>> bdrv_ref is used anywhere. 05~08 patches calls bdrv_ref for device attach,
>> block-migration and nbd.
>>
>> The rule is: Either bdrv_ref() or bdrv_new() must have a matching
>> bdrv_unref() call, and the last matching bdrv_unref deletes the bs.
>>
>> v4:
>>      08: Added, let block job use BDS reference.
>>      02: Fix leak of bs.opaque
>>
>> v3:
>>      03: Removed unnecessary bdrv_close() call.
>>
>> v2:
>>      05: Removed: "block: use BlockDriverState refcnt for device attach/detach"
>>      07: Fix xen_disk blk_disconnect() as it depended on device attach refcnt.
>
> Sorry, can't merge this because it breaks qemu-iotests 041 and 055:
>
>    $ ./check -qcow2 055 041
>
> Please always run qemu-iotests before submitting patches.
>
> Stefan
>
Hi,
   What is the correct steps to run full qemu-iotests?
I modified qemu-iotests-quick.sh as:

#!/bin/sh

# We don't know which of the system emulator binaries there is (or if 
there is
# any at all), so the 'quick' group doesn't contain any tests that require
# running qemu proper. Assign a fake binary name so that qemu-iotests 
doesn't
# complain about the missing binary.
export QEMU_PROG="$(pwd)/x86_64-softmmu/qemu-system-x86_64"

export QEMU_IMG_PROG="$(pwd)/qemu-img"
export QEMU_IO_PROG="$(pwd)/qemu-io"
export QEMU_NBD_PROG="$(pwd)/qemu-nbd"

cd $SRC_PATH/tests/qemu-iotests

ret=0
./check -T -nocache -qcow2 || ret=1

exit $ret

   Then make check-block, 026 038 fail, 038 sometimes fail.
The code from is upstream, host is RH6.3 @ x86_64. Do I missed some
steps?
-- 
Best Regards

Wenchao Xia

Re: [Qemu-devel] [PATCH v5 0/8] Implement reference count for BlockDriverState [resend]

[Stefan Hajnoczi]

On Fri, Aug 23, 2013 at 10:17:37AM +0800, Wenchao Xia wrote:
>   What is the correct steps to run full qemu-iotests?
> I modified qemu-iotests-quick.sh as:
> 
> #!/bin/sh
> 
> # We don't know which of the system emulator binaries there is (or
> if there is
> # any at all), so the 'quick' group doesn't contain any tests that require
> # running qemu proper. Assign a fake binary name so that
> qemu-iotests doesn't
> # complain about the missing binary.
> export QEMU_PROG="$(pwd)/x86_64-softmmu/qemu-system-x86_64"
> 
> export QEMU_IMG_PROG="$(pwd)/qemu-img"
> export QEMU_IO_PROG="$(pwd)/qemu-io"
> export QEMU_NBD_PROG="$(pwd)/qemu-nbd"
> 
> cd $SRC_PATH/tests/qemu-iotests
> 
> ret=0
> ./check -T -nocache -qcow2 || ret=1
> 
> exit $ret
> 
>   Then make check-block, 026 038 fail, 038 sometimes fail.
> The code from is upstream, host is RH6.3 @ x86_64. Do I missed some
> steps?

You are running it correctly.

There are a couple of known failures in qemu-iotests.  It would be good
to fix them but no one has posted patches so far.  If in doubt, run
qemu-iotests on qemu.git/master to see which tests are known failures.

Stefan