[Qemu-devel] [PATCH 0/4] VHDX Fixes for zero init

[Qemu-devel] [PATCH 0/4] VHDX Fixes for zero init

[Jeff Cody]

A couple of VHDX fixes in this series:
 * updating the driver to reflect the 1.00 spec for the value of
   PAYLOAD_BLOCK_UNMAPPED (thanks Kevin)
 * enabling VHDX to support zero init in qemu-img convert

1/4: compiling: e8718ae: block: vhdx - remove redundant comments
2/4: compiling: 8a4d2da: block: vhdx - update PAYLOAD_BLOCK_UNMAPPED value to match 1.00 spec
3/4: compiling: 44f08c2: block: vhdx - change .vhdx_create default block state to ZERO
4/4: compiling: 3eb0cd2: block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

Jeff Cody (4):
  block: vhdx - remove redundant comments
  block: vhdx - update PAYLOAD_BLOCK_UNMAPPED value to match 1.00 spec
  block: vhdx - change .vhdx_create default block state to ZERO
  block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

 block/vhdx.c  | 18 +++++++++++-------
 block/vhdx.h  |  3 ++-
 qemu-doc.texi |  6 +++++-
 3 files changed, 18 insertions(+), 9 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 1/4] block: vhdx - remove redundant comments

[Jeff Cody]

Minor cleanup.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/vhdx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index 12bfe75..f1e1e2e 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1109,8 +1109,8 @@ static coroutine_fn int vhdx_co_readv(BlockDriverState *bs, int64_t sector_num,
             /* check the payload block state */
             switch (s->bat[sinfo.bat_idx] & VHDX_BAT_STATE_BIT_MASK) {
             case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
-            case PAYLOAD_BLOCK_UNDEFINED:   /* fall through */
-            case PAYLOAD_BLOCK_UNMAPPED:    /* fall through */
+            case PAYLOAD_BLOCK_UNDEFINED:
+            case PAYLOAD_BLOCK_UNMAPPED:
             case PAYLOAD_BLOCK_ZERO:
                 /* return zero */
                 qemu_iovec_memset(&hd_qiov, 0, 0, sinfo.bytes_avail);
@@ -1280,8 +1280,8 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
 
                 /* fall through */
             case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
-            case PAYLOAD_BLOCK_UNMAPPED:    /* fall through */
-            case PAYLOAD_BLOCK_UNDEFINED:   /* fall through */
+            case PAYLOAD_BLOCK_UNMAPPED:
+            case PAYLOAD_BLOCK_UNDEFINED:
                 bat_prior_offset = sinfo.file_offset;
                 ret = vhdx_allocate_block(bs, s, &sinfo.file_offset);
                 if (ret < 0) {
-- 
1.9.3

[Qemu-devel] [PATCH 2/4] block: vhdx - update PAYLOAD_BLOCK_UNMAPPED value to match 1.00 spec

[Jeff Cody]

The 0.95 VHDX spec defined PAYLOAD_BLOCK_UNMAPPED to be 5.  The 1.00
VHDX spec redefines PAYLOAD_BLOCK_UNMAPPED to be 3 instead.

The original value of 5 is now an undefined state in the spec, but it
should be safe to treat it the same and return zeros for data read.
This way, we can maintain compatibility with any images out in the wild
that may have been created in accordance to the 0.95 spec.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/vhdx.c | 3 ++-
 block/vhdx.h | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index f1e1e2e..bec10bd 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1111,6 +1111,7 @@ static coroutine_fn int vhdx_co_readv(BlockDriverState *bs, int64_t sector_num,
             case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
             case PAYLOAD_BLOCK_UNDEFINED:
             case PAYLOAD_BLOCK_UNMAPPED:
+            case PAYLOAD_BLOCK_UNMAPPED_v095:
             case PAYLOAD_BLOCK_ZERO:
                 /* return zero */
                 qemu_iovec_memset(&hd_qiov, 0, 0, sinfo.bytes_avail);
@@ -1277,10 +1278,10 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
                         sectors_to_write += iov2.iov_len >> BDRV_SECTOR_BITS;
                     }
                 }
-
                 /* fall through */
             case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
             case PAYLOAD_BLOCK_UNMAPPED:
+            case PAYLOAD_BLOCK_UNMAPPED_v095:
             case PAYLOAD_BLOCK_UNDEFINED:
                 bat_prior_offset = sinfo.file_offset;
                 ret = vhdx_allocate_block(bs, s, &sinfo.file_offset);
diff --git a/block/vhdx.h b/block/vhdx.h
index b4a12a0..7003ab7 100644
--- a/block/vhdx.h
+++ b/block/vhdx.h
@@ -226,7 +226,8 @@ typedef struct QEMU_PACKED VHDXLogDataSector {
 #define PAYLOAD_BLOCK_NOT_PRESENT       0
 #define PAYLOAD_BLOCK_UNDEFINED         1
 #define PAYLOAD_BLOCK_ZERO              2
-#define PAYLOAD_BLOCK_UNMAPPED          5
+#define PAYLOAD_BLOCK_UNMAPPED          3
+#define PAYLOAD_BLOCK_UNMAPPED_v095     5
 #define PAYLOAD_BLOCK_FULLY_PRESENT     6
 #define PAYLOAD_BLOCK_PARTIALLY_PRESENT 7
 
-- 
1.9.3

[Qemu-devel] [PATCH 3/4] block: vhdx - change .vhdx_create default block state to ZERO

[Jeff Cody]

The VHDX spec specifies that the default new block state is
PAYLOAD_BLOCK_NOT_PRESENT for a dynamic VHDX image, and
PAYLOAD_BLOCK_FULLY_PRESENT for a fixed VHDX image.

However, in order to create space-efficient VHDX images with qemu-img
convert, it is desirable to be able to set has_zero_init to true for
VHDX.

There is currently an option when creating VHDX images, to use block
state ZERO for new blocks.  However, this currently defaults to 'off'.
In order to be able to eventually set has_zero_init to true for VHDX,
this needs to default to 'on'.

This patch changes the default to 'on', and provides some help
information to warn against setting it to 'off' when using qemu-img
convert.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/vhdx.c  | 6 ++++--
 qemu-doc.texi | 6 +++++-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index bec10bd..ddefc2a 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1774,7 +1774,7 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
     log_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_LOG_SIZE, 0);
     block_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_BLOCK_SIZE, 0);
     type = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
-    use_zero_blocks = qemu_opt_get_bool_del(opts, VHDX_BLOCK_OPT_ZERO, false);
+    use_zero_blocks = qemu_opt_get_bool_del(opts, VHDX_BLOCK_OPT_ZERO, true);
 
     if (image_size > VHDX_MAX_IMAGE_SIZE) {
         error_setg_errno(errp, EINVAL, "Image size too large; max of 64TB");
@@ -1936,7 +1936,9 @@ static QemuOptsList vhdx_create_opts = {
        {
            .name = VHDX_BLOCK_OPT_ZERO,
            .type = QEMU_OPT_BOOL,
-           .help = "Force use of payload blocks of type 'ZERO'.  Non-standard."
+           .help = "Force use of payload blocks of type 'ZERO'. "\
+                   "Non-standard, but default.  Do not set to 'off' when "\
+                   "using 'qemu-img convert' with subformat=dynamic"
        },
        { NULL }
     }
diff --git a/qemu-doc.texi b/qemu-doc.texi
index ad418f8..aabe8df 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -698,7 +698,11 @@ Supported options:
 Specifies which VHDX subformat to use. Valid options are
 @code{dynamic} (default) and @code{fixed}.
 @item block_state_zero
-Force use of payload blocks of type 'ZERO'.
+Force use of payload blocks of type 'ZERO'.  Can be set to @code{on} (default)
+or @code{off}.  When set to @code{off}, new blocks will be created as
+@code{PAYLOAD_BLOCK_NOT_PRESENT}, which means parsers are free to return
+arbitrary data for those blocks.  Do not set to @code{off} when using
+@code{qemu-img convert} with @code{subformat=dynamic}.
 @item block_size
 Block size; min 1 MB, max 256 MB.  0 means auto-calculate based on image size.
 @item log_size
-- 
1.9.3

[Qemu-devel] [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

[Jeff Cody]

Now that new VHDX images will default to BAT block states of
PAYLOAD_BLOCK_ZERO, we can indicate that VHDX has zero init.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/vhdx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/vhdx.c b/block/vhdx.c
index ddefc2a..2bbb3ee 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1956,6 +1956,7 @@ static BlockDriver bdrv_vhdx = {
     .bdrv_create            = vhdx_create,
     .bdrv_get_info          = vhdx_get_info,
     .bdrv_check             = vhdx_check,
+    .bdrv_has_zero_init     = bdrv_has_zero_init_1,
 
     .create_opts            = &vhdx_create_opts,
 };
-- 
1.9.3

Re: [Qemu-devel] [PATCH 1/4] block: vhdx - remove redundant comments

[Max Reitz]

On 2014-12-08 at 07:07, Jeff Cody wrote:
> Minor cleanup.
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>   block/vhdx.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/block/vhdx.c b/block/vhdx.c
> index 12bfe75..f1e1e2e 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -1109,8 +1109,8 @@ static coroutine_fn int vhdx_co_readv(BlockDriverState *bs, int64_t sector_num,
>               /* check the payload block state */
>               switch (s->bat[sinfo.bat_idx] & VHDX_BAT_STATE_BIT_MASK) {
>               case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
> -            case PAYLOAD_BLOCK_UNDEFINED:   /* fall through */
> -            case PAYLOAD_BLOCK_UNMAPPED:    /* fall through */
> +            case PAYLOAD_BLOCK_UNDEFINED:
> +            case PAYLOAD_BLOCK_UNMAPPED:
>               case PAYLOAD_BLOCK_ZERO:
>                   /* return zero */
>                   qemu_iovec_memset(&hd_qiov, 0, 0, sinfo.bytes_avail);
> @@ -1280,8 +1280,8 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
>   
>                   /* fall through */
>               case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */

Still a bit of redundancy left ;-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

> -            case PAYLOAD_BLOCK_UNMAPPED:    /* fall through */
> -            case PAYLOAD_BLOCK_UNDEFINED:   /* fall through */
> +            case PAYLOAD_BLOCK_UNMAPPED:
> +            case PAYLOAD_BLOCK_UNDEFINED:
>                   bat_prior_offset = sinfo.file_offset;
>                   ret = vhdx_allocate_block(bs, s, &sinfo.file_offset);
>                   if (ret < 0) {

Re: [Qemu-devel] [PATCH 2/4] block: vhdx - update PAYLOAD_BLOCK_UNMAPPED value to match 1.00 spec

[Max Reitz]

On 2014-12-08 at 07:07, Jeff Cody wrote:
> The 0.95 VHDX spec defined PAYLOAD_BLOCK_UNMAPPED to be 5.  The 1.00
> VHDX spec redefines PAYLOAD_BLOCK_UNMAPPED to be 3 instead.
>
> The original value of 5 is now an undefined state in the spec, but it
> should be safe to treat it the same and return zeros for data read.
> This way, we can maintain compatibility with any images out in the wild
> that may have been created in accordance to the 0.95 spec.
>
> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>   block/vhdx.c | 3 ++-
>   block/vhdx.h | 3 ++-
>   2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/block/vhdx.c b/block/vhdx.c
> index f1e1e2e..bec10bd 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -1111,6 +1111,7 @@ static coroutine_fn int vhdx_co_readv(BlockDriverState *bs, int64_t sector_num,
>               case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
>               case PAYLOAD_BLOCK_UNDEFINED:
>               case PAYLOAD_BLOCK_UNMAPPED:
> +            case PAYLOAD_BLOCK_UNMAPPED_v095:
>               case PAYLOAD_BLOCK_ZERO:
>                   /* return zero */
>                   qemu_iovec_memset(&hd_qiov, 0, 0, sinfo.bytes_avail);
> @@ -1277,10 +1278,10 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
>                           sectors_to_write += iov2.iov_len >> BDRV_SECTOR_BITS;
>                       }
>                   }
> -

Was this line removal intended?

Anyway:

Reviewed-by: Max Reitz <mreitz@redhat.com>

>                   /* fall through */
>               case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
>               case PAYLOAD_BLOCK_UNMAPPED:
> +            case PAYLOAD_BLOCK_UNMAPPED_v095:
>               case PAYLOAD_BLOCK_UNDEFINED:
>                   bat_prior_offset = sinfo.file_offset;
>                   ret = vhdx_allocate_block(bs, s, &sinfo.file_offset);
> diff --git a/block/vhdx.h b/block/vhdx.h
> index b4a12a0..7003ab7 100644
> --- a/block/vhdx.h
> +++ b/block/vhdx.h
> @@ -226,7 +226,8 @@ typedef struct QEMU_PACKED VHDXLogDataSector {
>   #define PAYLOAD_BLOCK_NOT_PRESENT       0
>   #define PAYLOAD_BLOCK_UNDEFINED         1
>   #define PAYLOAD_BLOCK_ZERO              2
> -#define PAYLOAD_BLOCK_UNMAPPED          5
> +#define PAYLOAD_BLOCK_UNMAPPED          3
> +#define PAYLOAD_BLOCK_UNMAPPED_v095     5
>   #define PAYLOAD_BLOCK_FULLY_PRESENT     6
>   #define PAYLOAD_BLOCK_PARTIALLY_PRESENT 7

Re: [Qemu-devel] [PATCH 3/4] block: vhdx - change .vhdx_create default block state to ZERO

[Max Reitz]

On 2014-12-08 at 07:07, Jeff Cody wrote:
> The VHDX spec specifies that the default new block state is
> PAYLOAD_BLOCK_NOT_PRESENT for a dynamic VHDX image, and
> PAYLOAD_BLOCK_FULLY_PRESENT for a fixed VHDX image.
>
> However, in order to create space-efficient VHDX images with qemu-img
> convert, it is desirable to be able to set has_zero_init to true for
> VHDX.
>
> There is currently an option when creating VHDX images, to use block
> state ZERO for new blocks.  However, this currently defaults to 'off'.
> In order to be able to eventually set has_zero_init to true for VHDX,
> this needs to default to 'on'.
>
> This patch changes the default to 'on', and provides some help
> information to warn against setting it to 'off' when using qemu-img
> convert.
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>   block/vhdx.c  | 6 ++++--
>   qemu-doc.texi | 6 +++++-
>   2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/block/vhdx.c b/block/vhdx.c
> index bec10bd..ddefc2a 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -1774,7 +1774,7 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
>       log_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_LOG_SIZE, 0);
>       block_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_BLOCK_SIZE, 0);
>       type = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
> -    use_zero_blocks = qemu_opt_get_bool_del(opts, VHDX_BLOCK_OPT_ZERO, false);
> +    use_zero_blocks = qemu_opt_get_bool_del(opts, VHDX_BLOCK_OPT_ZERO, true);
>   
>       if (image_size > VHDX_MAX_IMAGE_SIZE) {
>           error_setg_errno(errp, EINVAL, "Image size too large; max of 64TB");
> @@ -1936,7 +1936,9 @@ static QemuOptsList vhdx_create_opts = {
>          {
>              .name = VHDX_BLOCK_OPT_ZERO,
>              .type = QEMU_OPT_BOOL,
> -           .help = "Force use of payload blocks of type 'ZERO'.  Non-standard."
> +           .help = "Force use of payload blocks of type 'ZERO'. "\
> +                   "Non-standard, but default.  Do not set to 'off' when "\
> +                   "using 'qemu-img convert' with subformat=dynamic"

Full stop missing?

>          },
>          { NULL }
>       }
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index ad418f8..aabe8df 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -698,7 +698,11 @@ Supported options:
>   Specifies which VHDX subformat to use. Valid options are
>   @code{dynamic} (default) and @code{fixed}.
>   @item block_state_zero
> -Force use of payload blocks of type 'ZERO'.
> +Force use of payload blocks of type 'ZERO'.  Can be set to @code{on} (default)
> +or @code{off}.  When set to @code{off}, new blocks will be created as
> +@code{PAYLOAD_BLOCK_NOT_PRESENT}, which means parsers are free to return
> +arbitrary data for those blocks.  Do not set to @code{off} when using
> +@code{qemu-img convert} with @code{subformat=dynamic}.

Well, it's fine to use block_state_zero=off with -S 0, but then you 
won't get much usage out of having set it to off because all blocks will 
be allocated anyway.

>   @item block_size
>   Block size; min 1 MB, max 256 MB.  0 means auto-calculate based on image size.
>   @item log_size

With the full stop added:

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [Qemu-devel] [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

[Max Reitz]

On 2014-12-08 at 07:07, Jeff Cody wrote:
> Now that new VHDX images will default to BAT block states of
> PAYLOAD_BLOCK_ZERO, we can indicate that VHDX has zero init.
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>   block/vhdx.c | 1 +
>   1 file changed, 1 insertion(+)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [Qemu-devel] [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

[Lokesha, Amulya]

Hi,

I raised a bug https://bugs.launchpad.net/qemu/+bug/1399191 in the qemu bugzilla and immediately started getting these patch mails. But, I am not seeing any update in the bugzilla site.
Till now I have got 4 patch mails regarding the issue. Please let me know which patch I should apply. Also let me know where can I get the patch files individually to be downloaded.

We are waiting for the patch to provide to our customers.

Thanks,
Amulya


-----Original Message-----
From: Max Reitz [mailto:mreitz@redhat.com] 
Sent: Monday, December 08, 2014 2:20 PM
To: Jeff Cody; qemu-devel@nongnu.org
Cc: kwolf@redhat.com; Lokesha, Amulya; stefanha@redhat.com
Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

On 2014-12-08 at 07:07, Jeff Cody wrote:
> Now that new VHDX images will default to BAT block states of 
> PAYLOAD_BLOCK_ZERO, we can indicate that VHDX has zero init.
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>   block/vhdx.c | 1 +
>   1 file changed, 1 insertion(+)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [Qemu-devel] [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

[Max Reitz]

On 2014-12-11 at 05:21, Lokesha, Amulya wrote:
> Hi,
>
> I raised a bug https://bugs.launchpad.net/qemu/+bug/1399191 in the qemu bugzilla and immediately started getting these patch mails. But, I am not seeing any update in the bugzilla site.
> Till now I have got 4 patch mails regarding the issue. Please let me know which patch I should apply. Also let me know where can I get the patch files individually to be downloaded.
>
> We are waiting for the patch to provide to our customers.
>
> Thanks,
> Amulya

Hi Amulya,

Patches 3 and 4 are absolutely necessary for this issue; however, I 
would recommend to simply apply all patches of the series.

In order to download the patches you can simply save the patch emails 
and apply them with git am. If for some reason you don't want to use 
git, patch -p1 works as well.

Max

Re: [Qemu-devel] [PATCH 1/4] block: vhdx - remove redundant comments

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1869 bytes --]

On Mon, Dec 08, 2014 at 01:07:42AM -0500, Jeff Cody wrote:
> Minor cleanup.
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/vhdx.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/block/vhdx.c b/block/vhdx.c
> index 12bfe75..f1e1e2e 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -1109,8 +1109,8 @@ static coroutine_fn int vhdx_co_readv(BlockDriverState *bs, int64_t sector_num,
>              /* check the payload block state */
>              switch (s->bat[sinfo.bat_idx] & VHDX_BAT_STATE_BIT_MASK) {
>              case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
> -            case PAYLOAD_BLOCK_UNDEFINED:   /* fall through */
> -            case PAYLOAD_BLOCK_UNMAPPED:    /* fall through */
> +            case PAYLOAD_BLOCK_UNDEFINED:
> +            case PAYLOAD_BLOCK_UNMAPPED:
>              case PAYLOAD_BLOCK_ZERO:
>                  /* return zero */
>                  qemu_iovec_memset(&hd_qiov, 0, 0, sinfo.bytes_avail);
> @@ -1280,8 +1280,8 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
>  
>                  /* fall through */
>              case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
> -            case PAYLOAD_BLOCK_UNMAPPED:    /* fall through */
> -            case PAYLOAD_BLOCK_UNDEFINED:   /* fall through */
> +            case PAYLOAD_BLOCK_UNMAPPED:
> +            case PAYLOAD_BLOCK_UNDEFINED:
>                  bat_prior_offset = sinfo.file_offset;
>                  ret = vhdx_allocate_block(bs, s, &sinfo.file_offset);
>                  if (ret < 0) {

Fall through comments are used by some static checkers.  Not sure if all
checkers are smart enough to propagate the comment from adjacent case
statements.  I would have left the comments alone but am okay with
merging this.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

[Lokesha, Amulya]

[-- Attachment #1.1: Type: text/plain, Size: 2231 bytes --]

Hi Max,



We applied all the 5 patches from the mail chain I got since the last week. Please find attached the patches used by us.

We were unable to apply the patch3 as it failed with the following error



# patch -p1 < patch3

patching file block/vhdx.c

patch: **** malformed patch at line 17:          error_setg_errno(errp, EINVAL, "Image size too large; max of 64TB"); @@ -1936,7 +1936,9 @@ static QemuOptsList vhdx_create_opts = {





Hence, we manually added the patch3 changes and recompiled the qemu. We then used the patched qemu-img to convert  our vmdk image to dynamic VHDX format. We found that the image created this time had a considerable decrease in its size from 50GB to 12GB.

However, when we deployed it into our SCVMM 2012, the import of the VHDX image failed with a "syntax error" as below



Information (10804)

Unable to import \\Test.com\Library\VHDs\Test-disk1.vhdx because of a syntax error in the file.



Please let us know if we missed anything.



Thanks,

Amulya



-----Original Message-----
From: Max Reitz [mailto:mreitz@redhat.com]
Sent: Thursday, December 11, 2014 2:36 PM
To: Lokesha, Amulya; Jeff Cody; qemu-devel@nongnu.org
Cc: kwolf@redhat.com; stefanha@redhat.com
Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1



On 2014-12-11 at 05:21, Lokesha, Amulya wrote:

> Hi,

>

> I raised a bug https://bugs.launchpad.net/qemu/+bug/1399191 in the qemu bugzilla and immediately started getting these patch mails. But, I am not seeing any update in the bugzilla site.

> Till now I have got 4 patch mails regarding the issue. Please let me know which patch I should apply. Also let me know where can I get the patch files individually to be downloaded.

>

> We are waiting for the patch to provide to our customers.

>

> Thanks,

> Amulya



Hi Amulya,



Patches 3 and 4 are absolutely necessary for this issue; however, I would recommend to simply apply all patches of the series.



In order to download the patches you can simply save the patch emails and apply them with git am. If for some reason you don't want to use git, patch -p1 works as well.



Max

[-- Attachment #1.2: Type: text/html, Size: 6013 bytes --]

[-- Attachment #2: patch0.patch --]
[-- Type: application/octet-stream, Size: 453 bytes --]

diff --git a/block/vhdx.c b/block/vhdx.c index 12bfe75..8a54da0 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1951,6 +1951,7 @@ static BlockDriver bdrv_vhdx = {
     .bdrv_co_readv          = vhdx_co_readv,
     .bdrv_co_writev         = vhdx_co_writev,
     .bdrv_create            = vhdx_create,
+    .bdrv_has_zero_init     = bdrv_has_zero_init_1,
     .bdrv_get_info          = vhdx_get_info,
     .bdrv_check             = vhdx_check,

[-- Attachment #3: patch1.patch --]
[-- Type: application/octet-stream, Size: 1330 bytes --]

diff --git a/block/vhdx.c b/block/vhdx.c index 12bfe75..f1e1e2e 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1109,8 +1109,8 @@ static coroutine_fn int vhdx_co_readv(BlockDriverState *bs, int64_t sector_num,
             /* check the payload block state */
             switch (s->bat[sinfo.bat_idx] & VHDX_BAT_STATE_BIT_MASK) {
             case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
-            case PAYLOAD_BLOCK_UNDEFINED:   /* fall through */
-            case PAYLOAD_BLOCK_UNMAPPED:    /* fall through */
+            case PAYLOAD_BLOCK_UNDEFINED:
+            case PAYLOAD_BLOCK_UNMAPPED:
             case PAYLOAD_BLOCK_ZERO:
                 /* return zero */
                 qemu_iovec_memset(&hd_qiov, 0, 0, sinfo.bytes_avail); 
@@ -1280,8 +1280,8 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
 
                 /* fall through */
             case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
-            case PAYLOAD_BLOCK_UNMAPPED:    /* fall through */
-            case PAYLOAD_BLOCK_UNDEFINED:   /* fall through */
+            case PAYLOAD_BLOCK_UNMAPPED:
+            case PAYLOAD_BLOCK_UNDEFINED:
                 bat_prior_offset = sinfo.file_offset;
                 ret = vhdx_allocate_block(bs, s, &sinfo.file_offset);
                 if (ret < 0) {

[-- Attachment #4: patch2.patch --]
[-- Type: application/octet-stream, Size: 1659 bytes --]

diff --git a/block/vhdx.c b/block/vhdx.c index f1e1e2e..bec10bd 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1111,6 +1111,7 @@ static coroutine_fn int vhdx_co_readv(BlockDriverState *bs, int64_t sector_num,
             case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
             case PAYLOAD_BLOCK_UNDEFINED:
             case PAYLOAD_BLOCK_UNMAPPED:
+            case PAYLOAD_BLOCK_UNMAPPED_v095:
             case PAYLOAD_BLOCK_ZERO:
                 /* return zero */
                 qemu_iovec_memset(&hd_qiov, 0, 0, sinfo.bytes_avail);
@@ -1277,10 +1278,10 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
                         sectors_to_write += iov2.iov_len >> BDRV_SECTOR_BITS;
                     }
                 }
-
                 /* fall through */
             case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
             case PAYLOAD_BLOCK_UNMAPPED:
+            case PAYLOAD_BLOCK_UNMAPPED_v095:
             case PAYLOAD_BLOCK_UNDEFINED:
                 bat_prior_offset = sinfo.file_offset;
                 ret = vhdx_allocate_block(bs, s, &sinfo.file_offset); diff --git a/block/vhdx.h b/block/vhdx.h index b4a12a0..7003ab7 100644
--- a/block/vhdx.h
+++ b/block/vhdx.h
@@ -226,7 +226,8 @@ typedef struct QEMU_PACKED VHDXLogDataSector {
 #define PAYLOAD_BLOCK_NOT_PRESENT       0
 #define PAYLOAD_BLOCK_UNDEFINED         1
 #define PAYLOAD_BLOCK_ZERO              2
-#define PAYLOAD_BLOCK_UNMAPPED          5
+#define PAYLOAD_BLOCK_UNMAPPED          3
+#define PAYLOAD_BLOCK_UNMAPPED_v095     5
 #define PAYLOAD_BLOCK_FULLY_PRESENT     6
 #define PAYLOAD_BLOCK_PARTIALLY_PRESENT 7
 

[-- Attachment #5: patch3.patch --]
[-- Type: application/octet-stream, Size: 1972 bytes --]

diff --git a/block/vhdx.c b/block/vhdx.c index bec10bd..ddefc2a 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1774,7 +1774,7 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
     log_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_LOG_SIZE, 0);
     block_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_BLOCK_SIZE, 0);
     type = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
-    use_zero_blocks = qemu_opt_get_bool_del(opts, VHDX_BLOCK_OPT_ZERO, false);
+    use_zero_blocks = qemu_opt_get_bool_del(opts, VHDX_BLOCK_OPT_ZERO, 
+ true);
 
     if (image_size > VHDX_MAX_IMAGE_SIZE) {
         error_setg_errno(errp, EINVAL, "Image size too large; max of 64TB"); 
@@ -1936,7 +1936,9 @@ static QemuOptsList vhdx_create_opts = {
        {
            .name = VHDX_BLOCK_OPT_ZERO,
            .type = QEMU_OPT_BOOL,
-           .help = "Force use of payload blocks of type 'ZERO'.  Non-standard."
+           .help = "Force use of payload blocks of type 'ZERO'. "\
+                   "Non-standard, but default.  Do not set to 'off' when "\
+                   "using 'qemu-img convert' with subformat=dynamic"
        },
        { NULL }
     }
diff --git a/qemu-doc.texi b/qemu-doc.texi index ad418f8..aabe8df 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -698,7 +698,11 @@ Supported options:
 Specifies which VHDX subformat to use. Valid options are  @code{dynamic} (default) and @code{fixed}.
 @item block_state_zero
-Force use of payload blocks of type 'ZERO'.
+Force use of payload blocks of type 'ZERO'.  Can be set to @code{on} 
+(default) or @code{off}.  When set to @code{off}, new blocks will be 
+created as @code{PAYLOAD_BLOCK_NOT_PRESENT}, which means parsers are 
+free to return arbitrary data for those blocks.  Do not set to 
+@code{off} when using @code{qemu-img convert} with @code{subformat=dynamic}.
 @item block_size
 Block size; min 1 MB, max 256 MB.  0 means auto-calculate based on image size.
 @item log_size

[-- Attachment #6: patch4.patch --]
[-- Type: application/octet-stream, Size: 406 bytes --]

diff --git a/block/vhdx.c b/block/vhdx.c index ddefc2a..2bbb3ee 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1956,6 +1956,7 @@ static BlockDriver bdrv_vhdx = {
     .bdrv_create            = vhdx_create,
     .bdrv_get_info          = vhdx_get_info,
     .bdrv_check             = vhdx_check,
+    .bdrv_has_zero_init     = bdrv_has_zero_init_1,
 
     .create_opts            = &vhdx_create_opts,
 };

Re: [Qemu-devel] [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

[Jeff Cody]

On Fri, Dec 12, 2014 at 09:43:16AM -0500, Lokesha, Amulya wrote:
>    Hi Max,
> 
>     

Please reply in-line, it makes it easier to follow technical
discussions - thanks :)

> 
>    We applied all the 5 patches from the mail chain I got since the last
>    week. Please find attached the patches used by us.
> 
>    We were unable to apply the patch3 as it failed with the following error
> 
>     
> 
>    # patch -p1 < patch3
> 
>    patching file block/vhdx.c
> 
>    patch: **** malformed patch at line 17:          error_setg_errno(errp,
>    EINVAL, "Image size too large; max of 64TB"); @@ -1936,7 +1936,9 @@ static
>    QemuOptsList vhdx_create_opts = {
> 
>

It looks like however you saved the patch file, it was corrupted.
Looking at your attached patch 3, it split line 9 across 2 lines.
Your patch also has whitespace differences from the patch I sent.

You also attached 5 patches - Why are you using patch 0?  You should
only be applying patches 1-4.  This should not be causing any actual
issues, however.

Are you using git for your qemu version?  If so, 'git am' is the
preferred method of applying the patches - just save each of the patch
emails (the whole email should be fine), and run 'git am' on each
file.


> 
>     
> 
>    Hence, we manually added the patch3 changes and recompiled the qemu. We
>    then used the patched qemu-img to convert  our vmdk image to dynamic VHDX
>    format. We found that the image created this time had a considerable
>    decrease in its size from 50GB to 12GB.
> 

Could you tell me the file size of the VMDK image you were converting?
Is it roughly 12GB as well?

>    However, when we deployed it into our SCVMM 2012, the import of the VHDX
>    image failed with a "syntax error" as below
> 
>     
> 
>    Information (10804)
> 
>    Unable to import \\Test.com\Library\VHDs\Test-disk1.vhdx because of a
>    syntax error in the file.
> 
>     

If you run qemu-img info on Test-disk1.vhdx, what does it say?

> 
>    Please let us know if we missed anything.
> 
>     
> 
>    Thanks,
> 
>    Amulya
> 
>     
>

[...]

Jeff

Re: [Qemu-devel] [PATCH 0/4] VHDX Fixes for zero init

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]

On Mon, Dec 08, 2014 at 01:07:41AM -0500, Jeff Cody wrote:
> A couple of VHDX fixes in this series:
>  * updating the driver to reflect the 1.00 spec for the value of
>    PAYLOAD_BLOCK_UNMAPPED (thanks Kevin)
>  * enabling VHDX to support zero init in qemu-img convert
> 
> 1/4: compiling: e8718ae: block: vhdx - remove redundant comments
> 2/4: compiling: 8a4d2da: block: vhdx - update PAYLOAD_BLOCK_UNMAPPED value to match 1.00 spec
> 3/4: compiling: 44f08c2: block: vhdx - change .vhdx_create default block state to ZERO
> 4/4: compiling: 3eb0cd2: block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1
> 
> Jeff Cody (4):
>   block: vhdx - remove redundant comments
>   block: vhdx - update PAYLOAD_BLOCK_UNMAPPED value to match 1.00 spec
>   block: vhdx - change .vhdx_create default block state to ZERO
>   block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1
> 
>  block/vhdx.c  | 18 +++++++++++-------
>  block/vhdx.h  |  3 ++-
>  qemu-doc.texi |  6 +++++-
>  3 files changed, 18 insertions(+), 9 deletions(-)
> 
> -- 
> 1.9.3
> 
> 

Thanks, applied to my HEAD tree:
https://github.com/stefanha/qemu/commits/HEAD

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

[Lokesha, Amulya]

Please find my comments inline

Thanks,
Amulya

-----Original Message-----
From: Jeff Cody [mailto:jcody@redhat.com] 
Sent: Friday, December 12, 2014 8:48 PM
To: Lokesha, Amulya
Cc: Max Reitz; qemu-devel@nongnu.org; kwolf@redhat.com; stefanha@redhat.com
Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

On Fri, Dec 12, 2014 at 09:43:16AM -0500, Lokesha, Amulya wrote:
>    Hi Max,
> 
>     

Please reply in-line, it makes it easier to follow technical discussions - thanks :)

> 
>    We applied all the 5 patches from the mail chain I got since the last
>    week. Please find attached the patches used by us.
> 
>    We were unable to apply the patch3 as it failed with the following 
> error
> 
>     
> 
>    # patch -p1 < patch3
> 
>    patching file block/vhdx.c
> 
>    patch: **** malformed patch at line 17:          error_setg_errno(errp,
>    EINVAL, "Image size too large; max of 64TB"); @@ -1936,7 +1936,9 @@ static
>    QemuOptsList vhdx_create_opts = {
> 
>

It looks like however you saved the patch file, it was corrupted.
Looking at your attached patch 3, it split line 9 across 2 lines.
Your patch also has whitespace differences from the patch I sent.

You also attached 5 patches - Why are you using patch 0?  You should only be applying patches 1-4.  This should not be causing any actual issues, however.
First time we applied patches 1 to 4, created VHDX image and deployed to HyperV Server, but we got the same error. Then we took a fresh qemu source and applied patches 0 to 4 and deployed to HyperV and again got the same syntax error.


Are you using git for your qemu version?  If so, 'git am' is the preferred method of applying the patches - just save each of the patch emails (the whole email should be fine), and run 'git am' on each file.

[Amulya] : No. We don't have a git repository for our team. Could you please let us know how to apply these patches without git. What is the difference in applying the patch directly and modifying the code directly? Does it have any impact?

> 
>     
> 
>    Hence, we manually added the patch3 changes and recompiled the qemu. We
>    then used the patched qemu-img to convert  our vmdk image to dynamic VHDX
>    format. We found that the image created this time had a considerable
>    decrease in its size from 50GB to 12GB.
> 

Could you tell me the file size of the VMDK image you were converting?
Is it roughly 12GB as well?
[Amulya] :  No, the vmdk image which we used for conversion is just 1.4GB


>    However, when we deployed it into our SCVMM 2012, the import of the VHDX
>    image failed with a "syntax error" as below
> 
>     
> 
>    Information (10804)
> 
>    Unable to import \\Test.com\Library\VHDs\Test-disk1.vhdx because of a
>    syntax error in the file.
> 
>     

If you run qemu-img info on Test-disk1.vhdx, what does it say?

[Amulya] :   The following is seen with qemu-img info
# qemu-img info Test-disk1.vhdx
image: Test-disk1.vhdx
file format: vhdx
virtual size: 50G (53687091200 bytes)
disk size: 3.4G
cluster_size: 16777216

The size of our images
# ls -ltrh
total 4.8G
-rw-r--r-- 1 root root 1.4G Dec  9 10:48 Test-disk1.vmdk
-rw-r--r-- 1 root root  12G Dec 12 04:49 Test-disk1.vhdx

> 
>    Please let us know if we missed anything.
> 
>     
> 
>    Thanks,
> 
>    Amulya
> 
>     
>

[...]

Jeff

Re: [Qemu-devel] [PATCH 1/4] block: vhdx - remove redundant comments

[Markus Armbruster]

Stefan Hajnoczi <stefanha@gmail.com> writes:

> On Mon, Dec 08, 2014 at 01:07:42AM -0500, Jeff Cody wrote:
>> Minor cleanup.
>> 
>> Signed-off-by: Jeff Cody <jcody@redhat.com>
>> ---
>>  block/vhdx.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/block/vhdx.c b/block/vhdx.c
>> index 12bfe75..f1e1e2e 100644
>> --- a/block/vhdx.c
>> +++ b/block/vhdx.c
>> @@ -1109,8 +1109,8 @@ static coroutine_fn int vhdx_co_readv(BlockDriverState *bs, int64_t sector_num,
>>              /* check the payload block state */
>>              switch (s->bat[sinfo.bat_idx] & VHDX_BAT_STATE_BIT_MASK) {
>>              case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */

Please drop this one as well, it's 100% certified noise.

>> -            case PAYLOAD_BLOCK_UNDEFINED:   /* fall through */
>> -            case PAYLOAD_BLOCK_UNMAPPED:    /* fall through */
>> +            case PAYLOAD_BLOCK_UNDEFINED:
>> +            case PAYLOAD_BLOCK_UNMAPPED:
>>              case PAYLOAD_BLOCK_ZERO:
>>                  /* return zero */
>>                  qemu_iovec_memset(&hd_qiov, 0, 0, sinfo.bytes_avail);
>> @@ -1280,8 +1280,8 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
>>  
>>                  /* fall through */

This is a keeper.

>>              case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */

This isn't.

>> -            case PAYLOAD_BLOCK_UNMAPPED:    /* fall through */
>> -            case PAYLOAD_BLOCK_UNDEFINED:   /* fall through */
>> +            case PAYLOAD_BLOCK_UNMAPPED:
>> +            case PAYLOAD_BLOCK_UNDEFINED:
>>                  bat_prior_offset = sinfo.file_offset;
>>                  ret = vhdx_allocate_block(bs, s, &sinfo.file_offset);
>>                  if (ret < 0) {
>
> Fall through comments are used by some static checkers.  Not sure if all
> checkers are smart enough to propagate the comment from adjacent case
> statements.  I would have left the comments alone but am okay with
> merging this.

I have never found a compiler or static checker so fanciful it actually
nags its users about the perfectly obvious and common pattern "several
case labels without code in between".

The problematic pattern is "control falls from one case's *code* through
to the next case".  That one needs a comment indeed.

Re: [Qemu-devel] [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

[Lokesha, Amulya]

Hi Max, Jeff,

We were able to get the qemu patch files downloaded from the qemu patch site -  https://patchwork.ozlabs.org and were able to apply the patches successfully without any errors. With the patches applied, we recompiled the qemu and converted the VDK vmdk to vhdx format and uploaded to the SCVMM Server. But it failed again with the syntax error as below:

Information (10804)
Unable to import \\TestServer\MSSCVMMLibrary\VHDs\Product-disk1.vhdx because of a syntax error in the file.

Please find my comments inline for your questions

Please let us know if there is anything else you need from us.


Thanks,
Amulya


-----Original Message-----
From: Jeff Cody [mailto:jcody@redhat.com]
Sent: Friday, December 12, 2014 8:48 PM
To: Lokesha, Amulya
Cc: Max Reitz; qemu-devel@nongnu.org; kwolf@redhat.com; stefanha@redhat.com
Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

On Fri, Dec 12, 2014 at 09:43:16AM -0500, Lokesha, Amulya wrote:
>    Hi Max,
> 
>     

Please reply in-line, it makes it easier to follow technical discussions - thanks :)

> 
>    We applied all the 5 patches from the mail chain I got since the last
>    week. Please find attached the patches used by us.
> 
>    We were unable to apply the patch3 as it failed with the following 
> error
> 
>     
> 
>    # patch -p1 < patch3
> 
>    patching file block/vhdx.c
> 
>    patch: **** malformed patch at line 17:          error_setg_errno(errp,
>    EINVAL, "Image size too large; max of 64TB"); @@ -1936,7 +1936,9 @@ static
>    QemuOptsList vhdx_create_opts = {
> 
>

It looks like however you saved the patch file, it was corrupted.
Looking at your attached patch 3, it split line 9 across 2 lines.
Your patch also has whitespace differences from the patch I sent.

You also attached 5 patches - Why are you using patch 0?  You should only be applying patches 1-4.  This should not be causing any actual issues, however.
[Amulya]: First time we applied patches 1 to 4, created VHDX image and deployed to HyperV Server, but we got the same error. Then we took a fresh qemu source and applied patches 0 to 4 and deployed to HyperV and again got the same syntax error.


Are you using git for your qemu version?  If so, 'git am' is the preferred method of applying the patches - just save each of the patch emails (the whole email should be fine), and run 'git am' on each file.

[Amulya] : No. We don't have a git repository for our team. Could you please let us know how to apply these patches without git. What is the difference in applying the patch directly and modifying the code directly? Does it have any impact?

> 
>     
> 
>    Hence, we manually added the patch3 changes and recompiled the qemu. We
>    then used the patched qemu-img to convert  our vmdk image to dynamic VHDX
>    format. We found that the image created this time had a considerable
>    decrease in its size from 50GB to 12GB.
> 

Could you tell me the file size of the VMDK image you were converting?
Is it roughly 12GB as well?
[Amulya] :  No, the vmdk image which we used for conversion is just 1.4GB


>    However, when we deployed it into our SCVMM 2012, the import of the VHDX
>    image failed with a "syntax error" as below
> 
>     
> 
>    Information (10804)
> 
>    Unable to import \\Test.com\Library\VHDs\Test-disk1.vhdx because of a
>    syntax error in the file.
> 
>     

If you run qemu-img info on Test-disk1.vhdx, what does it say?

[Amulya] :   The following is seen with qemu-img info
# qemu-img info Test-disk1.vhdx
image: Test-disk1.vhdx
file format: vhdx
virtual size: 50G (53687091200 bytes)
disk size: 3.4G
cluster_size: 16777216

The size of our images
# ls -ltrh
total 4.8G
-rw-r--r-- 1 root root 1.4G Dec  9 10:48 Test-disk1.vmdk
-rw-r--r-- 1 root root  12G Dec 12 04:49 Test-disk1.vhdx

> 
>    Please let us know if we missed anything.
> 
>     
> 
>    Thanks,
> 
>    Amulya
> 
>     
>

[...]

Jeff

Re: [Qemu-devel] [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

[Jeff Cody]

On Wed, Dec 17, 2014 at 05:46:32AM -0500, Lokesha, Amulya wrote:
> Hi Max, Jeff,
> 
> We were able to get the qemu patch files downloaded from the qemu patch site -  https://patchwork.ozlabs.org and were able to apply the patches successfully without any errors. With the patches applied, we recompiled the qemu and converted the VDK vmdk to vhdx format and uploaded to the SCVMM Server. But it failed again with the syntax error as below:
> 
> Information (10804)
> Unable to import \\TestServer\MSSCVMMLibrary\VHDs\Product-disk1.vhdx because of a syntax error in the file.
> 
> Please find my comments inline for your questions
> 
> Please let us know if there is anything else you need from us.
> 
>

Amulya,

I will try to test this on Windows Server, and see if I can reproduce
what you are seeing.

-Jeff

> 
> 
> -----Original Message-----
> From: Jeff Cody [mailto:jcody@redhat.com]
> Sent: Friday, December 12, 2014 8:48 PM
> To: Lokesha, Amulya
> Cc: Max Reitz; qemu-devel@nongnu.org; kwolf@redhat.com; stefanha@redhat.com
> Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1
> 
> On Fri, Dec 12, 2014 at 09:43:16AM -0500, Lokesha, Amulya wrote:
> >    Hi Max,
> > 
> >     
> 
> Please reply in-line, it makes it easier to follow technical discussions - thanks :)
> 
> > 
> >    We applied all the 5 patches from the mail chain I got since the last
> >    week. Please find attached the patches used by us.
> > 
> >    We were unable to apply the patch3 as it failed with the following 
> > error
> > 
> >     
> > 
> >    # patch -p1 < patch3
> > 
> >    patching file block/vhdx.c
> > 
> >    patch: **** malformed patch at line 17:          error_setg_errno(errp,
> >    EINVAL, "Image size too large; max of 64TB"); @@ -1936,7 +1936,9 @@ static
> >    QemuOptsList vhdx_create_opts = {
> > 
> >
> 
> It looks like however you saved the patch file, it was corrupted.
> Looking at your attached patch 3, it split line 9 across 2 lines.
> Your patch also has whitespace differences from the patch I sent.
> 
> You also attached 5 patches - Why are you using patch 0?  You should only be applying patches 1-4.  This should not be causing any actual issues, however.
> [Amulya]: First time we applied patches 1 to 4, created VHDX image and deployed to HyperV Server, but we got the same error. Then we took a fresh qemu source and applied patches 0 to 4 and deployed to HyperV and again got the same syntax error.
> 
> 
> Are you using git for your qemu version?  If so, 'git am' is the preferred method of applying the patches - just save each of the patch emails (the whole email should be fine), and run 'git am' on each file.
> 
> [Amulya] : No. We don't have a git repository for our team. Could you please let us know how to apply these patches without git. What is the difference in applying the patch directly and modifying the code directly? Does it have any impact?
> 
> > 
> >     
> > 
> >    Hence, we manually added the patch3 changes and recompiled the qemu. We
> >    then used the patched qemu-img to convert  our vmdk image to dynamic VHDX
> >    format. We found that the image created this time had a considerable
> >    decrease in its size from 50GB to 12GB.
> > 
> 
> Could you tell me the file size of the VMDK image you were converting?
> Is it roughly 12GB as well?
> [Amulya] :  No, the vmdk image which we used for conversion is just 1.4GB
> 
> 
> >    However, when we deployed it into our SCVMM 2012, the import of the VHDX
> >    image failed with a "syntax error" as below
> > 
> >     
> > 
> >    Information (10804)
> > 
> >    Unable to import \\Test.com\Library\VHDs\Test-disk1.vhdx because of a
> >    syntax error in the file.
> > 
> >     
> 
> If you run qemu-img info on Test-disk1.vhdx, what does it say?
> 
> [Amulya] :   The following is seen with qemu-img info
> # qemu-img info Test-disk1.vhdx
> image: Test-disk1.vhdx
> file format: vhdx
> virtual size: 50G (53687091200 bytes)
> disk size: 3.4G
> cluster_size: 16777216
> 
> The size of our images
> # ls -ltrh
> total 4.8G
> -rw-r--r-- 1 root root 1.4G Dec  9 10:48 Test-disk1.vmdk
> -rw-r--r-- 1 root root  12G Dec 12 04:49 Test-disk1.vhdx
> 
> > 
> >    Please let us know if we missed anything.
> > 
> >     
> > 
> >    Thanks,
> > 
> >    Amulya
> > 
> >     
> >
> 
> [...]
> 
> Jeff

Re: [Qemu-devel] [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

[Lokesha, Amulya]

-----Original Message-----
From: Jeff Cody [mailto:jcody@redhat.com] 
Sent: Wednesday, December 17, 2014 5:44 PM
To: Lokesha, Amulya
Cc: Max Reitz; qemu-devel@nongnu.org; kwolf@redhat.com; stefanha@redhat.com
Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

On Wed, Dec 17, 2014 at 05:46:32AM -0500, Lokesha, Amulya wrote:
> Hi Max, Jeff,
> 
> We were able to get the qemu patch files downloaded from the qemu patch site -  https://patchwork.ozlabs.org and were able to apply the patches successfully without any errors. With the patches applied, we recompiled the qemu and converted the VDK vmdk to vhdx format and uploaded to the SCVMM Server. But it failed again with the syntax error as below:
> 
> Information (10804)
> Unable to import \\TestServer\MSSCVMMLibrary\VHDs\Product-disk1.vhdx because of a syntax error in the file.
> 
> Please find my comments inline for your questions
> 
> Please let us know if there is anything else you need from us.
> 
>

Amulya,

I will try to test this on Windows Server, and see if I can reproduce what you are seeing.

-Jeff


Hi Jeff,

Any updates on this? Were you able to test it

Thanks,
Amulya


> 
> 
> -----Original Message-----
> From: Jeff Cody [mailto:jcody@redhat.com]
> Sent: Friday, December 12, 2014 8:48 PM
> To: Lokesha, Amulya
> Cc: Max Reitz; qemu-devel@nongnu.org; kwolf@redhat.com; 
> stefanha@redhat.com
> Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to 
> bdrv_has_zero_init_1
> 
> On Fri, Dec 12, 2014 at 09:43:16AM -0500, Lokesha, Amulya wrote:
> >    Hi Max,
> > 
> >     
> 
> Please reply in-line, it makes it easier to follow technical 
> discussions - thanks :)
> 
> > 
> >    We applied all the 5 patches from the mail chain I got since the last
> >    week. Please find attached the patches used by us.
> > 
> >    We were unable to apply the patch3 as it failed with the 
> > following error
> > 
> >     
> > 
> >    # patch -p1 < patch3
> > 
> >    patching file block/vhdx.c
> > 
> >    patch: **** malformed patch at line 17:          error_setg_errno(errp,
> >    EINVAL, "Image size too large; max of 64TB"); @@ -1936,7 +1936,9 @@ static
> >    QemuOptsList vhdx_create_opts = {
> > 
> >
> 
> It looks like however you saved the patch file, it was corrupted.
> Looking at your attached patch 3, it split line 9 across 2 lines.
> Your patch also has whitespace differences from the patch I sent.
> 
> You also attached 5 patches - Why are you using patch 0?  You should only be applying patches 1-4.  This should not be causing any actual issues, however.
> [Amulya]: First time we applied patches 1 to 4, created VHDX image and deployed to HyperV Server, but we got the same error. Then we took a fresh qemu source and applied patches 0 to 4 and deployed to HyperV and again got the same syntax error.
> 
> 
> Are you using git for your qemu version?  If so, 'git am' is the preferred method of applying the patches - just save each of the patch emails (the whole email should be fine), and run 'git am' on each file.
> 
> [Amulya] : No. We don't have a git repository for our team. Could you please let us know how to apply these patches without git. What is the difference in applying the patch directly and modifying the code directly? Does it have any impact?
> 
> > 
> >     
> > 
> >    Hence, we manually added the patch3 changes and recompiled the qemu. We
> >    then used the patched qemu-img to convert  our vmdk image to dynamic VHDX
> >    format. We found that the image created this time had a considerable
> >    decrease in its size from 50GB to 12GB.
> > 
> 
> Could you tell me the file size of the VMDK image you were converting?
> Is it roughly 12GB as well?
> [Amulya] :  No, the vmdk image which we used for conversion is just 
> 1.4GB
> 
> 
> >    However, when we deployed it into our SCVMM 2012, the import of the VHDX
> >    image failed with a "syntax error" as below
> > 
> >     
> > 
> >    Information (10804)
> > 
> >    Unable to import \\Test.com\Library\VHDs\Test-disk1.vhdx because of a
> >    syntax error in the file.
> > 
> >     
> 
> If you run qemu-img info on Test-disk1.vhdx, what does it say?
> 
> [Amulya] :   The following is seen with qemu-img info
> # qemu-img info Test-disk1.vhdx
> image: Test-disk1.vhdx
> file format: vhdx
> virtual size: 50G (53687091200 bytes)
> disk size: 3.4G
> cluster_size: 16777216
> 
> The size of our images
> # ls -ltrh
> total 4.8G
> -rw-r--r-- 1 root root 1.4G Dec  9 10:48 Test-disk1.vmdk
> -rw-r--r-- 1 root root  12G Dec 12 04:49 Test-disk1.vhdx
> 
> > 
> >    Please let us know if we missed anything.
> > 
> >     
> > 
> >    Thanks,
> > 
> >    Amulya
> > 
> >     
> >
> 
> [...]
> 
> Jeff

Re: [Qemu-devel] [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

[Jeff Cody]

On Tue, Dec 23, 2014 at 05:07:16AM -0500, Lokesha, Amulya wrote:
> 
> -----Original Message-----
> From: Jeff Cody [mailto:jcody@redhat.com] 
> Sent: Wednesday, December 17, 2014 5:44 PM
> To: Lokesha, Amulya
> Cc: Max Reitz; qemu-devel@nongnu.org; kwolf@redhat.com; stefanha@redhat.com
> Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1
> 
> On Wed, Dec 17, 2014 at 05:46:32AM -0500, Lokesha, Amulya wrote:
> > Hi Max, Jeff,
> > 
> > We were able to get the qemu patch files downloaded from the qemu patch site -  https://patchwork.ozlabs.org and were able to apply the patches successfully without any errors. With the patches applied, we recompiled the qemu and converted the VDK vmdk to vhdx format and uploaded to the SCVMM Server. But it failed again with the syntax error as below:
> > 
> > Information (10804)
> > Unable to import \\TestServer\MSSCVMMLibrary\VHDs\Product-disk1.vhdx because of a syntax error in the file.
> > 
> > Please find my comments inline for your questions
> > 
> > Please let us know if there is anything else you need from us.
> > 
> >
> 
> Amulya,
> 
> I will try to test this on Windows Server, and see if I can reproduce what you are seeing.
> 
> -Jeff
> 
> 
> Hi Jeff,
> 
> Any updates on this? Were you able to test it
> 
> Thanks,
> Amulya
>

Still in process.  I am working to get some MSDN licensing issues
resolved, and then I will be able to test.

> 
> > 
> > 
> > -----Original Message-----
> > From: Jeff Cody [mailto:jcody@redhat.com]
> > Sent: Friday, December 12, 2014 8:48 PM
> > To: Lokesha, Amulya
> > Cc: Max Reitz; qemu-devel@nongnu.org; kwolf@redhat.com; 
> > stefanha@redhat.com
> > Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to 
> > bdrv_has_zero_init_1
> > 
> > On Fri, Dec 12, 2014 at 09:43:16AM -0500, Lokesha, Amulya wrote:
> > >    Hi Max,
> > > 
> > >     
> > 
> > Please reply in-line, it makes it easier to follow technical 
> > discussions - thanks :)
> > 
> > > 
> > >    We applied all the 5 patches from the mail chain I got since the last
> > >    week. Please find attached the patches used by us.
> > > 
> > >    We were unable to apply the patch3 as it failed with the 
> > > following error
> > > 
> > >     
> > > 
> > >    # patch -p1 < patch3
> > > 
> > >    patching file block/vhdx.c
> > > 
> > >    patch: **** malformed patch at line 17:          error_setg_errno(errp,
> > >    EINVAL, "Image size too large; max of 64TB"); @@ -1936,7 +1936,9 @@ static
> > >    QemuOptsList vhdx_create_opts = {
> > > 
> > >
> > 
> > It looks like however you saved the patch file, it was corrupted.
> > Looking at your attached patch 3, it split line 9 across 2 lines.
> > Your patch also has whitespace differences from the patch I sent.
> > 
> > You also attached 5 patches - Why are you using patch 0?  You should only be applying patches 1-4.  This should not be causing any actual issues, however.
> > [Amulya]: First time we applied patches 1 to 4, created VHDX image and deployed to HyperV Server, but we got the same error. Then we took a fresh qemu source and applied patches 0 to 4 and deployed to HyperV and again got the same syntax error.
> > 
> > 
> > Are you using git for your qemu version?  If so, 'git am' is the preferred method of applying the patches - just save each of the patch emails (the whole email should be fine), and run 'git am' on each file.
> > 
> > [Amulya] : No. We don't have a git repository for our team. Could you please let us know how to apply these patches without git. What is the difference in applying the patch directly and modifying the code directly? Does it have any impact?
> > 
> > > 
> > >     
> > > 
> > >    Hence, we manually added the patch3 changes and recompiled the qemu. We
> > >    then used the patched qemu-img to convert  our vmdk image to dynamic VHDX
> > >    format. We found that the image created this time had a considerable
> > >    decrease in its size from 50GB to 12GB.
> > > 
> > 
> > Could you tell me the file size of the VMDK image you were converting?
> > Is it roughly 12GB as well?
> > [Amulya] :  No, the vmdk image which we used for conversion is just 
> > 1.4GB
> > 
> > 
> > >    However, when we deployed it into our SCVMM 2012, the import of the VHDX
> > >    image failed with a "syntax error" as below
> > > 
> > >     
> > > 
> > >    Information (10804)
> > > 
> > >    Unable to import \\Test.com\Library\VHDs\Test-disk1.vhdx because of a
> > >    syntax error in the file.
> > > 
> > >     
> > 
> > If you run qemu-img info on Test-disk1.vhdx, what does it say?
> > 
> > [Amulya] :   The following is seen with qemu-img info
> > # qemu-img info Test-disk1.vhdx
> > image: Test-disk1.vhdx
> > file format: vhdx
> > virtual size: 50G (53687091200 bytes)
> > disk size: 3.4G
> > cluster_size: 16777216
> > 
> > The size of our images
> > # ls -ltrh
> > total 4.8G
> > -rw-r--r-- 1 root root 1.4G Dec  9 10:48 Test-disk1.vmdk
> > -rw-r--r-- 1 root root  12G Dec 12 04:49 Test-disk1.vhdx
> > 
> > > 
> > >    Please let us know if we missed anything.
> > > 
> > >     
> > > 
> > >    Thanks,
> > > 
> > >    Amulya
> > > 
> > >     
> > >
> > 
> > [...]
> > 
> > Jeff

Re: [Qemu-devel] [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

[Lokesha, Amulya]

Comments inline

-----Original Message-----
From: Jeff Cody [mailto:jcody@redhat.com] 
Sent: Tuesday, December 23, 2014 7:33 PM
To: Lokesha, Amulya
Cc: Max Reitz; qemu-devel@nongnu.org; kwolf@redhat.com; stefanha@redhat.com
Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to bdrv_has_zero_init_1

On Tue, Dec 23, 2014 at 05:07:16AM -0500, Lokesha, Amulya wrote:
> 
> -----Original Message-----
> From: Jeff Cody [mailto:jcody@redhat.com]
> Sent: Wednesday, December 17, 2014 5:44 PM
> To: Lokesha, Amulya
> Cc: Max Reitz; qemu-devel@nongnu.org; kwolf@redhat.com; 
> stefanha@redhat.com
> Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to 
> bdrv_has_zero_init_1
> 
> On Wed, Dec 17, 2014 at 05:46:32AM -0500, Lokesha, Amulya wrote:
> > Hi Max, Jeff,
> > 
> > We were able to get the qemu patch files downloaded from the qemu patch site -  https://patchwork.ozlabs.org and were able to apply the patches successfully without any errors. With the patches applied, we recompiled the qemu and converted the VDK vmdk to vhdx format and uploaded to the SCVMM Server. But it failed again with the syntax error as below:
> > 
> > Information (10804)
> > Unable to import \\TestServer\MSSCVMMLibrary\VHDs\Product-disk1.vhdx because of a syntax error in the file.
> > 
> > Please find my comments inline for your questions
> > 
> > Please let us know if there is anything else you need from us.
> > 
> >
> 
> Amulya,
> 
> I will try to test this on Windows Server, and see if I can reproduce what you are seeing.
> 
> -Jeff
> 
> 
> Hi Jeff,
> 
> Any updates on this? Were you able to test it
> 
> Thanks,
> Amulya
>

Still in process.  I am working to get some MSDN licensing issues resolved, and then I will be able to test.


Hi Jeff,

Were you able to get it tested? We are waiting to deliver the patches to our customers. Please let us know
 
Thanks,
Amulya
> > 
> > 
> > -----Original Message-----
> > From: Jeff Cody [mailto:jcody@redhat.com]
> > Sent: Friday, December 12, 2014 8:48 PM
> > To: Lokesha, Amulya
> > Cc: Max Reitz; qemu-devel@nongnu.org; kwolf@redhat.com; 
> > stefanha@redhat.com
> > Subject: Re: [PATCH 4/4] block: vhdx - set .bdrv_has_zero_init to
> > bdrv_has_zero_init_1
> > 
> > On Fri, Dec 12, 2014 at 09:43:16AM -0500, Lokesha, Amulya wrote:
> > >    Hi Max,
> > > 
> > >     
> > 
> > Please reply in-line, it makes it easier to follow technical 
> > discussions - thanks :)
> > 
> > > 
> > >    We applied all the 5 patches from the mail chain I got since the last
> > >    week. Please find attached the patches used by us.
> > > 
> > >    We were unable to apply the patch3 as it failed with the 
> > > following error
> > > 
> > >     
> > > 
> > >    # patch -p1 < patch3
> > > 
> > >    patching file block/vhdx.c
> > > 
> > >    patch: **** malformed patch at line 17:          error_setg_errno(errp,
> > >    EINVAL, "Image size too large; max of 64TB"); @@ -1936,7 +1936,9 @@ static
> > >    QemuOptsList vhdx_create_opts = {
> > > 
> > >
> > 
> > It looks like however you saved the patch file, it was corrupted.
> > Looking at your attached patch 3, it split line 9 across 2 lines.
> > Your patch also has whitespace differences from the patch I sent.
> > 
> > You also attached 5 patches - Why are you using patch 0?  You should only be applying patches 1-4.  This should not be causing any actual issues, however.
> > [Amulya]: First time we applied patches 1 to 4, created VHDX image and deployed to HyperV Server, but we got the same error. Then we took a fresh qemu source and applied patches 0 to 4 and deployed to HyperV and again got the same syntax error.
> > 
> > 
> > Are you using git for your qemu version?  If so, 'git am' is the preferred method of applying the patches - just save each of the patch emails (the whole email should be fine), and run 'git am' on each file.
> > 
> > [Amulya] : No. We don't have a git repository for our team. Could you please let us know how to apply these patches without git. What is the difference in applying the patch directly and modifying the code directly? Does it have any impact?
> > 
> > > 
> > >     
> > > 
> > >    Hence, we manually added the patch3 changes and recompiled the qemu. We
> > >    then used the patched qemu-img to convert  our vmdk image to dynamic VHDX
> > >    format. We found that the image created this time had a considerable
> > >    decrease in its size from 50GB to 12GB.
> > > 
> > 
> > Could you tell me the file size of the VMDK image you were converting?
> > Is it roughly 12GB as well?
> > [Amulya] :  No, the vmdk image which we used for conversion is just 
> > 1.4GB
> > 
> > 
> > >    However, when we deployed it into our SCVMM 2012, the import of the VHDX
> > >    image failed with a "syntax error" as below
> > > 
> > >     
> > > 
> > >    Information (10804)
> > > 
> > >    Unable to import \\Test.com\Library\VHDs\Test-disk1.vhdx because of a
> > >    syntax error in the file.
> > > 
> > >     
> > 
> > If you run qemu-img info on Test-disk1.vhdx, what does it say?
> > 
> > [Amulya] :   The following is seen with qemu-img info
> > # qemu-img info Test-disk1.vhdx
> > image: Test-disk1.vhdx
> > file format: vhdx
> > virtual size: 50G (53687091200 bytes) disk size: 3.4G
> > cluster_size: 16777216
> > 
> > The size of our images
> > # ls -ltrh
> > total 4.8G
> > -rw-r--r-- 1 root root 1.4G Dec  9 10:48 Test-disk1.vmdk
> > -rw-r--r-- 1 root root  12G Dec 12 04:49 Test-disk1.vhdx
> > 
> > > 
> > >    Please let us know if we missed anything.
> > > 
> > >     
> > > 
> > >    Thanks,
> > > 
> > >    Amulya
> > > 
> > >     
> > >
> > 
> > [...]
> > 
> > Jeff