[Qemu-devel] [PATCH v3 0/4] VHD/VPC format compatibility

[Qemu-devel] [PATCH v3 0/4] VHD/VPC format compatibility

[Jeff Cody]

Changes from v3:

Patch 2: Added a sample image & tests for Disk2vhd
Patch 3: When using force_size, set the CHS geometry to max
         (as Disk2vhd does), to maximize backward compatibility (thanks Peter)
Patch 4: Updated test results due to the above changes in patch 3

Changes from v2:

Patches 2,4: Just use qemu-io instead of a qemu instance (thanks Kevin, Max)
Patch 3: Use "qem2" for the creator app field, when forcing the size (thanks Kevin)

This is a long-standing issue that has come up many times, and has had
several different patches posted to fix it.  Virtual PC, and Hyper-V
calculate the disk geometry differently for VHD, leading to compatibility
issues.

We want to fix these compatibility problems, however we want to make sure
we do not break backwards compatibility.

There are two areas of compatibility addressed:

* Reading images (Patch 1)
* Creating images (Patch 3)

Please see the commit messages in Patches 1,3 for details.

Jeff Cody (4):
  block/vpc: choose size calculation method based on creator_app field
  block/vpc: tests for auto-detecting VPC and Hyper-V VHD images
  block/vpc: give option to force the current_size field in .bdrv_create
  block/vpc: add tests for image creation force_size parameter

 block/vpc.c                                        | 129 ++++++++++++++--
 tests/qemu-iotests/146                             | 165 +++++++++++++++++++++
 tests/qemu-iotests/146.out                         |  70 +++++++++
 tests/qemu-iotests/group                           |   1 +
 .../sample_images/d2v-zerofilled.vhd.bz2           | Bin 0 -> 1021 bytes
 .../sample_images/hyperv2012r2-dynamic.vhd.bz2     | Bin 0 -> 214 bytes
 .../sample_images/virtualpc-dynamic.vhd.bz2        | Bin 0 -> 212 bytes
 7 files changed, 353 insertions(+), 12 deletions(-)
 create mode 100755 tests/qemu-iotests/146
 create mode 100644 tests/qemu-iotests/146.out
 create mode 100644 tests/qemu-iotests/sample_images/d2v-zerofilled.vhd.bz2
 create mode 100644 tests/qemu-iotests/sample_images/hyperv2012r2-dynamic.vhd.bz2
 create mode 100644 tests/qemu-iotests/sample_images/virtualpc-dynamic.vhd.bz2

-- 
1.9.3

[Qemu-devel] [PATCH v3 1/4] block/vpc: choose size calculation method based on creator_app field

[Jeff Cody]

The VHD file format is used by both Virtual PC, and Hyper-V.  However,
how the virtual disk size is calculated varies between the two.

Virtual PC uses the CHS drive parameters to determine the drive size.
Hyper-V, on the other hand, uses the current_size field in the footer
when determining image size.

This is problematic for a few reasons:

* VHD images from Hyper-V, using CHS calculations, will likely be
  trunctated.

* If we just rely always on current_size, then QEMU may have data
  compatibility issues with Virtual PC (we may write too much data
  into a VHD file to be used by Virtual PC, for instance).

* Existing VHD images created by QEMU have used the CHS calculations,
  except for images exceeding the 127GB limit.  We want to remain
  compatible with our own generated images.

Luckily, the VHD specification defines a 'Creator App' field, that is
used to indicate what software created the VHD file.

This patch does two things:

    1. Uses the 'Creator App' field to help determine how to calculate
       size, and

    2. Adds a VPC format option 'force_size_calc', so that the user can
       override the 'Creator App' auto-detection, in case there exist
       VHD images with unknown or contradictory 'Creator App' entries.

N.B.: We currently use the maximum CHS value as an indication to use the
current_size field.  This patch does not change that, even with the
'force_size_calc' option.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/vpc.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 82 insertions(+), 5 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index f504536..54a38a7 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -128,6 +128,8 @@ typedef struct BDRVVPCState {
 
     uint32_t block_size;
     uint32_t bitmap_size;
+    bool force_use_chs;
+    bool force_use_sz;
 
 #ifdef CACHE
     uint8_t *pageentry_u8;
@@ -140,6 +142,22 @@ typedef struct BDRVVPCState {
     Error *migration_blocker;
 } BDRVVPCState;
 
+#define VPC_OPT_SIZE_CALC "force_size_calc"
+static QemuOptsList vpc_runtime_opts = {
+    .name = "vpc-runtime-opts",
+    .head = QTAILQ_HEAD_INITIALIZER(vpc_runtime_opts.head),
+    .desc = {
+        {
+            .name = VPC_OPT_SIZE_CALC,
+            .type = QEMU_OPT_STRING,
+            .help = "Force disk size calculation to use either CHS geometry, "
+                    "or use the disk current_size specified in the VHD footer. "
+                    "{chs, current_size}"
+        },
+        { /* end of list */ }
+    }
+};
+
 static uint32_t vpc_checksum(uint8_t* buf, size_t size)
 {
     uint32_t res = 0;
@@ -159,6 +177,25 @@ static int vpc_probe(const uint8_t *buf, int buf_size, const char *filename)
     return 0;
 }
 
+static void vpc_parse_options(BlockDriverState *bs, QemuOpts *opts,
+                              Error **errp)
+{
+    BDRVVPCState *s = bs->opaque;
+    const char *size_calc;
+
+    size_calc = qemu_opt_get(opts, VPC_OPT_SIZE_CALC);
+
+    if (!size_calc) {
+       /* no override, use autodetect only */
+    } else if (!strcmp(size_calc, "current_size")) {
+        s->force_use_sz = true;
+    } else if (!strcmp(size_calc, "chs")) {
+        s->force_use_chs = true;
+    } else {
+        error_setg(errp, "Invalid size calculation mode: '%s'", size_calc);
+    }
+}
+
 static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
                     Error **errp)
 {
@@ -166,6 +203,9 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
     int i;
     VHDFooter *footer;
     VHDDynDiskHeader *dyndisk_header;
+    QemuOpts *opts = NULL;
+    Error *local_err = NULL;
+    bool use_chs;
     uint8_t buf[HEADER_SIZE];
     uint32_t checksum;
     uint64_t computed_size;
@@ -173,6 +213,21 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
     int disk_type = VHD_DYNAMIC;
     int ret;
 
+    opts = qemu_opts_create(&vpc_runtime_opts, NULL, 0, &error_abort);
+    qemu_opts_absorb_qdict(opts, options, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    vpc_parse_options(bs, opts, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+        goto fail;
+    }
+
     ret = bdrv_pread(bs->file->bs, 0, s->footer_buf, HEADER_SIZE);
     if (ret < 0) {
         goto fail;
@@ -218,12 +273,34 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
     bs->total_sectors = (int64_t)
         be16_to_cpu(footer->cyls) * footer->heads * footer->secs_per_cyl;
 
-    /* Images that have exactly the maximum geometry are probably bigger and
-     * would be truncated if we adhered to the geometry for them. Rely on
-     * footer->current_size for them. */
-    if (bs->total_sectors == VHD_MAX_GEOMETRY) {
+    /* Microsoft Virtual PC and Microsoft Hyper-V produce and read
+     * VHD image sizes differently.  VPC will rely on CHS geometry,
+     * while Hyper-V and disk2vhd use the size specified in the footer.
+     *
+     * We use a couple of approaches to try and determine the correct method:
+     * look at the Creator App field, and look for images that have CHS
+     * geometry that is the maximum value.
+     *
+     * If the CHS geometry is the maximum CHS geometry, then we assume that
+     * the size is the footer->current_size to avoid truncation.  Otherwise,
+     * we follow the table based on footer->creator_app:
+     *
+     *  Known creator apps:
+     *      'vpc '  :  CHS              Virtual PC (uses disk geometry)
+     *      'qemu'  :  CHS              QEMU (uses disk geometry)
+     *      'win '  :  current_size     Hyper-V
+     *      'd2v '  :  current_size     Disk2vhd
+     *
+     *  The user can override the table values via drive options, however
+     *  even with an override we will still use current_size for images
+     *  that have CHS geometry of the maximum size.
+     */
+    use_chs = (!!strncmp(footer->creator_app, "win ", 4) &&
+               !!strncmp(footer->creator_app, "d2v ", 4)) || s->force_use_chs;
+
+    if (!use_chs || bs->total_sectors == VHD_MAX_GEOMETRY || s->force_use_sz) {
         bs->total_sectors = be64_to_cpu(footer->current_size) /
-                            BDRV_SECTOR_SIZE;
+                                        BDRV_SECTOR_SIZE;
     }
 
     /* Allow a maximum disk size of approximately 2 TB */
-- 
1.9.3

[Qemu-devel] [PATCH v3 2/4] block/vpc: tests for auto-detecting VPC and Hyper-V VHD images

[Jeff Cody]

This tests auto-detection, and overrides, of VHD image sizes created
by Virtual PC, Hyper-V, and Disk2vhd.

This adds three sample images:

hyperv2012r2-dynamic.vhd.bz2 - dynamic VHD image created with Hyper-V
virtualpc-dynamic.vhd.bz2    - dynamic VHD image created with Virtual PC
d2v-zerofilled.vhd.bz2       - dynamic VHD image created with Disk2vhd

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 tests/qemu-iotests/146                             | 114 +++++++++++++++++++++
 tests/qemu-iotests/146.out                         |  38 +++++++
 tests/qemu-iotests/group                           |   1 +
 .../sample_images/d2v-zerofilled.vhd.bz2           | Bin 0 -> 1021 bytes
 .../sample_images/hyperv2012r2-dynamic.vhd.bz2     | Bin 0 -> 214 bytes
 .../sample_images/virtualpc-dynamic.vhd.bz2        | Bin 0 -> 212 bytes
 6 files changed, 153 insertions(+)
 create mode 100755 tests/qemu-iotests/146
 create mode 100644 tests/qemu-iotests/146.out
 create mode 100644 tests/qemu-iotests/sample_images/d2v-zerofilled.vhd.bz2
 create mode 100644 tests/qemu-iotests/sample_images/hyperv2012r2-dynamic.vhd.bz2
 create mode 100644 tests/qemu-iotests/sample_images/virtualpc-dynamic.vhd.bz2

diff --git a/tests/qemu-iotests/146 b/tests/qemu-iotests/146
new file mode 100755
index 0000000..4cbe1f4
--- /dev/null
+++ b/tests/qemu-iotests/146
@@ -0,0 +1,114 @@
+#!/bin/bash
+#
+# Test VHD image format creator detection and override
+#
+# Copyright (C) 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=jcody@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1    # failure is the default!
+
+_cleanup()
+{
+    _cleanup_qemu
+    _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_supported_fmt vpc
+_supported_proto file
+_supported_os Linux
+
+
+qemu_comm_method="monitor"
+silent=
+
+echo
+echo === Testing VPC Autodetect ===
+echo
+_use_sample_img virtualpc-dynamic.vhd.bz2
+
+${QEMU_IO} -c "open -o driver=vpc ${TEST_IMG}" -c 'map'
+
+echo
+echo === Testing VPC with current_size force ===
+echo
+
+${QEMU_IO} -c "open -o driver=vpc,force_size_calc=current_size ${TEST_IMG}" -c 'map'
+
+echo
+echo === Testing VPC with chs force ===
+echo
+
+${QEMU_IO} -c "open -o driver=vpc,force_size_calc=chs ${TEST_IMG}" -c 'map'
+
+_cleanup_test_img
+
+echo
+echo === Testing Hyper-V Autodetect ===
+echo
+_use_sample_img hyperv2012r2-dynamic.vhd.bz2
+
+${QEMU_IO} -c "open -o driver=vpc ${TEST_IMG}" -c 'map'
+
+echo
+echo === Testing Hyper-V with current_size force ===
+echo
+
+${QEMU_IO} -c "open -o driver=vpc,force_size_calc=current_size ${TEST_IMG}" -c 'map'
+
+echo
+echo === Testing Hyper-V with chs force ===
+echo
+
+${QEMU_IO} -c "open -o driver=vpc,force_size_calc=chs ${TEST_IMG}" -c 'map'
+
+_cleanup_test_img
+
+echo
+echo === Testing d2v Autodetect ===
+echo
+_use_sample_img d2v-zerofilled.vhd.bz2
+
+${QEMU_IO} -c "open -o driver=vpc ${TEST_IMG}" -c 'map'
+
+echo
+echo === Testing d2v with current_size force ===
+echo
+
+${QEMU_IO} -c "open -o driver=vpc,force_size_calc=current_size ${TEST_IMG}" -c 'map'
+
+echo
+echo === Testing d2v with chs force ===
+echo
+
+${QEMU_IO} -c "open -o driver=vpc,force_size_calc=chs ${TEST_IMG}" -c 'map'
+
+
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/146.out b/tests/qemu-iotests/146.out
new file mode 100644
index 0000000..478ad4e
--- /dev/null
+++ b/tests/qemu-iotests/146.out
@@ -0,0 +1,38 @@
+QA output created by 146
+
+=== Testing VPC Autodetect ===
+
+[                       0]  266334240/ 266334240 sectors not allocated at offset 0 bytes (0)
+
+=== Testing VPC with current_size force ===
+
+[                       0]  266338304/ 266338304 sectors not allocated at offset 0 bytes (0)
+
+=== Testing VPC with chs force ===
+
+[                       0]  266334240/ 266334240 sectors not allocated at offset 0 bytes (0)
+
+=== Testing Hyper-V Autodetect ===
+
+[                       0]  266338304/ 266338304 sectors not allocated at offset 0 bytes (0)
+
+=== Testing Hyper-V with current_size force ===
+
+[                       0]  266338304/ 266338304 sectors not allocated at offset 0 bytes (0)
+
+=== Testing Hyper-V with chs force ===
+
+[                       0]  266334240/ 266334240 sectors not allocated at offset 0 bytes (0)
+
+=== Testing d2v Autodetect ===
+
+[                       0]   514560/  514560 sectors     allocated at offset 0 bytes (1)
+
+=== Testing d2v with current_size force ===
+
+[                       0]   514560/  514560 sectors     allocated at offset 0 bytes (1)
+
+=== Testing d2v with chs force ===
+
+[                       0]   514560/  514560 sectors     allocated at offset 0 bytes (1)
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 47fd40c..1211149 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -148,3 +148,4 @@
 143 auto quick
 144 rw auto quick
 145 auto quick
+146 auto quick
diff --git a/tests/qemu-iotests/sample_images/d2v-zerofilled.vhd.bz2 b/tests/qemu-iotests/sample_images/d2v-zerofilled.vhd.bz2
new file mode 100644
index 0000000000000000000000000000000000000000..f12cb9203aa3271af1d610d5313a3aa3ffad1a6e
GIT binary patch
literal 1021
zcmV<Z0|NX)T4*^jL0KkKS)^6Dg9UxffB*mg|NsC0|NsC0|NsC0|Nr0szyL@rL{dNj
zKoCKaNGiYq+}XQ@h%_{4#2PfoguyUOCPM)R0S1^Rj3x-cOaPiNO&JX|$P7TiN)l#j
zo~Mc$e*CDLRM{S=H9ba60h&e=0ER}585s>R0Kl057?_45N2K&kFhwSy{iam`qxDZv
z00000Gynhq0000000000HlQMuspUUPey9PViI4zfG|8rhKmY&%qednmXblEH&>Ca_
zWYB5qc4VLNl7#$VQwjl0Xaxii5h#j6sEUm1_%YBbC@P|3{PF>V2$@`#TQ8H$W@$Lg
zunSNdj{OKq5`>`%N>Y?123z>=l}RDjq{>_xT55WUad`6}nE`}o-m$~F`x_^ZphAfn
zO|8An)TxuGQmI<a?fwoC%Rq51UcrklHhi3|8n$oEnBmK(Z#nuNzJCG?D19*^#*ZRQ
zsdFXG>YdlwK7|@o=XTJk-QQNXhmVo7%?O?8@<|vm>5o(%$o0Y^psI;F6c2CaKv)BP
zpa2GWE&u_5&=y+8ic%G?Z+DQFvhR)xYNaV(A1s>;Fu<Higp?%&zb}j8NhE+60&yo~
zl1VWMB$7>EJ-BVQ(@hddCK)7>No9svWtM-S007|t0002M+$MmK002S&004>ZC;2vf
zS?+VWYEqSP%#rBgy6^ew%^6lpWoug2R<+8rT;7C8(p!1yl4xYQ>#oR-rAC0IDNZ=!
zDN0h5r6@uWgdq*Zz53)tL`CKd(?^+^nVIm2h=`2|NoAH<Wuwg5Y37<~rkZLLXaXV%
zsFDJg@F}K%2E0-yWFkZqube415-0q{1!S2`WY*k$m;l3TfCB&k01yBofB`|8XcUq<
z3KXN-6%jO?2~w&|sdAW8Ddj$5g0f7evX8l0NC3DgzyW{&00;mPKmef4Gzv)_1qxB^
z3W%Cc1gTXfRJlwkl=7c3L0KkKSup1(Isk`G-~hk?00aOCpa4*2ngt||f`usdg+xs!
z0#vG#DqN-%%6U(ipsbTAtoHiYihx-}Z~$Nc00IC6Pyi@1O#+ffK|+*!LZT*<fhtu=
zl`c~XWjv?MP*zEl)_32og8(Ym-}iq2FbP0N1ONaC0H6zs;ZOhoA`1Z&fB-;bfC8)l
zEwU0)Mn;AZ(8;4rn^V#}P(@Lw(WaUMMj*+c8ZatUAOHXW00V{4hvfp;g%Cv%J467e
zs-^%OtJ6eDAdv%=2^MHXK_UgKl*EQHB!mJ&0N}uqZ1o5;Z>&B(Yi-HR$=LqW7aHq>
r>VAzO3jW3zM|uPx5)cG{1Xu(02t5UF)d5g$5901frwS4U1zi6?E&-p(

literal 0
HcmV?d00001

diff --git a/tests/qemu-iotests/sample_images/hyperv2012r2-dynamic.vhd.bz2 b/tests/qemu-iotests/sample_images/hyperv2012r2-dynamic.vhd.bz2
new file mode 100644
index 0000000000000000000000000000000000000000..bfeccf7b9f2596f119bb35d7e650a29f8e6f17ef
GIT binary patch
literal 214
zcmV;{04e`MT4*^jL0KkKS<7>@aR2~j|Ns3EPyxUK5D)+a0HCTX+~fcS06+jB01!Yx
z0Eqx02&u3Dw$>7=8feLprXhl8!7=I+01-+c00006fB?`)si-s!00E((0ieW+9LW!<
z$bjJjeWx)D<T{8{s#%a3;RgedXsj@R;4uLp(v6U6!JLR7If;pnq2PRkq5G&e^eU7=
z35-UB5Lqny#MI_+)pEkpb6HV1dm&Nqu26F+C?ceBqzwrql6{7LB~SnfiDX4pUB1y(
QS^tZ<BAh5lWx3iofM<DCwg3PC

literal 0
HcmV?d00001

diff --git a/tests/qemu-iotests/sample_images/virtualpc-dynamic.vhd.bz2 b/tests/qemu-iotests/sample_images/virtualpc-dynamic.vhd.bz2
new file mode 100644
index 0000000000000000000000000000000000000000..783be3c8f068b6eb96d777b1290999899c6d5a25
GIT binary patch
literal 212
zcmV;_04x7OT4*^jL0KkKSvGGv761TIfB*deNCChg7yy6*0e~tg-CzblAV2^hfFMKw
z0Du4#2&u3DxmZf6lLW-cfC+$`Nr{sJzg0#i0x*mt000ERBuN@*003w<paFzwJPuT%
zF-0#dMpg_?GDOA)NV4&A2w9s5<DjH~=?f6cV8!+%m~lAMoz6$2>FnA{B7q@4*a;xX
z24}+|;{k09^toeVF+M+y8lDaaN{P?GVI@TYSp=OSB$8;1RzUy&6`?NT5C8<(0ssS^
Oh1`)&6eJCs&V_&uMoJF=

literal 0
HcmV?d00001

-- 
1.9.3

[Qemu-devel] [PATCH v3 3/4] block/vpc: give option to force the current_size field in .bdrv_create

[Jeff Cody]

When QEMU creates a VHD image, it goes by the original spec,
calculating the current_size based on the nearest CHS geometry (with an
exception for disks > 127GB).

Apparently, Azure will only allow images that are sized to the nearest
MB, and the current_size as calculated from CHS cannot guarantee that.

Allow QEMU to create images similar to how Hyper-V creates images, by
setting current_size to the specified virtual disk size.  This
introduces an option, force_size, to be passed to the vpc format during
image creation, e.g.:

    qemu-img convert -f raw -o force_size -O vpc test.img test.vhd

When using the "force_size" option, the creator app field used by
QEMU will be "qem2" instead of "qemu", to indicate the difference.
In light of this, we also add parsing of the "qem2" field during
vpc_open.

Bug reference: https://bugs.launchpad.net/qemu/+bug/1490611

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/vpc.c | 42 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index 54a38a7..318e6d6 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -46,8 +46,14 @@ enum vhd_type {
 // Seconds since Jan 1, 2000 0:00:00 (UTC)
 #define VHD_TIMESTAMP_BASE 946684800
 
+#define VHD_CHS_MAX_C   65535LL
+#define VHD_CHS_MAX_H   16
+#define VHD_CHS_MAX_S   255
+
 #define VHD_MAX_SECTORS       (65535LL * 255 * 255)
-#define VHD_MAX_GEOMETRY      (65535LL *  16 * 255)
+#define VHD_MAX_GEOMETRY      (VHD_CHS_MAX_C * VHD_CHS_MAX_H * VHD_CHS_MAX_S)
+
+#define VPC_OPT_FORCE_SIZE "force_size"
 
 // always big-endian
 typedef struct vhd_footer {
@@ -288,6 +294,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
      *  Known creator apps:
      *      'vpc '  :  CHS              Virtual PC (uses disk geometry)
      *      'qemu'  :  CHS              QEMU (uses disk geometry)
+     *      'qem2'  :  current_size     QEMU (uses current_size)
      *      'win '  :  current_size     Hyper-V
      *      'd2v '  :  current_size     Disk2vhd
      *
@@ -296,6 +303,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
      *  that have CHS geometry of the maximum size.
      */
     use_chs = (!!strncmp(footer->creator_app, "win ", 4) &&
+               !!strncmp(footer->creator_app, "qem2", 4) &&
                !!strncmp(footer->creator_app, "d2v ", 4)) || s->force_use_chs;
 
     if (!use_chs || bs->total_sectors == VHD_MAX_GEOMETRY || s->force_use_sz) {
@@ -850,6 +858,7 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
     int64_t total_size;
     int disk_type;
     int ret = -EIO;
+    bool force_size;
     Error *local_err = NULL;
     BlockDriverState *bs = NULL;
 
@@ -870,6 +879,8 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
         disk_type = VHD_DYNAMIC;
     }
 
+    force_size = qemu_opt_get_bool_del(opts, VPC_OPT_FORCE_SIZE, false);
+
     ret = bdrv_create_file(filename, opts, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
@@ -887,13 +898,20 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
      * sectors requested until we get enough (or fail). This ensures that
      * qemu-img convert doesn't truncate images, but rather rounds up.
      *
-     * If the image size can't be represented by a spec conform CHS geometry,
+     * If the image size can't be represented by a spec conformant CHS geometry,
      * we set the geometry to 65535 x 16 x 255 (CxHxS) sectors and use
      * the image size from the VHD footer to calculate total_sectors.
      */
-    total_sectors = MIN(VHD_MAX_GEOMETRY, total_size / BDRV_SECTOR_SIZE);
-    for (i = 0; total_sectors > (int64_t)cyls * heads * secs_per_cyl; i++) {
-        calculate_geometry(total_sectors + i, &cyls, &heads, &secs_per_cyl);
+    if (force_size) {
+        /* This will force the use of total_size for sector count, below */
+        cyls         = VHD_CHS_MAX_C;
+        heads        = VHD_CHS_MAX_H;
+        secs_per_cyl = VHD_CHS_MAX_S;
+    } else {
+        total_sectors = MIN(VHD_MAX_GEOMETRY, total_size / BDRV_SECTOR_SIZE);
+        for (i = 0; total_sectors > (int64_t)cyls * heads * secs_per_cyl; i++) {
+            calculate_geometry(total_sectors + i, &cyls, &heads, &secs_per_cyl);
+        }
     }
 
     if ((int64_t)cyls * heads * secs_per_cyl == VHD_MAX_GEOMETRY) {
@@ -912,8 +930,11 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
     memset(buf, 0, 1024);
 
     memcpy(footer->creator, "conectix", 8);
-    /* TODO Check if "qemu" creator_app is ok for VPC */
-    memcpy(footer->creator_app, "qemu", 4);
+    if (force_size) {
+        memcpy(footer->creator_app, "qem2", 4);
+    } else {
+        memcpy(footer->creator_app, "qemu", 4);
+    }
     memcpy(footer->creator_os, "Wi2k", 4);
 
     footer->features = cpu_to_be32(0x02);
@@ -994,6 +1015,13 @@ static QemuOptsList vpc_create_opts = {
                 "Type of virtual hard disk format. Supported formats are "
                 "{dynamic (default) | fixed} "
         },
+        {
+            .name = VPC_OPT_FORCE_SIZE,
+            .type = QEMU_OPT_BOOL,
+            .help = "Force disk size calculation to use the actual size "
+                    "specified, rather than using the nearest CHS-based "
+                    "calculation"
+        },
         { /* end of list */ }
     }
 };
-- 
1.9.3

[Qemu-devel] [PATCH v3 4/4] block/vpc: add tests for image creation force_size parameter

[Jeff Cody]

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 tests/qemu-iotests/146     | 51 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/146.out | 32 +++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+)

diff --git a/tests/qemu-iotests/146 b/tests/qemu-iotests/146
index 4cbe1f4..043711b 100755
--- a/tests/qemu-iotests/146
+++ b/tests/qemu-iotests/146
@@ -108,6 +108,57 @@ echo
 
 ${QEMU_IO} -c "open -o driver=vpc,force_size_calc=chs ${TEST_IMG}" -c 'map'
 
+_cleanup_test_img
+
+echo
+echo === Testing Image create, default ===
+echo
+
+TEST_IMG="${TEST_DIR}/vpc-create-test.vpc"
+
+_make_test_img 4G
+
+echo
+echo === Read created image, default opts ====
+echo
+
+${QEMU_IO} -c "open -o driver=vpc ${TEST_IMG}" -c 'map'
+
+echo
+echo === Read created image, force_size_calc=chs ====
+echo
+
+${QEMU_IO} -c "open -o driver=vpc,force_size_calc=chs ${TEST_IMG}" -c 'map'
+
+echo
+echo === Read created image, force_size_calc=current_size ====
+echo
+
+${QEMU_IO} -c "open -o driver=vpc,force_size_calc=current_size ${TEST_IMG}" -c 'map'
+
+echo
+echo === Testing Image create, force_size ===
+echo
+
+_make_test_img -o force_size 4G
+
+echo
+echo === Read created image, default opts ====
+echo
+
+${QEMU_IO} -c "open -o driver=vpc ${TEST_IMG}" -c 'map'
+
+echo
+echo === Read created image, force_size_calc=chs ====
+echo
+
+${QEMU_IO} -c "open -o driver=vpc,force_size_calc=chs ${TEST_IMG}" -c 'map'
+
+echo
+echo === Read created image, force_size_calc=current_size ====
+echo
+
+${QEMU_IO} -c "open -o driver=vpc,force_size_calc=current_size ${TEST_IMG}" -c 'map'
 
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/146.out b/tests/qemu-iotests/146.out
index 478ad4e..4f334d8 100644
--- a/tests/qemu-iotests/146.out
+++ b/tests/qemu-iotests/146.out
@@ -35,4 +35,36 @@ QA output created by 146
 === Testing d2v with chs force ===
 
 [                       0]   514560/  514560 sectors     allocated at offset 0 bytes (1)
+
+=== Testing Image create, default ===
+
+Formatting 'TEST_DIR/IMGFMT-create-test.IMGFMT', fmt=IMGFMT size=4294967296
+
+=== Read created image, default opts ====
+
+[                       0]  8389584/ 8389584 sectors not allocated at offset 0 bytes (0)
+
+=== Read created image, force_size_calc=chs ====
+
+[                       0]  8389584/ 8389584 sectors not allocated at offset 0 bytes (0)
+
+=== Read created image, force_size_calc=current_size ====
+
+[                       0]  8389584/ 8389584 sectors not allocated at offset 0 bytes (0)
+
+=== Testing Image create, force_size ===
+
+Formatting 'TEST_DIR/IMGFMT-create-test.IMGFMT', fmt=IMGFMT size=4294967296 force_size=on
+
+=== Read created image, default opts ====
+
+[                       0]  8388608/ 8388608 sectors not allocated at offset 0 bytes (0)
+
+=== Read created image, force_size_calc=chs ====
+
+[                       0]  8388608/ 8388608 sectors not allocated at offset 0 bytes (0)
+
+=== Read created image, force_size_calc=current_size ====
+
+[                       0]  8388608/ 8388608 sectors not allocated at offset 0 bytes (0)
 *** done
-- 
1.9.3

Re: [Qemu-devel] [PATCH v3 0/4] VHD/VPC format compatibility

[Kevin Wolf]

Am 25.02.2016 um 18:27 hat Jeff Cody geschrieben:
> Changes from v3:
> 
> Patch 2: Added a sample image & tests for Disk2vhd
> Patch 3: When using force_size, set the CHS geometry to max
>          (as Disk2vhd does), to maximize backward compatibility (thanks Peter)
> Patch 4: Updated test results due to the above changes in patch 3
> 
> Changes from v2:
> 
> Patches 2,4: Just use qemu-io instead of a qemu instance (thanks Kevin, Max)
> Patch 3: Use "qem2" for the creator app field, when forcing the size (thanks Kevin)
> 
> This is a long-standing issue that has come up many times, and has had
> several different patches posted to fix it.  Virtual PC, and Hyper-V
> calculate the disk geometry differently for VHD, leading to compatibility
> issues.
> 
> We want to fix these compatibility problems, however we want to make sure
> we do not break backwards compatibility.
> 
> There are two areas of compatibility addressed:
> 
> * Reading images (Patch 1)
> * Creating images (Patch 3)
> 
> Please see the commit messages in Patches 1,3 for details.

Thanks, applied to the block branch.

Kevin