Re: [Qemu-block] [Qemu-devel] [PATCH] quorum: Only compile when supported

[Fam Zheng]

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 :|