Re: [Qemu-devel] [PATCH 3/7] nbd: use BDS refcount

[Paolo Bonzini <pbonzini@redhat.com>]

Il 02/07/2013 07:59, Fam Zheng ha scritto:
> Previously, nbd call drive_get_ref on the drive of bs. A BDS doesn't
> always have associated dinfo, it's more proper to use bdrv_get_ref().

This has the important side effect of marking the exported disk as
"in_use" (to use the terms before the series).  Right now you can serve
a disk and, at the same time, stream it or mirror it or create a live
snapshot of it.

Do we really want to block anything for a device being served?  Perhaps
truncation, but maybe not even that.  The NBD server is meant to be as
unobtrusive as possible (in some sense NBD accesses are the same as
guest accesses).

Paolo

> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  blockdev-nbd.c | 9 +--------
>  nbd.c          | 5 +++++
>  2 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> index 95f10c8..d8bcd6f 100644
> --- a/blockdev-nbd.c
> +++ b/blockdev-nbd.c
> @@ -69,12 +69,6 @@ static void nbd_close_notifier(Notifier *n, void *data)
>      g_free(cn);
>  }
>  
> -static void nbd_server_put_ref(NBDExport *exp)
> -{
> -    BlockDriverState *bs = nbd_export_get_blockdev(exp);
> -    drive_put_ref(drive_get_by_blockdev(bs));
> -}
> -
>  void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
>                          Error **errp)
>  {
> @@ -106,10 +100,9 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
>      }
>  
>      exp = nbd_export_new(bs, 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY,
> -                         nbd_server_put_ref);
> +                         NULL);
>  
>      nbd_export_set_name(exp, device);
> -    drive_get_ref(drive_get_by_blockdev(bs));
>  
>      n = g_malloc0(sizeof(NBDCloseNotifier));
>      n->n.notify = nbd_close_notifier;
> diff --git a/nbd.c b/nbd.c
> index 2606403..f28b9fb 100644
> --- a/nbd.c
> +++ b/nbd.c
> @@ -881,6 +881,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
>      exp->nbdflags = nbdflags;
>      exp->size = size == -1 ? bdrv_getlength(bs) : size;
>      exp->close = close;
> +    bdrv_get_ref(bs);
>      return exp;
>  }
>  
> @@ -927,6 +928,10 @@ void nbd_export_close(NBDExport *exp)
>      }
>      nbd_export_set_name(exp, NULL);
>      nbd_export_put(exp);
> +    if (exp->bs) {
> +        bdrv_put_ref(exp->bs);
> +        exp->bs = NULL;
> +    }
>  }
>  
>  void nbd_export_get(NBDExport *exp)
>