[Qemu-devel] [PATCH v3 6/7] 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 
---
 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);
-- 
1.8.3.4

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 
> ---
>   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