qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] qcow2: Zero-initialise first cluster for new images
@ 2013-11-26 10:48 Kevin Wolf
  2013-11-26 12:18 ` [Qemu-devel] [Qemu-stable] " Fam Zheng
  2013-11-27 11:18 ` [Qemu-devel] " Paolo Bonzini
  0 siblings, 2 replies; 4+ messages in thread
From: Kevin Wolf @ 2013-11-26 10:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, qemu-stable, stefanha

Strictly speaking, this is only required for has_zero_init() == false,
but it's easy enough to just do a cluster-aligned write that is padded
with zeros after the header.

This fixes that after 'qemu-img create' header extensions are attempted
to be parsed that are really just random leftover data.

Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 6e5d98d..7c18587 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1471,7 +1471,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
      * size for any qcow2 image.
      */
     BlockDriverState* bs;
-    QCowHeader header;
+    QCowHeader *header;
     uint8_t* refcount_table;
     Error *local_err = NULL;
     int ret;
@@ -1489,30 +1489,35 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     }
 
     /* Write the header */
-    memset(&header, 0, sizeof(header));
-    header.magic = cpu_to_be32(QCOW_MAGIC);
-    header.version = cpu_to_be32(version);
-    header.cluster_bits = cpu_to_be32(cluster_bits);
-    header.size = cpu_to_be64(0);
-    header.l1_table_offset = cpu_to_be64(0);
-    header.l1_size = cpu_to_be32(0);
-    header.refcount_table_offset = cpu_to_be64(cluster_size);
-    header.refcount_table_clusters = cpu_to_be32(1);
-    header.refcount_order = cpu_to_be32(3 + REFCOUNT_SHIFT);
-    header.header_length = cpu_to_be32(sizeof(header));
+    QEMU_BUILD_BUG_ON((1 << MIN_CLUSTER_BITS) < sizeof(*header));
+    header = g_malloc(cluster_size);
+    memset(header, 0, cluster_size);
+    *header = (QCowHeader) {
+        .magic                      = cpu_to_be32(QCOW_MAGIC),
+        .version                    = cpu_to_be32(version),
+        .cluster_bits               = cpu_to_be32(cluster_bits),
+        .size                       = cpu_to_be64(0),
+        .l1_table_offset            = cpu_to_be64(0),
+        .l1_size                    = cpu_to_be32(0),
+        .refcount_table_offset      = cpu_to_be64(cluster_size),
+        .refcount_table_clusters    = cpu_to_be32(1),
+        .refcount_order             = cpu_to_be32(3 + REFCOUNT_SHIFT),
+        .header_length              = cpu_to_be32(sizeof(*header)),
+    };
 
     if (flags & BLOCK_FLAG_ENCRYPT) {
-        header.crypt_method = cpu_to_be32(QCOW_CRYPT_AES);
+        header->crypt_method = cpu_to_be32(QCOW_CRYPT_AES);
     } else {
-        header.crypt_method = cpu_to_be32(QCOW_CRYPT_NONE);
+        header->crypt_method = cpu_to_be32(QCOW_CRYPT_NONE);
     }
 
     if (flags & BLOCK_FLAG_LAZY_REFCOUNTS) {
-        header.compatible_features |=
+        header->compatible_features |=
             cpu_to_be64(QCOW2_COMPAT_LAZY_REFCOUNTS);
     }
 
-    ret = bdrv_pwrite(bs, 0, &header, sizeof(header));
+    ret = bdrv_pwrite(bs, 0, header, cluster_size);
+    g_free(header);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not write qcow2 header");
         goto out;
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [Qemu-stable] [PATCH] qcow2: Zero-initialise first cluster for new images
  2013-11-26 10:48 [Qemu-devel] [PATCH] qcow2: Zero-initialise first cluster for new images Kevin Wolf
@ 2013-11-26 12:18 ` Fam Zheng
  2013-11-27 12:44   ` Kevin Wolf
  2013-11-27 11:18 ` [Qemu-devel] " Paolo Bonzini
  1 sibling, 1 reply; 4+ messages in thread
From: Fam Zheng @ 2013-11-26 12:18 UTC (permalink / raw)
  To: Kevin Wolf, qemu-devel; +Cc: qemu-stable, stefanha

On 2013年11月26日 18:48, Kevin Wolf wrote:
> Strictly speaking, this is only required for has_zero_init() == false,
> but it's easy enough to just do a cluster-aligned write that is padded
> with zeros after the header.
>
> This fixes that after 'qemu-img create' header extensions are attempted
> to be parsed that are really just random leftover data.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block/qcow2.c | 37 +++++++++++++++++++++----------------
>   1 file changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 6e5d98d..7c18587 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1471,7 +1471,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
>        * size for any qcow2 image.
>        */
>       BlockDriverState* bs;
> -    QCowHeader header;
> +    QCowHeader *header;
>       uint8_t* refcount_table;
>       Error *local_err = NULL;
>       int ret;
> @@ -1489,30 +1489,35 @@ static int qcow2_create2(const char *filename, int64_t total_size,
>       }
>
>       /* Write the header */
> -    memset(&header, 0, sizeof(header));
> -    header.magic = cpu_to_be32(QCOW_MAGIC);
> -    header.version = cpu_to_be32(version);
> -    header.cluster_bits = cpu_to_be32(cluster_bits);
> -    header.size = cpu_to_be64(0);
> -    header.l1_table_offset = cpu_to_be64(0);
> -    header.l1_size = cpu_to_be32(0);
> -    header.refcount_table_offset = cpu_to_be64(cluster_size);
> -    header.refcount_table_clusters = cpu_to_be32(1);
> -    header.refcount_order = cpu_to_be32(3 + REFCOUNT_SHIFT);
> -    header.header_length = cpu_to_be32(sizeof(header));
> +    QEMU_BUILD_BUG_ON((1 << MIN_CLUSTER_BITS) < sizeof(*header));
> +    header = g_malloc(cluster_size);
> +    memset(header, 0, cluster_size);

Could just be:

     header = g_malloc0(cluster_size);

But either way,

Reviewed-by: Fam Zheng <famz@redhat.com>

> +    *header = (QCowHeader) {
> +        .magic                      = cpu_to_be32(QCOW_MAGIC),
> +        .version                    = cpu_to_be32(version),
> +        .cluster_bits               = cpu_to_be32(cluster_bits),
> +        .size                       = cpu_to_be64(0),
> +        .l1_table_offset            = cpu_to_be64(0),
> +        .l1_size                    = cpu_to_be32(0),
> +        .refcount_table_offset      = cpu_to_be64(cluster_size),
> +        .refcount_table_clusters    = cpu_to_be32(1),
> +        .refcount_order             = cpu_to_be32(3 + REFCOUNT_SHIFT),
> +        .header_length              = cpu_to_be32(sizeof(*header)),
> +    };
>
>       if (flags & BLOCK_FLAG_ENCRYPT) {
> -        header.crypt_method = cpu_to_be32(QCOW_CRYPT_AES);
> +        header->crypt_method = cpu_to_be32(QCOW_CRYPT_AES);
>       } else {
> -        header.crypt_method = cpu_to_be32(QCOW_CRYPT_NONE);
> +        header->crypt_method = cpu_to_be32(QCOW_CRYPT_NONE);
>       }
>
>       if (flags & BLOCK_FLAG_LAZY_REFCOUNTS) {
> -        header.compatible_features |=
> +        header->compatible_features |=
>               cpu_to_be64(QCOW2_COMPAT_LAZY_REFCOUNTS);
>       }
>
> -    ret = bdrv_pwrite(bs, 0, &header, sizeof(header));
> +    ret = bdrv_pwrite(bs, 0, header, cluster_size);
> +    g_free(header);
>       if (ret < 0) {
>           error_setg_errno(errp, -ret, "Could not write qcow2 header");
>           goto out;
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] qcow2: Zero-initialise first cluster for new images
  2013-11-26 10:48 [Qemu-devel] [PATCH] qcow2: Zero-initialise first cluster for new images Kevin Wolf
  2013-11-26 12:18 ` [Qemu-devel] [Qemu-stable] " Fam Zheng
@ 2013-11-27 11:18 ` Paolo Bonzini
  1 sibling, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2013-11-27 11:18 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-devel, stefanha, qemu-stable

Il 26/11/2013 11:48, Kevin Wolf ha scritto:
> Strictly speaking, this is only required for has_zero_init() == false,
> but it's easy enough to just do a cluster-aligned write that is padded
> with zeros after the header.
> 
> This fixes that after 'qemu-img create' header extensions are attempted
> to be parsed that are really just random leftover data.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

> ---
>  block/qcow2.c | 37 +++++++++++++++++++++----------------
>  1 file changed, 21 insertions(+), 16 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 6e5d98d..7c18587 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1471,7 +1471,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
>       * size for any qcow2 image.
>       */
>      BlockDriverState* bs;
> -    QCowHeader header;
> +    QCowHeader *header;
>      uint8_t* refcount_table;
>      Error *local_err = NULL;
>      int ret;
> @@ -1489,30 +1489,35 @@ static int qcow2_create2(const char *filename, int64_t total_size,
>      }
>  
>      /* Write the header */
> -    memset(&header, 0, sizeof(header));
> -    header.magic = cpu_to_be32(QCOW_MAGIC);
> -    header.version = cpu_to_be32(version);
> -    header.cluster_bits = cpu_to_be32(cluster_bits);
> -    header.size = cpu_to_be64(0);
> -    header.l1_table_offset = cpu_to_be64(0);
> -    header.l1_size = cpu_to_be32(0);
> -    header.refcount_table_offset = cpu_to_be64(cluster_size);
> -    header.refcount_table_clusters = cpu_to_be32(1);
> -    header.refcount_order = cpu_to_be32(3 + REFCOUNT_SHIFT);
> -    header.header_length = cpu_to_be32(sizeof(header));
> +    QEMU_BUILD_BUG_ON((1 << MIN_CLUSTER_BITS) < sizeof(*header));
> +    header = g_malloc(cluster_size);
> +    memset(header, 0, cluster_size);
> +    *header = (QCowHeader) {
> +        .magic                      = cpu_to_be32(QCOW_MAGIC),
> +        .version                    = cpu_to_be32(version),
> +        .cluster_bits               = cpu_to_be32(cluster_bits),
> +        .size                       = cpu_to_be64(0),
> +        .l1_table_offset            = cpu_to_be64(0),
> +        .l1_size                    = cpu_to_be32(0),
> +        .refcount_table_offset      = cpu_to_be64(cluster_size),
> +        .refcount_table_clusters    = cpu_to_be32(1),
> +        .refcount_order             = cpu_to_be32(3 + REFCOUNT_SHIFT),
> +        .header_length              = cpu_to_be32(sizeof(*header)),
> +    };
>  
>      if (flags & BLOCK_FLAG_ENCRYPT) {
> -        header.crypt_method = cpu_to_be32(QCOW_CRYPT_AES);
> +        header->crypt_method = cpu_to_be32(QCOW_CRYPT_AES);
>      } else {
> -        header.crypt_method = cpu_to_be32(QCOW_CRYPT_NONE);
> +        header->crypt_method = cpu_to_be32(QCOW_CRYPT_NONE);
>      }
>  
>      if (flags & BLOCK_FLAG_LAZY_REFCOUNTS) {
> -        header.compatible_features |=
> +        header->compatible_features |=
>              cpu_to_be64(QCOW2_COMPAT_LAZY_REFCOUNTS);
>      }
>  
> -    ret = bdrv_pwrite(bs, 0, &header, sizeof(header));
> +    ret = bdrv_pwrite(bs, 0, header, cluster_size);
> +    g_free(header);
>      if (ret < 0) {
>          error_setg_errno(errp, -ret, "Could not write qcow2 header");
>          goto out;
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [Qemu-stable] [PATCH] qcow2: Zero-initialise first cluster for new images
  2013-11-26 12:18 ` [Qemu-devel] [Qemu-stable] " Fam Zheng
@ 2013-11-27 12:44   ` Kevin Wolf
  0 siblings, 0 replies; 4+ messages in thread
From: Kevin Wolf @ 2013-11-27 12:44 UTC (permalink / raw)
  To: Fam Zheng; +Cc: qemu-devel, stefanha, qemu-stable

Am 26.11.2013 um 13:18 hat Fam Zheng geschrieben:
> On 2013年11月26日 18:48, Kevin Wolf wrote:
> >Strictly speaking, this is only required for has_zero_init() == false,
> >but it's easy enough to just do a cluster-aligned write that is padded
> >with zeros after the header.
> >
> >This fixes that after 'qemu-img create' header extensions are attempted
> >to be parsed that are really just random leftover data.
> >
> >Cc: qemu-stable@nongnu.org
> >Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> >---
> >  block/qcow2.c | 37 +++++++++++++++++++++----------------
> >  1 file changed, 21 insertions(+), 16 deletions(-)
> >
> >diff --git a/block/qcow2.c b/block/qcow2.c
> >index 6e5d98d..7c18587 100644
> >--- a/block/qcow2.c
> >+++ b/block/qcow2.c
> >@@ -1471,7 +1471,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
> >       * size for any qcow2 image.
> >       */
> >      BlockDriverState* bs;
> >-    QCowHeader header;
> >+    QCowHeader *header;
> >      uint8_t* refcount_table;
> >      Error *local_err = NULL;
> >      int ret;
> >@@ -1489,30 +1489,35 @@ static int qcow2_create2(const char *filename, int64_t total_size,
> >      }
> >
> >      /* Write the header */
> >-    memset(&header, 0, sizeof(header));
> >-    header.magic = cpu_to_be32(QCOW_MAGIC);
> >-    header.version = cpu_to_be32(version);
> >-    header.cluster_bits = cpu_to_be32(cluster_bits);
> >-    header.size = cpu_to_be64(0);
> >-    header.l1_table_offset = cpu_to_be64(0);
> >-    header.l1_size = cpu_to_be32(0);
> >-    header.refcount_table_offset = cpu_to_be64(cluster_size);
> >-    header.refcount_table_clusters = cpu_to_be32(1);
> >-    header.refcount_order = cpu_to_be32(3 + REFCOUNT_SHIFT);
> >-    header.header_length = cpu_to_be32(sizeof(header));
> >+    QEMU_BUILD_BUG_ON((1 << MIN_CLUSTER_BITS) < sizeof(*header));
> >+    header = g_malloc(cluster_size);
> >+    memset(header, 0, cluster_size);
> 
> Could just be:
> 
>     header = g_malloc0(cluster_size);
> 
> But either way,

Yes, that's better. I'll change it.

> Reviewed-by: Fam Zheng <famz@redhat.com>

Thanks.

Kevin

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-11-27 12:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-26 10:48 [Qemu-devel] [PATCH] qcow2: Zero-initialise first cluster for new images Kevin Wolf
2013-11-26 12:18 ` [Qemu-devel] [Qemu-stable] " Fam Zheng
2013-11-27 12:44   ` Kevin Wolf
2013-11-27 11:18 ` [Qemu-devel] " Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).