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 <f...@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);
>
--
Best Regards
Wenchao Xia
