Series comparison

-New patch
+[PATCH v4 0/2] vpc: Read images exported from Azure correctly
+Changes since v3:
+- Split the change into two patches [Philippe Mathieu-Daude].
+It was found that 'qemu-nbd' is not able to work with some disk images
+exported from Azure as it uses a currently unknown 'wa\0\0' 'creator app'
+signature. QEMU currently supports two methods for determining the image
+size: CHS and 'current_size' and the list of known 'creator app's is used
+to decide between the two. Invert the logic in QEMU and make 'current_size'
+the default as it seems that VPC and old QEMU are the only two legacy apps
+where preferring CHS makes sense.
+Vitaly Kuznetsov (2):
+  vpc: Split off vpc_ignore_current_size() helper
+  vpc: Read images exported from Azure correctly
+ block/vpc.c | 65 ++++++++++++++++++++++++++++-------------------------
+file changed, 35 insertions(+), 30 deletions(-)
+--
+.47.0

-[PATCH v3] vpc: Read images exported from Azure correctly
+[PATCH v4 1/2] vpc: Split off vpc_ignore_current_size() helper
-It was found that 'qemu-nbd' is not able to work with some disk images
+In preparation to making changes to the logic deciding whether CHS or
-exported from Azure. Looking at the 512b footer (which contains VPC
+'current_size' need to be used in determining the image size, split off
-metadata):
+vpc_ignore_current_size() helper.
-00000000  63 6f 6e 65 63 74 69 78  00 00 00 02 00 01 00 00  |conectix........|
+No functional change intended.
 00000010  ff ff ff ff ff ff ff ff  2e c7 9b 96 77 61 00 00  |............wa..|
 00000020  00 07 00 00 57 69 32 6b  00 00 00 01 40 00 00 00  |....Wi2k....@...|
 00000030  00 00 00 01 40 00 00 00  28 a2 10 3f 00 00 00 02  |....@...(..?....|
 00000040  ff ff e7 47 8c 54 df 94  bd 35 71 4c 94 5f e5 44  |...G.T...5qL._.D|
 00000050  44 53 92 1a 00 00 00 00  00 00 00 00 00 00 00 00  |DS..............|
 00000060  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
 we can see that Azure uses a different 'Creator application' --
 'wa\0\0' (offset 0x1c, likely reads as 'Windows Azure') and QEMU uses this
 field to determine how it can get image size. Apparently, Azure uses 'new'
 method, just like Hyper-V.
 Overall, it seems that only VPC and old QEMUs need to be ignored as all new
 creator apps seem to have reliable current_size. Invert the logic and make
 'current_size' method the default to avoid adding every new creator app to
 the list.
 Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
 ---
-Changes since v1/v2: invert the logic and make 'vpc' and 'qemu' use CHS
+ block/vpc.c | 67 +++++++++++++++++++++++++++++------------------------
-while defaulting to current_size.
+file changed, 37 insertions(+), 30 deletions(-)
 ---
  block/vpc.c | 65 ++++++++++++++++++++++++++++-------------------------
 file changed, 35 insertions(+), 30 deletions(-)
 diff --git a/block/vpc.c b/block/vpc.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/vpc.c
 +++ b/block/vpc.c
 ...
 + *
 + * 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:
 + *
-+ *  Currently known creator apps:
++ *  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
 + *      'tap\0' :  current_size     XenServer
 + *      'CTXS'  :  current_size     XenConverter
-+ *      'wa\0\0':  current_size     Azure
 + *
 + *  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.
 + */
 +static bool vpc_ignore_current_size(VHDFooter *footer)
 +{
-+    return !strncmp(footer->creator_app, "vpc ", 4) ||
++    return !!strncmp(footer->creator_app, "win ", 4) &&
-+        !strncmp(footer->creator_app, "qemu", 4);
++           !!strncmp(footer->creator_app, "qem2", 4) &&
 +           !!strncmp(footer->creator_app, "d2v ", 4) &&
 +           !!strncmp(footer->creator_app, "CTXS", 4) &&
 +           !!memcmp(footer->creator_app, "tap", 4));
 +}
 +
  static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
                      Error **errp)
  {
 ...
 -    use_chs = (!!strncmp(footer->creator_app, "win ", 4) &&
 -               !!strncmp(footer->creator_app, "qem2", 4) &&
 -               !!strncmp(footer->creator_app, "d2v ", 4) &&
 -               !!strncmp(footer->creator_app, "CTXS", 4) &&
 -               !!memcmp(footer->creator_app, "tap", 4)) || s->force_use_chs;
-+    /* Use CHS or current_size to determine the image size */
++    /* Use CHS or current_size to determine the image size. */
 +    use_chs = vpc_ignore_current_size(footer) || 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) /
 --
 .47.0

-New patch
+[PATCH v4 2/2] vpc: Read images exported from Azure correctly
+It was found that 'qemu-nbd' is not able to work with some disk images
+exported from Azure. Looking at the 512b footer (which contains VPC
+metadata):
+00000000  63 6f 6e 65 63 74 69 78  00 00 00 02 00 01 00 00  |conectix........|
+00000010  ff ff ff ff ff ff ff ff  2e c7 9b 96 77 61 00 00  |............wa..|
+00000020  00 07 00 00 57 69 32 6b  00 00 00 01 40 00 00 00  |....Wi2k....@...|
+00000030  00 00 00 01 40 00 00 00  28 a2 10 3f 00 00 00 02  |....@...(..?....|
+00000040  ff ff e7 47 8c 54 df 94  bd 35 71 4c 94 5f e5 44  |...G.T...5qL._.D|
+00000050  44 53 92 1a 00 00 00 00  00 00 00 00 00 00 00 00  |DS..............|
+00000060  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+we can see that Azure uses a different 'Creator application' --
+'wa\0\0' (offset 0x1c, likely reads as 'Windows Azure') and QEMU uses this
+field to determine how it can get image size. Apparently, Azure uses 'new'
+method, just like Hyper-V.
+Overall, it seems that only VPC and old QEMUs need to be ignored as all new
+creator apps seem to have reliable current_size. Invert the logic and make
+'current_size' method the default to avoid adding every new creator app to
+the list.
+Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
+---
+ block/vpc.c | 8 +++-----
+file changed, 3 insertions(+), 5 deletions(-)
+diff --git a/block/vpc.c b/block/vpc.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/vpc.c
++++ b/block/vpc.c
+@@ -XXX,XX +XXX,XX @@ static void vpc_parse_options(BlockDriverState *bs, QemuOpts *opts,
+  *      'd2v '  :  current_size     Disk2vhd
+  *      'tap\0' :  current_size     XenServer
+  *      'CTXS'  :  current_size     XenConverter
++ *      'wa\0\0':  current_size     Azure
+  *
+  *  The user can override the table values via drive options, however
+  *  even with an override we will still use current_size for images
+@@ -XXX,XX +XXX,XX @@ static void vpc_parse_options(BlockDriverState *bs, QemuOpts *opts,
+  */
+ static bool vpc_ignore_current_size(VHDFooter *footer)
+ {
+-    return !!strncmp(footer->creator_app, "win ", 4) &&
+-           !!strncmp(footer->creator_app, "qem2", 4) &&
+-           !!strncmp(footer->creator_app, "d2v ", 4) &&
+-           !!strncmp(footer->creator_app, "CTXS", 4) &&
+-           !!memcmp(footer->creator_app, "tap", 4));
++    return !strncmp(footer->creator_app, "vpc ", 4) ||
++           !strncmp(footer->creator_app, "qemu", 4);
+ }
+ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
+--
+.47.0

It was found that 'qemu-nbd' is not able to work with some disk images
exported from Azure. Looking at the 512b footer (which contains VPC
metadata):

00000000  63 6f 6e 65 63 74 69 78  00 00 00 02 00 01 00 00  |conectix........|
00000010  ff ff ff ff ff ff ff ff  2e c7 9b 96 77 61 00 00  |............wa..|
00000020  00 07 00 00 57 69 32 6b  00 00 00 01 40 00 00 00  |....Wi2k....@...|
00000030  00 00 00 01 40 00 00 00  28 a2 10 3f 00 00 00 02  |....@...(..?....|
00000040  ff ff e7 47 8c 54 df 94  bd 35 71 4c 94 5f e5 44  |...G.T...5qL._.D|
00000050  44 53 92 1a 00 00 00 00  00 00 00 00 00 00 00 00  |DS..............|
00000060  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|

we can see that Azure uses a different 'Creator application' --
'wa\0\0' (offset 0x1c, likely reads as 'Windows Azure') and QEMU uses this
field to determine how it can get image size. Apparently, Azure uses 'new'
method, just like Hyper-V.

Overall, it seems that only VPC and old QEMUs need to be ignored as all new
creator apps seem to have reliable current_size. Invert the logic and make
'current_size' method the default to avoid adding every new creator app to
the list.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes since v1/v2: invert the logic and make 'vpc' and 'qemu' use CHS
while defaulting to current_size.
---
 block/vpc.c | 65 ++++++++++++++++++++++++++++-------------------------
 1 file changed, 35 insertions(+), 30 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index XXXXXXX..XXXXXXX 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -XXX,XX +XXX,XX @@ static void vpc_parse_options(BlockDriverState *bs, QemuOpts *opts,
     }
 }
 
+/*
+ * 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:
+ *
+ *  Currently 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
+ *      'tap\0' :  current_size     XenServer
+ *      'CTXS'  :  current_size     XenConverter
+ *      'wa\0\0':  current_size     Azure
+ *
+ *  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.
+ */
+static bool vpc_ignore_current_size(VHDFooter *footer)
+{
+    return !strncmp(footer->creator_app, "vpc ", 4) ||
+        !strncmp(footer->creator_app, "qemu", 4);
+}
+
 static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
                     Error **errp)
 {
@@ -XXX,XX +XXX,XX @@ 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;
 
-    /* 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)
-     *      'qem2'  :  current_size     QEMU (uses current_size)
-     *      'win '  :  current_size     Hyper-V
-     *      'd2v '  :  current_size     Disk2vhd
-     *      'tap\0' :  current_size     XenServer
-     *      'CTXS'  :  current_size     XenConverter
-     *
-     *  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, "qem2", 4) &&
-               !!strncmp(footer->creator_app, "d2v ", 4) &&
-               !!strncmp(footer->creator_app, "CTXS", 4) &&
-               !!memcmp(footer->creator_app, "tap", 4)) || s->force_use_chs;
+    /* Use CHS or current_size to determine the image size */
+    use_chs = vpc_ignore_current_size(footer) || 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) /
-- 
2.47.0

In preparation to making changes to the logic deciding whether CHS or
'current_size' need to be used in determining the image size, split off
vpc_ignore_current_size() helper.

No functional change intended.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 block/vpc.c | 67 +++++++++++++++++++++++++++++------------------------
 1 file changed, 37 insertions(+), 30 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index XXXXXXX..XXXXXXX 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -XXX,XX +XXX,XX @@ static void vpc_parse_options(BlockDriverState *bs, QemuOpts *opts,
     }
 }
 
+/*
+ * 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)
+ *      'qem2'  :  current_size     QEMU (uses current_size)
+ *      'win '  :  current_size     Hyper-V
+ *      'd2v '  :  current_size     Disk2vhd
+ *      'tap\0' :  current_size     XenServer
+ *      'CTXS'  :  current_size     XenConverter
+ *
+ *  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.
+ */
+static bool vpc_ignore_current_size(VHDFooter *footer)
+{
+    return !!strncmp(footer->creator_app, "win ", 4) &&
+           !!strncmp(footer->creator_app, "qem2", 4) &&
+           !!strncmp(footer->creator_app, "d2v ", 4) &&
+           !!strncmp(footer->creator_app, "CTXS", 4) &&
+           !!memcmp(footer->creator_app, "tap", 4));
+}
+
 static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
                     Error **errp)
 {
@@ -XXX,XX +XXX,XX @@ 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;
 
-    /* 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)
-     *      'qem2'  :  current_size     QEMU (uses current_size)
-     *      'win '  :  current_size     Hyper-V
-     *      'd2v '  :  current_size     Disk2vhd
-     *      'tap\0' :  current_size     XenServer
-     *      'CTXS'  :  current_size     XenConverter
-     *
-     *  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, "qem2", 4) &&
-               !!strncmp(footer->creator_app, "d2v ", 4) &&
-               !!strncmp(footer->creator_app, "CTXS", 4) &&
-               !!memcmp(footer->creator_app, "tap", 4)) || s->force_use_chs;
+    /* Use CHS or current_size to determine the image size. */
+    use_chs = vpc_ignore_current_size(footer) || 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) /
-- 
2.47.0

It was found that 'qemu-nbd' is not able to work with some disk images
exported from Azure. Looking at the 512b footer (which contains VPC
metadata):

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 block/vpc.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index XXXXXXX..XXXXXXX 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -XXX,XX +XXX,XX @@ static void vpc_parse_options(BlockDriverState *bs, QemuOpts *opts,
  *      'd2v '  :  current_size     Disk2vhd
  *      'tap\0' :  current_size     XenServer
  *      'CTXS'  :  current_size     XenConverter
+ *      'wa\0\0':  current_size     Azure
  *
  *  The user can override the table values via drive options, however
  *  even with an override we will still use current_size for images
@@ -XXX,XX +XXX,XX @@ static void vpc_parse_options(BlockDriverState *bs, QemuOpts *opts,
  */
 static bool vpc_ignore_current_size(VHDFooter *footer)
 {
-    return !!strncmp(footer->creator_app, "win ", 4) &&
-           !!strncmp(footer->creator_app, "qem2", 4) &&
-           !!strncmp(footer->creator_app, "d2v ", 4) &&
-           !!strncmp(footer->creator_app, "CTXS", 4) &&
-           !!memcmp(footer->creator_app, "tap", 4));
+    return !strncmp(footer->creator_app, "vpc ", 4) ||
+           !strncmp(footer->creator_app, "qemu", 4);
 }
 
 static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
-- 
2.47.0