Re: [Qemu-stable] [PATCH] qcow2: Zero-initialise first cluster for new images

[Kevin Wolf]

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: address@hidden
> >Signed-off-by: Kevin Wolf <address@hidden>
> >---
> >  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 <address@hidden>

Thanks.

Kevin