From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58925) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGbSr-0002Co-2f for qemu-devel@nongnu.org; Wed, 28 Jan 2015 17:52:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YGbSl-0001lC-W8 for qemu-devel@nongnu.org; Wed, 28 Jan 2015 17:52:01 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36865) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGbSl-0001kS-OY for qemu-devel@nongnu.org; Wed, 28 Jan 2015 17:51:55 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t0SMpshA018596 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 28 Jan 2015 17:51:54 -0500 Message-ID: <54C96808.3050502@redhat.com> Date: Wed, 28 Jan 2015 17:51:52 -0500 From: Max Reitz MIME-Version: 1.0 References: <1422300468-16216-1-git-send-email-mreitz@redhat.com> <1422300468-16216-5-git-send-email-mreitz@redhat.com> <54C96656.30206@redhat.com> In-Reply-To: <54C96656.30206@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 04/21] block: Add bdrv_close_all() handlers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: Kevin Wolf , Markus Armbruster , Stefan Hajnoczi On 2015-01-28 at 17:44, Eric Blake wrote: > On 01/26/2015 12:27 PM, 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. >> > In addition to the design question about whether NBD exports should be > their own new BB, > >> @@ -198,8 +207,12 @@ void blk_ref(BlockBackend *blk) >> * If this drops it to zero, destroy @blk. >> * For convenience, do nothing if @blk is null. >> */ >> -void blk_unref(BlockBackend *blk) >> +void blk_unref(BlockBackend *blk, Notifier *close_all_notifier) >> { >> + if (close_all_notifier) { >> + notifier_remove(close_all_notifier); >> + } >> + >> if (blk) { > Does removing a notifier when blk is NULL make sense? Considering the pattern is probably "allocate notifier; if that fails, goto fail; create BB; if that fails, goto fail; ...; fail: blk_unref(); free notifier", the is indeed a case where the notifier may be non-NULL but not yet registered (if "create BB" failed). Therefore, to simplify the caller code, notifier_remove() should only be called if blk is not NULL, right. Will fix. Max