qemu-img cannot read VMDK4 file

[Robert Hubbard]

Unable to read any vmdk4 type files. Goal was to convert to a qcow2, this worked after emitting code.

OS is Centos linux 2.6.32. I pulled the latest git tree down for qemu to see if this was resolved, it is not.

Starting program: /home/rhubbard/QEMU/qemu/qemu-img info -f vmdk /root/Juniper/beta1candidate-07122012-disk1.vmdk

There seems a mismatch with the l1_backup_tble_offset. this is now a uint64 type. The value is actually "-512" because of this and this causes the code check at line 418 in vmdk.c to erroneously think there is a backup table. This causes vmdk open to fail.
and message
qemu failed to open ....

from debug;
gdb) x/4x &(s->l1_backup_table_offset)
0xa61cd0: 0xfffffe00 0xffffffff 0x00a62770 0x00000000

(gdb) p *s
$1 = {hd = 0x0, l1_table_offset = 0, l1_backup_table_offset = -512, l1_table = 0xa62770,
  l1_backup_table = 0x0, l1_size = 64, l1_entry_sectors = 65536, l2_size = 512, l2_cache = 0x0,
  l2_cache_offsets = {0 <repeats 16 times>}, l2_cache_counts = {0 <repeats 16 times>},
  cluster_sectors = 128, parent_cid = 4294967295}

typedef struct BDRVVmdkState {
    BlockDriverState *hd;
    int64_t l1_table_offset; <==== ??? - what should this be , don't know what the actual layout on the vmdk spec says , is this a 64bit / 8 byte field ?

    int64_t l1_backup_table_offset;
    uint32_t *l1_table;
    uint32_t *l1_backup_table;
    unsigned int l1_size;
    uint32_t l1_entry_sectors;

    unsigned int l2_size;

from vmdk.c
    /*!!! if (s->l1_backup_table_offset) {
        s->l1_backup_table = qemu_malloc(l1_size);
        if (bdrv_pread(bs->file, s->l1_backup_table_offset, s->l1_backup_table, l1_size) != l1_size)
            goto fail;

Breaks here..

Don't know the correct fix , thanks for help.

[Robert Hubbard (hubbardmeister)]

Duplicate - Same issue related to header/footer - When does the code fix show up in the git release train ?

bug 907063 .
Read the VMDK 5.X release doc on format for VMDK. The files im using are created from Vsphere 5.X and are using a VMDK verion of 3.
Virtual Disk Format 5.0 - VMware
www.vmware.com/support/.../vmdk_50_technote.pdf?src=vm...
File Format: PDF/Adobe Acrobat - Quick View
Dec 20, 2011 – The document describes the virtual machine disk (VMDK) format and ... VMware designed the VMDK (virtual machine disk) format to mimic the ...

Fields look consistent with description in the VMDK 5.X doc and used that struct as referene(hacked patch attached for reference
)
First pass at Header:
gdb) p/x header
$1 = {version = 0x3, flags = 0x30001, capacity = 0x400000, granularity = 0x80, desc_offset = 0x1,
  desc_size = 0x1, num_gtes_per_gte = 0x200, rgd_offset = 0x0, gd_offset = 0xffffffffffffffff,
  grain_offset = 0x80, uncleanshutdown = 0x0, singlendlinechar = 0xa, nonendlinechar = 0x20,
  doublendlinechar1 = 0xd, doublendlinechar2 = 0xa, compressAlgorithm = 0x1, pad = {
    0x0 <repeats 433 times>}}

Now the footer:
(gdb) p/x header
$2 = {version = 0x3, flags = 0x30001, capacity = 0x400000, granularity = 0x80, desc_offset = 0x1,
  desc_size = 0x1, num_gtes_per_gte = 0x200, rgd_offset = 0x0, gd_offset = 0x68304,
  grain_offset = 0x80, uncleanshutdown = 0x0, singlendlinechar = 0xa, nonendlinechar = 0x20,
  doublendlinechar1 = 0xd, doublendlinechar2 = 0xa, compressAlgorithm = 0x1, pad = {
    0x0 <repeats 433 times>}}

[Stefan Hajnoczi (stefanha)]

On Tue, Nov 6, 2012 at 11:38 PM, Robert Hubbard
<email address hidden> wrote:
> Duplicate - Same issue related to header/footer - When does the code fix
> show up in the git release train ?
>
> bug 907063 .

The bug reporter did not follow the steps for submitting a patch that I posted:

http://wiki.qemu.org/Contribute/SubmitAPatch

Therefore the patch never reached the <email address hidden> mailing
list and was not merged.

If you'd like to submit your patch please follow the steps on the wiki
page above.

Thanks,
Stefan

[Robert Hubbard (hubbardmeister)]

Hi Stefan,
Will post later this week/end based on your requirements.

Regards.
Rob.

On Thu, Nov 8, 2012 at 12:15 AM, Stefan Hajnoczi <<email address hidden>
> wrote:

> On Tue, Nov 6, 2012 at 11:38 PM, Robert Hubbard
> <email address hidden> wrote:
> > Duplicate - Same issue related to header/footer - When does the code fix
> > show up in the git release train ?
> >
> > bug 907063 .
>
> The bug reporter did not follow the steps for submitting a patch that I
> posted:
>
> http://wiki.qemu.org/Contribute/SubmitAPatch
>
> Therefore the patch never reached the <email address hidden> mailing
> list and was not merged.
>
> If you'd like to submit your patch please follow the steps on the wiki
> page above.
>
> Thanks,
> Stefan
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1075252
>
> Title:
> qemu-img cannot read VMDK4 file
>
> Status in QEMU:
> New
>
> Bug description:
> Unable to read any vmdk4 type files. Goal was to convert to a qcow2,
> this worked after emitting code.
>
> OS is Centos linux 2.6.32. I pulled the latest git tree down for qemu
> to see if this was resolved, it is not.
>
> Starting program: /home/rhubbard/QEMU/qemu/qemu-img info -f vmdk
> /root/Juniper/beta1candidate-07122012-disk1.vmdk
>
>
> There seems a mismatch with the l1_backup_tble_offset. this is now a
> uint64 type. The value is actually "-512" because of this and this causes
> the code check at line 418 in vmdk.c to erroneously think there is a
> backup table. This causes vmdk open to fail.
> and message
> qemu failed to open ....
>
>
> from debug;
> gdb) x/4x &(s->l1_backup_table_offset)
> 0xa61cd0: 0xfffffe00 0xffffffff 0x00a62770 0x00000000
>
> (gdb) p *s
> $1 = {hd = 0x0, l1_table_offset = 0, l1_backup_table_offset = -512,
> l1_table = 0xa62770,
> l1_backup_table = 0x0, l1_size = 64, l1_entry_sectors = 65536, l2_size
> = 512, l2_cache = 0x0,
> l2_cache_offsets = {0 <repeats 16 times>}, l2_cache_counts = {0
> <repeats 16 times>},
> cluster_sectors = 128, parent_cid = 4294967295}
>
> typedef struct BDRVVmdkState {
> BlockDriverState *hd;
> int64_t l1_table_offset;
> <==== ??? - what should this be , don't know what the actual layout on the
> vmdk spec says , is this a 64bit / 8 byte field ?
>
> int64_t l1_backup_table_offset;
> uint32_t *l1_table;
> uint32_t *l1_backup_table;
> unsigned int l1_size;
> uint32_t l1_entry_sectors;
>
> unsigned int l2_size;
>
> from vmdk.c
> /*!!! if (s->l1_backup_table_offset) {
> s->l1_backup_table = qemu_malloc(l1_size);
> if (bdrv_pread(bs->file, s->l1_backup_table_offset,
> s->l1_backup_table, l1_size) != l1_size)
> goto fail;
>
> Breaks here..
>
> Don't know the correct fix , thanks for help.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/qemu/+bug/1075252/+subscriptions
>

--
Regards

Rob Hubbard

[Robert Hubbard (hubbardmeister)]

Attached Diff to resolve the "open issue" and to also to begin to cater for the fact that an image copy of streamoptimzed format is not supported. This will be added in later fix pending acceptance here.

[Robert Hubbard (hubbardmeister)]

Hi Stefan,
I have uploaded a patch - I am failing miserably to get any output from git
patch!!!!!! ... :^( . the code is structured to addres the fact that
convert will not work today, needs lots of work to do this. This would be
next effort.

i have a sub branch ...
root@rhubbard qemu]# git status
# On branch vmdk.c
# Untracked files:
# (use "git add <file>..." to include in what will be committed)
#
# myoutput
# rhubbard-patch-fix-vmdk

did a commit .. and here is git show..., don't know what to do next... i
have used cvs et all.. don't know why this is not working 4 Me...

commit 971ad8e198fa386214d8154904facbce2719fb06
Author: root <root@rhubbard.(none)>
Date: Mon Dec 3 17:11:28 2012 -0800

    Bug#1075252 by Robert Hubbard: Fixed problems with StreammOptimized
open, needs more work for convert.

diff --git a/block.h b/block.h
index 78ecfac..1f55f6d 100644
--- a/block.h
+++ b/block.h
@@ -14,6 +14,8 @@ typedef struct BlockDriverInfo {
     int cluster_size;
     /* offset at which the VM state can be saved (0 if not possible) */
     int64_t vm_state_offset;
+ int drv_type;
+ const char *drv_work;
 } BlockDriverInfo;

On Thu, Nov 8, 2012 at 12:15 AM, Stefan Hajnoczi <<email address hidden>
> wrote:

> On Tue, Nov 6, 2012 at 11:38 PM, Robert Hubbard
> <email address hidden> wrote:
> > Duplicate - Same issue related to header/footer - When does the code fix
> > show up in the git release train ?
> >
> > bug 907063 .
>
> The bug reporter did not follow the steps for submitting a patch that I
> posted:
>
> http://wiki.qemu.org/Contribute/SubmitAPatch
>
> Therefore the patch never reached the <email address hidden> mailing
> list and was not merged.
>
> If you'd like to submit your patch please follow the steps on the wiki
> page above.
>
> Thanks,
> Stefan
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1075252
>
> Title:
> qemu-img cannot read VMDK4 file
>
> Status in QEMU:
> New
>
> Bug description:
> Unable to read any vmdk4 type files. Goal was to convert to a qcow2,
> this worked after emitting code.
>
> OS is Centos linux 2.6.32. I pulled the latest git tree down for qemu
> to see if this was resolved, it is not.
>
> Starting program: /home/rhubbard/QEMU/qemu/qemu-img info -f vmdk
> /root/Juniper/beta1candidate-07122012-disk1.vmdk
>
>
> There seems a mismatch with the l1_backup_tble_offset. this is now a
> uint64 type. The value is actually "-512" because of this and this causes
> the code check at line 418 in vmdk.c to erroneously think there is a
> backup table. This causes vmdk open to fail.
> and message
> qemu failed to open ....
>
>
> from debug;
> gdb) x/4x &(s->l1_backup_table_offset)
> 0xa61cd0: 0xfffffe00 0xffffffff 0x00a62770 0x00000000
>
> (gdb) p *s
> $1 = {hd = 0x0, l1_table_offset = 0, l1_backup_table_offset = -512,
> l1_table = 0xa62770,
> l1_backup_table = 0x0, l1_size = 64, l1_entry_sectors = 65536, l2_size
> = 512, l2_cache = 0x0,
> l2_cache_offsets = {0 <repeats 16 times>}, l2_cache_counts = {0
> <repeats 16 times>},
> cluster_sectors = 128, parent...

[Stefan Hajnoczi (stefanha)]

On Tue, Dec 4, 2012 at 2:56 AM, Robert Hubbard <email address hidden> wrote:
> I have uploaded a patch - I am failing miserably to get any output from git
> patch!!!!!! ... :^( . the code is structured to addres the fact that
> convert will not work today, needs lots of work to do this. This would be
> next effort.

Hi Rob,
git-patch(1) is used to apply patches - it's not the command for
producing patch emails.

Try git-format-patch(1). Here is a short post I found on creating a
commit and using git-format-patch(1):

http://andrewprice.me.uk/weblog/entry/generating-patch-emails-with-git

There are several git tutorials that cover much more and will help you
get familiar. If you want to learn git I recommend:

http://git-scm.com/book
http://www-cs-students.stanford.edu/~blynn/gitmagic/

But remember you don't need to use git - some people use other tools
or simply diff(1). You just need to send patches to the mailing list
as described at http://wiki.qemu.org/Contribute/SubmitAPatch.

If you have doubts about how to structure a patch series, try peeking
at what other people have sent to the mailing list.

I took a quick look at the patch you uploaded:

It helps review to split changes up into multiple patches, one patch
for each logical code change. For example, renaming a struct field
also involves changing code that uses the field because the name has
changed. This is a good candidate for a patch - just the struct field
rename and updates to code that uses the old name. If you think at
this level of code change your diff can be split into several
independent changes which are easier to review.

That said, renaming fields or changing whitespace should only be done
when necessary. It introduces noise in the form of extra work to
review - the compiled object code probably doesn't change and the
behavior of the program won't either. So it's best to only make
changes that are necessary or that provide clear value. (I'm not an
expert on block/vmdk.c, for example, so any non-essential changes
basically mean extra work for me to check whether they are okay or
not.)

Going back to the original bug: can you confirm that qemu.git/master
qemu-img correctly displays the VMDK file you have? Fam Zheng
indicated the bug you originally hit has already been fixed.

Please send patches or questions for new VMDK changes that are
unrelated to this bug report directly to qemu-devel. Your patch seems
to be beyond the scope of this bug report and adds some additional
qemu-img info output.

Hope this helps. If you want real-time discussion, try asking on
#qemu on irc.oftc.net where a lot of QEMU developers hang out.

Stefan

[Robert Hubbard (hubbardmeister)]

Hi Stefan,
Good exercise, I just pulled the latest branch. The block/vmdk.c has been
completely revised and includes the correct code for vmdk4 support now for
the vmdk compressed stream optimized. I also "missed" for the git-patch
the fact i needed "master" in the command, this worked and thanks for
pointers.

Branch is 1.3.50

Regards

Rob.

On Tuesday, December 4, 2012, Stefan Hajnoczi <email address hidden>
wrote:
> On Tue, Dec 4, 2012 at 2:56 AM, Robert Hubbard <email address hidden>
wrote:
>> I have uploaded a patch - I am failing miserably to get any output from
git
>> patch!!!!!! ... :^( . the code is structured to addres the fact that
>> convert will not work today, needs lots of work to do this. This would be
>> next effort.
>
> Hi Rob,
> git-patch(1) is used to apply patches - it's not the command for
> producing patch emails.
>
> Try git-format-patch(1). Here is a short post I found on creating a
> commit and using git-format-patch(1):
>
> http://andrewprice.me.uk/weblog/entry/generating-patch-emails-with-git
>
> There are several git tutorials that cover much more and will help you
> get familiar. If you want to learn git I recommend:
>
> http://git-scm.com/book
> http://www-cs-students.stanford.edu/~blynn/gitmagic/
>
> But remember you don't need to use git - some people use other tools
> or simply diff(1). You just need to send patches to the mailing list
> as described at http://wiki.qemu.org/Contribute/SubmitAPatch.
>
> If you have doubts about how to structure a patch series, try peeking
> at what other people have sent to the mailing list.
>
> I took a quick look at the patch you uploaded:
>
> It helps review to split changes up into multiple patches, one patch
> for each logical code change. For example, renaming a struct field
> also involves changing code that uses the field because the name has
> changed. This is a good candidate for a patch - just the struct field
> rename and updates to code that uses the old name. If you think at
> this level of code change your diff can be split into several
> independent changes which are easier to review.
>
> That said, renaming fields or changing whitespace should only be done
> when necessary. It introduces noise in the form of extra work to
> review - the compiled object code probably doesn't change and the
> behavior of the program won't either. So it's best to only make
> changes that are necessary or that provide clear value. (I'm not an
> expert on block/vmdk.c, for example, so any non-essential changes
> basically mean extra work for me to check whether they are okay or
> not.)
>
> Going back to the original bug: can you confirm that qemu.git/master
> qemu-img correctly displays the VMDK file you have? Fam Zheng
> indicated the bug you originally hit has already been fixed.
>
> Please send patches or questions for new VMDK changes that are
> unrelated to this bug report directly to qemu-devel. Your patch seems
> to be beyond the scope of this bug report and adds some additional
> qemu-img info output.
>
> Hope this helps. If you want real-time discussion, try asking on
> #qemu on irc.oftc.net where a lot of QEMU developers hang out.
>
> Stefan
>
>...

[Robert Hubbard (hubbardmeister)]

This issue is resolved by 1.3.50 release , this bug should be closed now.