Re: [Qemu-devel] [Bug 1075252] Re: qemu-img cannot read VMDK4 file

[Robert Hubbard]

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 <address@hidden>
wrote:
> On Tue, Dec 4, 2012 at 2:56 AM, Robert Hubbard <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
>
> --
> 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
>

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
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