[Qemu-devel] [PATCH] vpc: use current_size field for XenServer VHD images

[Qemu-devel] [PATCH] vpc: use current_size field for XenServer VHD images

[Stefan Hajnoczi]

The vpc driver has two methods of determining virtual disk size.  The
correct one to use depends on the software that generated the image
file.  Add the XenServer creator_app signature so that image size is
correctly detected for those images.

Reported-by: Grant Wu <grantwwu@gmail.com>
Reported-by: Spencer Baugh <sbaugh@catern.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vpc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/vpc.c b/block/vpc.c
index 8435205..7517f75 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -298,6 +298,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
      *      'qem2'  :  current_size     QEMU (uses current_size)
      *      'win '  :  current_size     Hyper-V
      *      'd2v '  :  current_size     Disk2vhd
+     *      'tap\0' :  current_size     XenServer
      *
      *  The user can override the table values via drive options, however
      *  even with an override we will still use current_size for images
@@ -305,7 +306,8 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
      */
     use_chs = (!!strncmp(footer->creator_app, "win ", 4) &&
                !!strncmp(footer->creator_app, "qem2", 4) &&
-               !!strncmp(footer->creator_app, "d2v ", 4)) || s->force_use_chs;
+               !!strncmp(footer->creator_app, "d2v ", 4) &&
+               !!memcmp(footer->creator_app, "tap", 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) /
-- 
2.5.0

Re: [Qemu-devel] [PATCH] vpc: use current_size field for XenServer VHD images

[Grant Wu]

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

Side note:  After manually patching tap\0 to qem2, Spencer and I were still
unable to get qemu-img info working with the image.  Unsure if this is
related to the patch or not.

Thanks,

Grant Wu
grantwwu@gmail.com

On Sat, Mar 19, 2016 at 3:42 PM, Stefan Hajnoczi <stefanha@redhat.com>
wrote:

> The vpc driver has two methods of determining virtual disk size.  The
> correct one to use depends on the software that generated the image
> file.  Add the XenServer creator_app signature so that image size is
> correctly detected for those images.
>
> Reported-by: Grant Wu <grantwwu@gmail.com>
> Reported-by: Spencer Baugh <sbaugh@catern.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/vpc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/block/vpc.c b/block/vpc.c
> index 8435205..7517f75 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -298,6 +298,7 @@ static int vpc_open(BlockDriverState *bs, QDict
> *options, int flags,
>       *      'qem2'  :  current_size     QEMU (uses current_size)
>       *      'win '  :  current_size     Hyper-V
>       *      'd2v '  :  current_size     Disk2vhd
> +     *      'tap\0' :  current_size     XenServer
>       *
>       *  The user can override the table values via drive options, however
>       *  even with an override we will still use current_size for images
> @@ -305,7 +306,8 @@ static int vpc_open(BlockDriverState *bs, QDict
> *options, int flags,
>       */
>      use_chs = (!!strncmp(footer->creator_app, "win ", 4) &&
>                 !!strncmp(footer->creator_app, "qem2", 4) &&
> -               !!strncmp(footer->creator_app, "d2v ", 4)) ||
> s->force_use_chs;
> +               !!strncmp(footer->creator_app, "d2v ", 4) &&
> +               !!memcmp(footer->creator_app, "tap", 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) /
> --
> 2.5.0
>
>

[-- Attachment #2: Type: text/html, Size: 3011 bytes --]

Re: [Qemu-devel] [PATCH] vpc: use current_size field for XenServer VHD images

[Stefan Hajnoczi]

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

On Sun, Mar 20, 2016 at 01:33:44AM -0400, Grant Wu wrote:
> Side note:  After manually patching tap\0 to qem2, Spencer and I were still
> unable to get qemu-img info working with the image.  Unsure if this is
> related to the patch or not.

What output did you get from "qemu-img info /dev/dm-1"?

Did qemu-nbd work?

Stefan

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

Re: [Qemu-devel] [PATCH] vpc: use current_size field for XenServer VHD images

[Spencer Baugh]

Stefan Hajnoczi <stefanha@redhat.com> writes:
> What output did you get from "qemu-img info /dev/dm-1"?

After the patching:

root@storage-4:~# hexdump -C -n 512 /dev/vgstorage/tartanfsbackup
00000000  63 6f 6e 65 63 74 69 78  00 00 00 02 00 01 00 00 |conectix........|
00000010  00 00 00 00 00 00 02 00  14 10 4b 1f 71 65 6d 32 |..........K.qem2|
00000020  00 01 00 03 00 00 00 00  00 00 01 c4 4f 40 00 00 |............O@..|
00000030  00 00 01 f4 00 00 00 00  ff ff 10 ff 00 00 00 03 |................|
00000040  ff ff ed 81 6d 0d 68 bd  9c fd 4d 65 a3 17 c7 6c |....m.h...Me...l|
00000050  06 a6 aa bf 00 00 00 00  00 00 00 00 00 00 00 00 |................|
00000060  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
*
00000200
root@storage-4:~# qemu-img info /dev/vgstorage/tartanfsbackup
block-vpc: The header checksum of '/dev/vgstorage/tartanfsbackup' is incorrect.
qemu-img: Could not open '/dev/vgstorage/tartanfsbackup': Could not open '/dev/vgstorage/tartanfsbackup': Invalid argument

> Did qemu-nbd work?

root@storage-4:~# qemu-nbd -c /dev/nbd1 /dev/vgstorage/tartanfsbackup
block-vpc: The header checksum of '/dev/vgstorage/tartanfsbackup' is incorrect.
qemu-nbd: Failed to blk_new_open '/dev/vgstorage/tartanfsbackup': Could not open '/dev/vgstorage/tartanfsbackup': Invalid argument

Re: [Qemu-devel] [PATCH] vpc: use current_size field for XenServer VHD images

[Stefan Hajnoczi]

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

On Mon, Mar 21, 2016 at 02:37:46PM -0400, Spencer Baugh wrote:
> Stefan Hajnoczi <stefanha@redhat.com> writes:
> > What output did you get from "qemu-img info /dev/dm-1"?
> 
> After the patching:
> 
> root@storage-4:~# hexdump -C -n 512 /dev/vgstorage/tartanfsbackup
> 00000000  63 6f 6e 65 63 74 69 78  00 00 00 02 00 01 00 00 |conectix........|
> 00000010  00 00 00 00 00 00 02 00  14 10 4b 1f 71 65 6d 32 |..........K.qem2|
> 00000020  00 01 00 03 00 00 00 00  00 00 01 c4 4f 40 00 00 |............O@..|
> 00000030  00 00 01 f4 00 00 00 00  ff ff 10 ff 00 00 00 03 |................|
> 00000040  ff ff ed 81 6d 0d 68 bd  9c fd 4d 65 a3 17 c7 6c |....m.h...Me...l|
> 00000050  06 a6 aa bf 00 00 00 00  00 00 00 00 00 00 00 00 |................|
> 00000060  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00 |................|
> *
> 00000200
> root@storage-4:~# qemu-img info /dev/vgstorage/tartanfsbackup
> block-vpc: The header checksum of '/dev/vgstorage/tartanfsbackup' is incorrect.

This is a warning message that does not cause opening the file to fail.
It makes sense since the header was modified without setting the new
checksum value.

> qemu-img: Could not open '/dev/vgstorage/tartanfsbackup': Could not open '/dev/vgstorage/tartanfsbackup': Invalid argument

Please also post the output of "hexdump -C -n 512 -s 512
/dev/vgstorage/tartanfsbackup".  This is another header structure and it
gets parsed while opening the file.

Thanks,
Stefan

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

Re: [Qemu-devel] [PATCH] vpc: use current_size field for XenServer VHD images

[Grant Wu]

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

root@storage-4:~# hexdump -C -n 512 -s 512 /dev/vgstorage/tartanfsbackup
00000200  63 78 73 70 61 72 73 65  ff ff ff ff ff ff ff ff
 |cxsparse........|
00000210  00 00 00 00 00 00 06 00  00 01 00 00 00 10 00 00
 |................|
00000220  00 20 00 00 ff ff f4 67  00 00 00 00 00 00 00 00  |.
.....g........|
00000230  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
 |................|
*
00000400


Thanks,

Grant Wu
grantwwu@gmail.com

[-- Attachment #2: Type: text/html, Size: 747 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH] vpc: use current_size field for XenServer VHD images

[Jeff Cody]

On Sat, Mar 19, 2016 at 07:42:43PM +0000, Stefan Hajnoczi wrote:
> The vpc driver has two methods of determining virtual disk size.  The
> correct one to use depends on the software that generated the image
> file.  Add the XenServer creator_app signature so that image size is
> correctly detected for those images.
> 
> Reported-by: Grant Wu <grantwwu@gmail.com>
> Reported-by: Spencer Baugh <sbaugh@catern.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/vpc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/block/vpc.c b/block/vpc.c
> index 8435205..7517f75 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -298,6 +298,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
>       *      'qem2'  :  current_size     QEMU (uses current_size)
>       *      'win '  :  current_size     Hyper-V
>       *      'd2v '  :  current_size     Disk2vhd
> +     *      'tap\0' :  current_size     XenServer

I just tried out XenConverter, and it set the creator_app field to
"CTXS".  We should probably add a check for that, too.

>       *
>       *  The user can override the table values via drive options, however
>       *  even with an override we will still use current_size for images
> @@ -305,7 +306,8 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
>       */
>      use_chs = (!!strncmp(footer->creator_app, "win ", 4) &&
>                 !!strncmp(footer->creator_app, "qem2", 4) &&
> -               !!strncmp(footer->creator_app, "d2v ", 4)) || s->force_use_chs;
> +               !!strncmp(footer->creator_app, "d2v ", 4) &&
> +               !!memcmp(footer->creator_app, "tap", 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) /
> -- 
> 2.5.0
> 
>