Patchwork [RFC,V3,08/24] qcow2: Implement qcow2_compute_cluster_hash.

login
register
mail settings
Submitter Benoît Canet
Date Nov. 26, 2012, 1:05 p.m.
Message ID <1353935123-24199-9-git-send-email-benoit@irqsave.net>
Download mbox | patch
Permalink /patch/201684/
State New
Headers show

Comments

Benoît Canet - Nov. 26, 2012, 1:05 p.m.
Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 Makefile            |    3 +++
 Makefile.target     |    2 +-
 block/qcow2-dedup.c |   10 ++++++++--
 3 files changed, 12 insertions(+), 3 deletions(-)
Stefan Hajnoczi - Dec. 11, 2012, 1:28 p.m.
On Mon, Nov 26, 2012 at 02:05:07PM +0100, Benoît Canet wrote:
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
>  Makefile            |    3 +++
>  Makefile.target     |    2 +-
>  block/qcow2-dedup.c |   10 ++++++++--
>  3 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 88285a4..c79b2da 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -168,6 +168,9 @@ qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y) $(qapi-obj-y) \
>                                qapi-visit.o qapi-types.o
>  qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y) $(block-obj-y)
>  qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y)
> +qemu-img$(EXESUF): LIBS+=-lcrypto
> +qemu-nbd$(EXESUF): LIBS+=-lcrypto
> +qemu-io$(EXESUF): LIBS+=-lcrypto
>  
>  qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
>  
> diff --git a/Makefile.target b/Makefile.target
> index 3822bc5..f9a988a 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -119,7 +119,7 @@ obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
>  obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o
>  obj-$(CONFIG_NO_GET_MEMORY_MAPPING) += memory_mapping-stub.o
>  obj-$(CONFIG_NO_CORE_DUMP) += dump-stub.o
> -LIBS+=-lz
> +LIBS+=-lz -lcrypto

Need a ./configure check for openssl?

VNC can already use gnutls so perhaps we should support that?
http://gnutls.org/manual/gnutls.html#Hash-and-HMAC-functions

>  
>  QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
>  QEMU_CFLAGS += $(VNC_SASL_CFLAGS)
> diff --git a/block/qcow2-dedup.c b/block/qcow2-dedup.c
> index 83ad61e..37e8266 100644
> --- a/block/qcow2-dedup.c
> +++ b/block/qcow2-dedup.c
> @@ -25,11 +25,13 @@
>   * THE SOFTWARE.
>   */
>  
> +#include <openssl/sha.h>
> +#include <openssl/evp.h>
>  #include "block_int.h"
>  #include "qemu-common.h"
>  #include "qcow2.h"
>  
> -#define HASH_LENGTH 32
> +#define HASH_LENGTH SHA256_DIGEST_LENGTH
>  
>  static int qcow2_dedup_read_write_hash(BlockDriverState *bs,
>                                         uint8_t **hash,
> @@ -188,7 +190,11 @@ static QCowHashNode *qcow2_dedup_build_qcow_hash_node(uint8_t *hash,
>  static uint8_t *qcow2_compute_cluster_hash(BlockDriverState *bs,
>                                             uint8_t *data)
>  {
> -    return NULL;
> +    BDRVQcowState *s = bs->opaque;
> +    uint8_t *hash = g_malloc0(HASH_LENGTH);
> +    EVP_Digest(data, s->cluster_size,
> +               hash, NULL, EVP_sha256(), NULL);
> +    return hash;
>  }

Not sure if it's worth allocating these relatively small objects on the
heap and worrying about their lifecycle.

It's simpler to pass references to the hashes and copy the entire object
to pass ownership.

This function would become:
void qcow2_compute_cluster_hash(BlockDriverState *bs, uint8_t *data, QcowHash *hash);

typedef struct {
    uint8_t data[SHA256_DIGEST_LENGTH];
} QcowHash;

The caller needs to decide whether a stack-allocated variable is
appropriate or if the hash should live inside a QcowHashNode, etc.

Stefan

Patch

diff --git a/Makefile b/Makefile
index 88285a4..c79b2da 100644
--- a/Makefile
+++ b/Makefile
@@ -168,6 +168,9 @@  qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y) $(qapi-obj-y) \
                               qapi-visit.o qapi-types.o
 qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y) $(block-obj-y)
 qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y)
+qemu-img$(EXESUF): LIBS+=-lcrypto
+qemu-nbd$(EXESUF): LIBS+=-lcrypto
+qemu-io$(EXESUF): LIBS+=-lcrypto
 
 qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
 
diff --git a/Makefile.target b/Makefile.target
index 3822bc5..f9a988a 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -119,7 +119,7 @@  obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
 obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o
 obj-$(CONFIG_NO_GET_MEMORY_MAPPING) += memory_mapping-stub.o
 obj-$(CONFIG_NO_CORE_DUMP) += dump-stub.o
-LIBS+=-lz
+LIBS+=-lz -lcrypto
 
 QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
 QEMU_CFLAGS += $(VNC_SASL_CFLAGS)
diff --git a/block/qcow2-dedup.c b/block/qcow2-dedup.c
index 83ad61e..37e8266 100644
--- a/block/qcow2-dedup.c
+++ b/block/qcow2-dedup.c
@@ -25,11 +25,13 @@ 
  * THE SOFTWARE.
  */
 
+#include <openssl/sha.h>
+#include <openssl/evp.h>
 #include "block_int.h"
 #include "qemu-common.h"
 #include "qcow2.h"
 
-#define HASH_LENGTH 32
+#define HASH_LENGTH SHA256_DIGEST_LENGTH
 
 static int qcow2_dedup_read_write_hash(BlockDriverState *bs,
                                        uint8_t **hash,
@@ -188,7 +190,11 @@  static QCowHashNode *qcow2_dedup_build_qcow_hash_node(uint8_t *hash,
 static uint8_t *qcow2_compute_cluster_hash(BlockDriverState *bs,
                                            uint8_t *data)
 {
-    return NULL;
+    BDRVQcowState *s = bs->opaque;
+    uint8_t *hash = g_malloc0(HASH_LENGTH);
+    EVP_Digest(data, s->cluster_size,
+               hash, NULL, EVP_sha256(), NULL);
+    return hash;
 }
 
 /* Try to find the offset of a given cluster if it's duplicated