Re: [Qemu-devel] [PATCH v2 07/22] block: Add bdrv_close_all() handlers

[Max Reitz <mreitz@redhat.com>]

On 2015-02-09 at 13:38, Max Reitz wrote:
> Every time a reference to a BlockBackend is taken, a notifier for
> bdrv_close_all() has to be deposited so the reference holder can
> relinquish its reference when bdrv_close_all() is called. That notifier
> should be revoked on a bdrv_unref() call.
>
> Add a Notifier * parameter to all the functions changing the reference
> count of a BlockBackend: blk_new(), blk_new_with_bs(), blk_new_open(),
> blk_ref() and blk_unref().
>
> By dropping the manual reference handling in hw/block/xen_disk.c, the
> blk_unref() in hw/ide/piix.c can be dropped as well.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   block/block-backend.c          | 69 ++++++++++++++++++++++++++++++++++++------
>   blockdev.c                     | 62 ++++++++++++++++++++++++++++++++++---
>   device-hotplug.c               |  2 +-
>   hw/block/xen_disk.c            | 16 +++++++---
>   hw/ide/piix.c                  |  1 -
>   include/sysemu/block-backend.h | 13 +++++---
>   include/sysemu/blockdev.h      |  3 ++
>   nbd.c                          | 26 ++++++++++++++--
>   qemu-img.c                     | 39 ++++++++++++------------
>   qemu-io.c                      |  6 ++--
>   qemu-nbd.c                     |  6 ++--
>   11 files changed, 189 insertions(+), 54 deletions(-)

[snip]

> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index ac1e5d6..5e6e4b4 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -694,7 +694,8 @@ int main(int argc, char **argv)
>       }
>   
>       srcpath = argv[optind];
> -    blk = blk_new_open("hda", srcpath, NULL, options, flags, &local_err);
> +    /* the reference will be passed on nbd_export_new() */
> +    blk = blk_new_open("hda", srcpath, NULL, options, flags, NULL, &local_err);
>       if (!blk) {
>           errx(EXIT_FAILURE, "Failed to blk_new_open '%s': %s", argv[optind],
>                error_get_pretty(local_err));
> @@ -728,7 +729,9 @@ int main(int argc, char **argv)
>           }
>       }
>   
> +    /* nbd_export_new() takes the reference to blk */
>       exp = nbd_export_new(blk, dev_offset, fd_size, nbdflags, nbd_export_closed);
> +    blk_unref(blk, NULL);
>   
>       if (sockpath) {
>           fd = unix_socket_incoming(sockpath);
> @@ -773,7 +776,6 @@ int main(int argc, char **argv)
>           }
>       } while (state != TERMINATED);
>   
> -    blk_unref(blk);
>       if (sockpath) {
>           unlink(sockpath);
>       }

Because qemu-nbd does not install a handler for relinquishing its 
reference, it has to make sure it drops that reference before a 
bdrv_close_all() occurs. Therefore, err() and errx() calls after 
blk_new_open() (except for the if (!blk) path) and before blk_unref(blk, 
NULL) need to be preceded by a blk_unref(blk, NULL).

I'll fix it in v3.

Max