Re: [Qemu-devel] [PATCH v3 6/7] xen_disk: simplify blk_disconnect with refcnt

[Wenchao Xia]

Better to split it into two patches:
1 bugfix: always call bdrv_detach_dev().
2 use refcnt to manage lifecycle.

> 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 <address@hidden>
> ---
>   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 99537e8..39757d9 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -812,6 +812,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);
> @@ -910,12 +913,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);
> 


-- 
Best Regards

Wenchao Xia