Patchwork [v3,6/7] xen_disk: simplify blk_disconnect with refcnt

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

Comments

Fam Zheng - July 31, 2013, 10:13 a.m.
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(-)
Wayne Xia - Aug. 2, 2013, 8:24 a.m.
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 <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 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);
>

Patch

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