diff mbox

[RFC,V4,12/30] qcow2: Load and save deduplication table header extension.

Message ID 1357143393-29832-13-git-send-email-benoit@irqsave.net
State New
Headers show

Commit Message

Benoît Canet Jan. 2, 2013, 4:16 p.m. UTC 
Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 block/qcow2.c |   38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

Comments

Eric Blake Jan. 5, 2013, 12:02 a.m. UTC | #1 
On 01/02/2013 09:16 AM, Benoît Canet wrote:
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
>  block/qcow2.c |   38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 410d3c1..9a7177b 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -53,9 +53,16 @@ typedef struct {
>      uint32_t len;
>  } QCowExtension;
>  
> +typedef struct {
> +    uint64_t offset;
> +    int32_t  size;
> +    uint8_t  hash_algo;
> +} QCowDedupTableExtension;

This struct has a hole at the end (that is, you only specify 13 bytes,
but sizeof(QCowDedupTableExtension) is 16)...

> +    if (s->has_dedup) {
> +        dedup_table_extension.offset = cpu_to_be64(s->dedup_table_offset);
> +        dedup_table_extension.size = cpu_to_be32(s->dedup_table_size);
> +        dedup_table_extension.hash_algo = s->dedup_hash_algo;
> +        ret = header_ext_add(buf,
> +                             QCOW2_EXT_MAGIC_DEDUP_TABLE,
> +                             &dedup_table_extension,
> +                             sizeof(dedup_table_extension),
> +                             buflen);

...but here you are writing out that hole.  It would be better if you
explicitly accounted for all bytes being written, so that the reserved
fields are guaranteed to be 0 instead of random data, so that future
extensions can make use of those reserved bytes.
diff mbox

Patch

diff --git a/block/qcow2.c b/block/qcow2.c
index 410d3c1..9a7177b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -53,9 +53,16 @@  typedef struct {
     uint32_t len;
 } QCowExtension;
 
+typedef struct {
+    uint64_t offset;
+    int32_t  size;
+    uint8_t  hash_algo;
+} QCowDedupTableExtension;
+
 #define  QCOW2_EXT_MAGIC_END 0
 #define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
 #define  QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
+#define  QCOW2_EXT_MAGIC_DEDUP_TABLE 0xCD8E819B
 
 static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
 {
@@ -83,6 +90,7 @@  static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
     QCowExtension ext;
     uint64_t offset;
     int ret;
+    QCowDedupTableExtension dedup_table_extension;
 
 #ifdef DEBUG_EXT
     printf("qcow2_read_extensions: start=%ld end=%ld\n", start_offset, end_offset);
@@ -147,6 +155,19 @@  static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
             }
             break;
 
+        case QCOW2_EXT_MAGIC_DEDUP_TABLE:
+                ret = bdrv_pread(bs->file, offset,
+                                 &dedup_table_extension, ext.len);
+                if (ret < 0) {
+                    return ret;
+                }
+                s->dedup_table_offset =
+                    be64_to_cpu(dedup_table_extension.offset);
+                s->dedup_table_size =
+                    be32_to_cpu(dedup_table_extension.size);
+                s->dedup_hash_algo = dedup_table_extension.hash_algo;
+            break;
+
         default:
             /* unknown magic - save it in case we need to rewrite the header */
             {
@@ -958,6 +979,7 @@  int qcow2_update_header(BlockDriverState *bs)
     uint32_t refcount_table_clusters;
     size_t header_length;
     Qcow2UnknownHeaderExtension *uext;
+    QCowDedupTableExtension dedup_table_extension;
 
     buf = qemu_blockalign(bs, buflen);
 
@@ -1061,6 +1083,22 @@  int qcow2_update_header(BlockDriverState *bs)
     buf += ret;
     buflen -= ret;
 
+    if (s->has_dedup) {
+        dedup_table_extension.offset = cpu_to_be64(s->dedup_table_offset);
+        dedup_table_extension.size = cpu_to_be32(s->dedup_table_size);
+        dedup_table_extension.hash_algo = s->dedup_hash_algo;
+        ret = header_ext_add(buf,
+                             QCOW2_EXT_MAGIC_DEDUP_TABLE,
+                             &dedup_table_extension,
+                             sizeof(dedup_table_extension),
+                             buflen);
+        if (ret < 0) {
+            goto fail;
+        }
+        buf += ret;
+        buflen -= ret;
+    }
+
     /* Keep unknown header extensions */
     QLIST_FOREACH(uext, &s->unknown_header_ext, next) {
         ret = header_ext_add(buf, uext->magic, uext->data, uext->len, buflen);