From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47669) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bKMBD-0002aA-0J for qemu-devel@nongnu.org; Tue, 05 Jul 2016 04:58:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bKMBA-0008Ky-Of for qemu-devel@nongnu.org; Tue, 05 Jul 2016 04:58:06 -0400 Date: Tue, 5 Jul 2016 16:57:58 +0800 From: Fam Zheng Message-ID: <20160705085758.GE29307@ad.usersys.redhat.com> References: <20160628014747.20971-1-famz@redhat.com> <20160705084521.GD6553@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160705084521.GD6553@redhat.com> Subject: Re: [Qemu-devel] [PATCH] quorum: Only compile when supported List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: qemu-devel@nongnu.org, Kevin Wolf , Alberto Garcia , qemu-block@nongnu.org, Max Reitz On Tue, 07/05 09:45, Daniel P. Berrange wrote: > On Tue, Jun 28, 2016 at 09:47:47AM +0800, Fam Zheng wrote: > > This was the only exceptional module init function that does something > > else than a simple list of bdrv_register() calls, in all the block > > drivers. > > > > The qcrypto_hash_supports is actually a static check, determined at > > compile time. Follow the block-job-$(CONFIG_FOO) convention for > > consistency. > > > > Signed-off-by: Fam Zheng > > The point of using qcrypto_hash_supports() is that it isolates the > block code Makefile rules from the details of the current specific > impl of the hash APIs in QEMU. As a prime example of why this is > important, try rebasing to GIT master, and you'll find we no longer > use gnutls for the hash APIs. We choose between libgcrypt, nettle > or a empty stub for hash impls now. I think it is a backwards step > to add back these makefile conditionals I don't want to spoil the isolation, but I think it's also worth to keep the reasoning of whether a driver is supported as simple as possible. In other words, if it's determined at configure time, there is no reason to use a runtime check, right? Fam > > > --- > > block/Makefile.objs | 2 +- > > block/quorum.c | 4 ---- > > 2 files changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/block/Makefile.objs b/block/Makefile.objs > > index 44a5416..c87d605 100644 > > --- a/block/Makefile.objs > > +++ b/block/Makefile.objs > > @@ -3,7 +3,7 @@ block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-c > > block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o > > block-obj-y += qed-check.o > > block-obj-$(CONFIG_VHDX) += vhdx.o vhdx-endian.o vhdx-log.o > > -block-obj-y += quorum.o > > +block-obj-$(CONFIG_GNUTLS_HASH) += quorum.o > > block-obj-y += parallels.o blkdebug.o blkverify.o blkreplay.o > > block-obj-y += block-backend.o snapshot.o qapi.o > > block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o > > diff --git a/block/quorum.c b/block/quorum.c > > index 331b726..18fbed8 100644 > > --- a/block/quorum.c > > +++ b/block/quorum.c > > @@ -1113,10 +1113,6 @@ static BlockDriver bdrv_quorum = { > > > > static void bdrv_quorum_init(void) > > { > > - if (!qcrypto_hash_supports(QCRYPTO_HASH_ALG_SHA256)) { > > - /* SHA256 hash support is required for quorum device */ > > - return; > > - } > > bdrv_register(&bdrv_quorum); > > } > > > > -- > > 2.9.0 > > > > > > Regards, > Daniel > -- > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|