Patchwork [RFC,2/4] nbd: call drive_put_ref() only if dinfo exists

login
register
mail settings
Submitter Fam Zheng
Date July 29, 2013, 4:25 a.m.
Message ID <1375071932-31627-3-git-send-email-famz@redhat.com>
Download mbox | patch
Permalink /patch/262664/
State New
Headers show

Comments

Fam Zheng - July 29, 2013, 4:25 a.m.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 blockdev-nbd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
Stefan Hajnoczi - Aug. 21, 2013, 12:47 p.m.
On Mon, Jul 29, 2013 at 12:25:30PM +0800, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  blockdev-nbd.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> index 95f10c8..c75df19 100644
> --- a/blockdev-nbd.c
> +++ b/blockdev-nbd.c
> @@ -72,7 +72,10 @@ static void nbd_close_notifier(Notifier *n, void *data)
>  static void nbd_server_put_ref(NBDExport *exp)
>  {
>      BlockDriverState *bs = nbd_export_get_blockdev(exp);
> -    drive_put_ref(drive_get_by_blockdev(bs));
> +    DriveInfo *dinfo = drive_get_by_blockdev(bs);
> +    if (dinfo) {
> +        drive_put_ref(dinfo);
> +    }

This doesn't make sense to me.  If we hold a reference, we must release
it.

Once your refcount series is applied, NBD should be able to hold just
the BDS refcount.  Then we don't care about DriveInfo at all anymore
besides looking up the BDS when the user creates an export.

Stefan

Patch

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 95f10c8..c75df19 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -72,7 +72,10 @@  static void nbd_close_notifier(Notifier *n, void *data)
 static void nbd_server_put_ref(NBDExport *exp)
 {
     BlockDriverState *bs = nbd_export_get_blockdev(exp);
-    drive_put_ref(drive_get_by_blockdev(bs));
+    DriveInfo *dinfo = drive_get_by_blockdev(bs);
+    if (dinfo) {
+        drive_put_ref(dinfo);
+    }
 }
 
 void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,