On 01/17/2013 08:51 AM, Benoît Canet wrote:
> Signed-off-by: Benoit Canet <benoit@irqsave.net>> ---> block/quorum.c | 278 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-> 1 file changed, 277 insertions(+), 1 deletion(-)> > diff --git a/block/quorum.c b/block/quorum.c> index 98052eb..2bffff4 100644> --- a/block/quorum.c> +++ b/block/quorum.c> @@ -14,6 +14,20 @@> */> > #include "block/block_int.h"> +#include "zlib.h"
Since zlib.h isn't part of qemu.git, this should be <zlib.h>.
> +> +static unsigned long quorum_compute_checksum(QuorumAIOCB *acb, int i)> +{> + int j;> + unsigned long adler = adler32(0L, Z_NULL, 0);
Hmm, adler32() is basically a weak hashing mechanism; are you sure you
won't have any false collisions? Furthermore, how does this compare
with the series for adding deduplication, which uses much
stronger/longer hashes, but where those take more time to compute? Is
there any way you can share efforts between the two series?
> + /* get the index of the first successfull read */
s/successfull/successful/ (several times in this patch)

> Hmm, adler32() is basically a weak hashing mechanism; are you sure you> won't have any false collisions? Furthermore, how does this compare> with the series for adding deduplication, which uses much> stronger/longer hashes, but where those take more time to compute? Is> there any way you can share efforts between the two series?
I agree that using sha256 would be a better option as it will not be called
frequently.
However the call to gnutls is a one liner so it's likely not shareable.
The configure would need to be modified to require gnutls and It likely not
factorisable either.
I'll change the code to use sha256.
Regards
Benoît
> > > + /* get the index of the first successfull read */> > s/successfull/successful/ (several times in this patch)> > -- > Eric Blake eblake redhat com +1-919-301-3266> Libvirt virtualization library http://libvirt.org>