From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36780) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Utxdr-0004i9-Ab for qemu-devel@nongnu.org; Tue, 02 Jul 2013 06:17:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Utxdp-0006mY-RI for qemu-devel@nongnu.org; Tue, 02 Jul 2013 06:16:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54574) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Utxdp-0006lR-Jq for qemu-devel@nongnu.org; Tue, 02 Jul 2013 06:16:57 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r62AGuRZ015230 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 2 Jul 2013 06:16:56 -0400 Message-ID: <51D2A890.4060806@redhat.com> Date: Tue, 02 Jul 2013 12:16:48 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1372744789-997-1-git-send-email-famz@redhat.com> <1372744789-997-4-git-send-email-famz@redhat.com> In-Reply-To: <1372744789-997-4-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/7] nbd: use BDS refcount List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: kwolf@redhat.com, obarenbo@redhat.com, armbru@redhat.com, roliveri@redhat.com, hbrock@redhat.com, qemu-devel@nongnu.org, rjones@redhat.com, pmyers@redhat.com, imain@redhat.com, stefanha@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 > --- > 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) >