Re: [Qemu-devel] [PATCH v3 2/7] iscsi: use bdrv_new() instead of stack structure

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 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;
>   }
> 


-- 
Best Regards

Wenchao Xia