| Message ID | 20160628014747.20971-1-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue 28 Jun 2016 03:47:47 AM CEST, Fam Zheng <famz@redhat.com> wrote: > 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 <famz@redhat.com> Oh, so we didn't see this in e94867e :-) Reviewed-by: Alberto Garcia <berto@igalia.com> Berto
On 28.06.2016 03:47, 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. This sounds like this patch specifically wants to drop the check from bdrv_quorum_init(). I think keeping the check would be better; while we can probably assume that SHA256 is supported if CONFIG_GNUTLS_HASH is defined, it would still technically be better to actually check for that support. However, since this is probably only a theoretical issue, if there's a good reason (like preparation for dynamic block driver modules) for having to drop the check from bdrv_quorum_init(), I think it's alright to do so. Max > 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 <famz@redhat.com> > --- > 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); > } > >
On Sat 02 Jul 2016 02:36:38 PM CEST, Max Reitz 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. > > This sounds like this patch specifically wants to drop the check from > bdrv_quorum_init(). > > I think keeping the check would be better; while we can probably > assume that SHA256 is supported if CONFIG_GNUTLS_HASH is defined, it > would still technically be better to actually check for that support. Right, but SHA256 was added to GNUTLS almost 10 years ago, so I don't think that adding such check has any practical effect. Berto
On Sat, 07/02 14:36, Max Reitz wrote: > On 28.06.2016 03:47, 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. > > This sounds like this patch specifically wants to drop the check from > bdrv_quorum_init(). > > I think keeping the check would be better; while we can probably assume > that SHA256 is supported if CONFIG_GNUTLS_HASH is defined, it would > still technically be better to actually check for that support. This one is effectively a static check so I think it's practically okay to skip it, and yes, the idea is this makes it easier to do dynamic driver loading. Thanks, Fam > > However, since this is probably only a theoretical issue, if there's a > good reason (like preparation for dynamic block driver modules) for > having to drop the check from bdrv_quorum_init(), I think it's alright > to do so. > > Max > > > 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 <famz@redhat.com> > > --- > > 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); > > } > > > > > >
Dear Fam (or Zheng?), Fam Zheng <famz@redhat.com> writes: > 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. Good idea. [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/quorum.c] > 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); The quorum driver needs SHA256 which was introduced in gnutls 2.11.1. However configure sets CONFIG_GNUTLS_HASH when gnutls 2.9.10+ is present. You should either bump the version in configure or add an explicit configure check for SHA256. Sascha
On Tue, 07/05 09:58, Sascha Silbe wrote: > Dear Fam (or Zheng?), Hi Sascha, Zheng is the last name here. :) > > Fam Zheng <famz@redhat.com> writes: > > > 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. > > Good idea. > > > [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/quorum.c] > > 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); > > The quorum driver needs SHA256 which was introduced in gnutls > 2.11.1. However configure sets CONFIG_GNUTLS_HASH when gnutls 2.9.10+ is > present. You should either bump the version in configure or add an > explicit configure check for SHA256. Yes, I just noticed commit 0c16c056a4f removed CONFIG_GNUTLS_HASH so I need to rebase anyway (that commit also fixed this version requirement we have been missing as you mentioned). Thanks for reviewing! Fam
On Tue 05 Jul 2016 09:58:25 AM CEST, Sascha Silbe wrote:
> The quorum driver needs SHA256 which was introduced in gnutls 2.11.1.
Are you sure about that?
* Version 1.7.4 (released 2007-02-05)
[...]
** API and ABI modifications:
GNUTLS_MAC_SHA256,
GNUTLS_MAC_SHA384,
GNUTLS_MAC_SHA512: New gnutls_mac_algorithm_t values.
GNUTLS_DIG_SHA256,
GNUTLS_DIG_SHA384,
GNUTLS_DIG_SHA512: New gnutls_digest_algorithm_t values.
GNUTLS_SIGN_RSA_SHA256,
GNUTLS_SIGN_RSA_SHA384,
GNUTLS_SIGN_RSA_SHA512: New gnutls_sign_algorithm_t values.
https://gitlab.com/gnutls/gnutls/blob/master/NEWS#L6570
Berto
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 <famz@redhat.com> 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 > --- > 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
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 <famz@redhat.com> > > 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 :|
Dear Alberto, Alberto Garcia <berto@igalia.com> writes: > On Tue 05 Jul 2016 09:58:25 AM CEST, Sascha Silbe wrote: > >> The quorum driver needs SHA256 which was introduced in gnutls 2.11.1. > > Are you sure about that? > > * Version 1.7.4 (released 2007-02-05) > > [...] > > ** API and ABI modifications: > GNUTLS_MAC_SHA256, [...] You are right, I misinterpreted the gnutls 2.11.1 NEWS entry. It added the RSA_NULL_SHA256 cipher suite, not the SHA256 hash algorithm. So the existing configure check is good enough, no need to bump the required gnutls version. Sascha
On Tue 05 Jul 2016 10:45:21 AM CEST, Daniel P. Berrange wrote: > 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 Now that you mention this I wonder why we are not using glib for the hashing functions. GChecksum is available since glib 2.16 (QEMU requires 2.22) and it supports MD5, SHA1, SHA256 and SHA512. I see that in git master there's now a few algorithms more, but for the Quorum case those ones are enough. Berto
On Tue, Jul 05, 2016 at 11:18:29AM +0200, Alberto Garcia wrote: > On Tue 05 Jul 2016 10:45:21 AM CEST, Daniel P. Berrange wrote: > > > 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 > > Now that you mention this I wonder why we are not using glib for the > hashing functions. GChecksum is available since glib 2.16 (QEMU requires > 2.22) and it supports MD5, SHA1, SHA256 and SHA512. I see that in git > master there's now a few algorithms more, but for the Quorum case those > ones are enough. The GChecksum API is inadequate for QEMU's needs, due to its limited range of algorithms. We absolutely do not want different areas of the code using different APIs either. The goal of the crypto APIs is to provide a standard internal API for all cryptographic related operations for use across the whole codebase. This has clarified much of our code by removing countless #ifdef conditionals from the code and similar from the build system. It also facilitates people auditing QEMU use & implementation of crypto as there is only one place to look at to review. It also ensures that QEMU is only using certified secure crypto libraries, not some custom re-implementation of the crypto algorithms that have never been through a security review. Finally is ensures that QEMU correctly responds to runtime configurable changes, such as FIPS mode which restricts use of certain crypto algorithms at runtime, even if they're technically available at compile time. Regards, Daniel
On Tue, Jul 05, 2016 at 04:57:58PM +0800, Fam Zheng wrote: > 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 <famz@redhat.com> > > > > 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? While the crypto algorithms may all be built-in at compile time, there can be situations where they are forbidden from use at runtime. For example the FIPS mode activation will disable many algorithms from use. You might say that this doesn't matter since FIPs mode dosn't affect SHA256, but the point of having the generic crypto API inside QEMU is to stop us sprinkling these kind of ad-hoc assumptions about crypto policies across the code base. Can you backup and explain more detail what the actual problem you're trying to solve is. IIUC, it is related to module loading, but I'm not seeing exactly what it is. Surely when we load the quorum.so module, we'll just invoke the bdrv_quorum_init() method as normal, so I would have expected the current logic to continue to "just work". ie, just because we load a module, does not mean that module should be required to register its block driver. The other alternative though is to simply remove the hash check from the init method *and* unconditionally compile it, and simply allow the quorum_open() method do the qcrypto_hash_supports() check. This would be the same way that the LUKS block driver works - it has many crypto algorithms in use, chosen dynamically, so it has no choice but to test this at open() time. > > > 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); > > > } Regards, Daniel
On Tue, Jul 05, 2016 at 10:26:56AM +0100, Daniel P. Berrange wrote: > On Tue, Jul 05, 2016 at 11:18:29AM +0200, Alberto Garcia wrote: > > On Tue 05 Jul 2016 10:45:21 AM CEST, Daniel P. Berrange wrote: > > > > > 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 > > > > Now that you mention this I wonder why we are not using glib for the > > hashing functions. GChecksum is available since glib 2.16 (QEMU requires > > 2.22) and it supports MD5, SHA1, SHA256 and SHA512. I see that in git > > master there's now a few algorithms more, but for the Quorum case those > > ones are enough. > > The GChecksum API is inadequate for QEMU's needs, due to its limited > range of algorithms. We absolutely do not want different areas of > the code using different APIs either. The goal of the crypto APIs is > to provide a standard internal API for all cryptographic related > operations for use across the whole codebase. This has clarified much > of our code by removing countless #ifdef conditionals from the code > and similar from the build system. It also facilitates people auditing > QEMU use & implementation of crypto as there is only one place to look > at to review. It also ensures that QEMU is only using certified secure > crypto libraries, not some custom re-implementation of the crypto > algorithms that have never been through a security review. Finally is > ensures that QEMU correctly responds to runtime configurable changes, > such as FIPS mode which restricts use of certain crypto algorithms > at runtime, even if they're technically available at compile time. Acutally, having said that, what we could do is to replace the no-op stub hash impl, with a GCheckusum based impl. That way if neither gcrypt or nettle are available, we can fallback to GChecksum for a sub-set of the hash algorithms. That would allow us to move the gcrypto_hash_supports() check out of the quorum register method, and into its open() method, and avoid any Makefile.objs conditionals. Regards, Daniel
On Tue, 07/05 10:35, Daniel P. Berrange wrote: > > Can you backup and explain more detail what the actual problem you're trying > to solve is. IIUC, it is related to module loading, but I'm not seeing exactly > what it is. This patch originated when I was researching all drivers' block_init functions about module loading, and realized this is the only one "runtime check" and is poorly established as it's actually "static". The unnecessity is more of the reason behind the patch. Its relationship with module loading is, it makes implementing this idea easier: https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg07580.html > Surely when we load the quorum.so module, we'll just invoke the > bdrv_quorum_init() method as normal, so I would have expected the current > logic to continue to "just work". ie, just because we load a module, does > not mean that module should be required to register its block driver. You are right, module init function can do anything it wants, as long as we really need it to. It's just not clear to me quorum is the case. BTW thanks for sending the reverting series! > > The other alternative though is to simply remove the hash check from the > init method *and* unconditionally compile it, and simply allow the > quorum_open() method do the qcrypto_hash_supports() check. This would > be the same way that the LUKS block driver works - it has many crypto > algorithms in use, chosen dynamically, so it has no choice but to test > this at open() time. This sounds good too. Fam > > > > > 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); > > > > } > > 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 :|
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); }
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 <famz@redhat.com> --- block/Makefile.objs | 2 +- block/quorum.c | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-)