Re: [Qemu-devel] [PATCH 07/10] block: convert quorum blockdrv to use crypto APIs

["Daniel P. Berrange" <berrange@redhat.com>]

On Fri, May 29, 2015 at 02:49:46PM +0800, Gonglei wrote:
> On 2015/5/21 18:56, Daniel P. Berrange wrote:
> > Get rid of direct use of gnutls APIs in quorum blockdrv in
> > favour of using the crypto APIs. This avoids the need to
> > do conditional compilation of the quorum driver. It can
> > simply report an error at file open file instead if the
> > required hash algorithm isn't supported by QEMU.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  block/Makefile.objs |  2 +-
> >  block/quorum.c      | 38 +++++++++++++++++++-------------------
> >  configure           | 39 ---------------------------------------
> >  3 files changed, 20 insertions(+), 59 deletions(-)
> > 
> > diff --git a/block/Makefile.objs b/block/Makefile.objs
> > index 0d8c2a4..8f908d9 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-$(CONFIG_QUORUM) += quorum.o
> > +block-obj-y += quorum.o
> >  block-obj-y += parallels.o blkdebug.o blkverify.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 f91ef75..4193f30 100644
> > --- a/block/quorum.c
> > +++ b/block/quorum.c
> > @@ -13,8 +13,6 @@
> >   * See the COPYING file in the top-level directory.
> >   */
> >  
> > -#include <gnutls/gnutls.h>
> > -#include <gnutls/crypto.h>
> >  #include "block/block_int.h"
> >  #include "qapi/qmp/qbool.h"
> >  #include "qapi/qmp/qdict.h"
> > @@ -23,6 +21,9 @@
> >  #include "qapi/qmp/qlist.h"
> >  #include "qapi/qmp/qstring.h"
> >  #include "qapi-event.h"
> > +#include "crypto/hash.h"
> > +
> > +#include <glib/gi18n.h>
> >  
> >  #define HASH_LENGTH 32
> >  
> > @@ -33,7 +34,7 @@
> >  
> >  /* This union holds a vote hash value */
> >  typedef union QuorumVoteValue {
> > -    char h[HASH_LENGTH];       /* SHA-256 hash */
> > +    uint8_t h[HASH_LENGTH];    /* SHA-256 hash */
> >      int64_t l;                 /* simpler 64 bits hash */
> >  } QuorumVoteValue;
> >  
> > @@ -427,25 +428,18 @@ static void quorum_free_vote_list(QuorumVotes *votes)
> >  
> >  static int quorum_compute_hash(QuorumAIOCB *acb, int i, QuorumVoteValue *hash)
> >  {
> > -    int j, ret;
> > -    gnutls_hash_hd_t dig;
> >      QEMUIOVector *qiov = &acb->qcrs[i].qiov;
> > -
> > -    ret = gnutls_hash_init(&dig, GNUTLS_DIG_SHA256);
> > -
> > -    if (ret < 0) {
> > -        return ret;
> > +    size_t len = sizeof(hash->h);
> > +    uint8_t *data = hash->h;
> > +
> > +    if (qcrypto_hash_bytesv(QCRYPTO_HASH_ALG_SHA256,
> > +                            qiov->iov, qiov->niov,
> > +                            &data, &len,
> > +                            NULL) < 0) {
> 
> The last argument should not be NULL, maybe miss error message.

Yes, it would be desirable to pass in an Error ** object, but this
quorum_compute_hash method doesn't currently have any error reporting
framework except for returning errno values. The quorum block driver
would need some refactoring to be able to propagate Error* objects
back up before we can make use of that here, so I just stuck with
the generic EINVAL.

> > +        return -EINVAL;
> >      }
> >  
> > -    for (j = 0; j < qiov->niov; j++) {
> > -        ret = gnutls_hash(dig, qiov->iov[j].iov_base, qiov->iov[j].iov_len);
> > -        if (ret < 0) {
> > -            break;
> > -        }
> > -    }
> > -
> > -    gnutls_hash_deinit(dig, (void *) hash);
> > -    return ret;
> > +    return 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 :|