Patchwork [v3,2/7] iscsi: use bdrv_new() instead of stack structure

login
register
mail settings
Submitter Fam Zheng
Date July 31, 2013, 10:13 a.m.
Message ID <1375265640-27307-3-git-send-email-famz@redhat.com>
Download mbox | patch
Permalink /patch/263674/
State New
Headers show

Comments

Fam Zheng - July 31, 2013, 10:13 a.m.
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 | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
Wayne Xia - Aug. 2, 2013, 7:34 a.m.
于 2013-7-31 18:13, 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 | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 5f28c6a..db8a699 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1247,11 +1247,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) {
> @@ -1261,12 +1261,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) {
> @@ -1280,7 +1280,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;
>       }
> @@ -1290,7 +1290,7 @@ out:
>       if (iscsilun->iscsi != NULL) {
>           iscsi_destroy_context(iscsilun->iscsi);
>       }
> -    g_free(bs.opaque);
  bs,opaque seems leaked. bdrv_delete() will not free it unless bs->drv
!= NULL.


> +    bdrv_delete(bs);
>       return ret;
>   }
>

Patch

diff --git a/block/iscsi.c b/block/iscsi.c
index 5f28c6a..db8a699 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1247,11 +1247,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) {
@@ -1261,12 +1261,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) {
@@ -1280,7 +1280,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;
     }
@@ -1290,7 +1290,7 @@  out:
     if (iscsilun->iscsi != NULL) {
         iscsi_destroy_context(iscsilun->iscsi);
     }
-    g_free(bs.opaque);
+    bdrv_delete(bs);
     return ret;
 }