Re: [Qemu-devel] [RFC V8 06/13] quorum: Add quorum mechanism.

[Kevin Wolf]

Am 26.09.2013 um 18:46 hat Benoît Canet geschrieben:
> Le Friday 08 Feb 2013 à 13:07:03 (+0100), Kevin Wolf a écrit :
> > Am 28.01.2013 18:07, schrieb Benoît Canet:
> > > Use gnutls's SHA-256 to compare versions.
> > > 
> > > Signed-off-by: Benoit Canet <address@hidden>
> > > ---
> > >  block/quorum.c |  303 
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > >  configure      |   22 ++++
> > >  2 files changed, 324 insertions(+), 1 deletion(-)

> > > +static bool quorum_iovec_compare(QEMUIOVector *a, QEMUIOVector *b)
> > > +{
> > > +    int i;
> > > +    int result;
> > > +
> > > +    assert(a->niov == b->niov);
> > > +    for (i = 0; i < a->niov; i++) {
> > > +        assert(a->iov[i].iov_len == b->iov[i].iov_len);
> > > +        result = memcmp(a->iov[i].iov_base,
> > > +                        b->iov[i].iov_base,
> > > +                        a->iov[i].iov_len);
> > > +        if (result) {
> > > +            return false;
> > > +        }
> > > +    }
> > > +
> > > +    return true;
> > > +}
> > 
> > qemu_iovec_compare() seems to do exactly the same, except that this
> > doesn't return the offset of the first difference.
> > 
> > Why is it a good idea to duplicate the code?
> 
> Quorum need speed: qemu_iovec_compare will do byte level comparison whereas
> memcmp will use SSE on large block.

Fair enough.

> > > +    /* we have a winner: copy it */
> > > +    quorum_copy_qiov(acb->qiov, &acb->aios[winner->index].qiov);
> > > +
> > > +    /* some versions are bad print them */
> > > +    quorum_print_bad_versions(acb, &winner->value);
> > 
> > Same here. Is this driver meant to be used in production or only for
> > debugging? Maybe it should have a debug mode that must explicitly be
> > enabled and messages are only printed in this mode?
> 
> Quorum needs to communicate some event to the user for maintainance.
> Would thowing QMP events be better ?

Yes, absolutely. Error messages on stderr end up in a log file at best
and can only be read by human administrators. Using QMP enables
management tools to take action.

Kevin