| Message ID | CAMnT0-ZGnMAA_AKLOHXPW39OHncCcHVPuBw=eFGHcZzWv+WJ4Q@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Dec 4, 2012 at 2:56 AM, Robert Hubbard <hubbardmeister@gmail.com> 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
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 <1075252@bugs.launchpad.net> wrote: > On Tue, Dec 4, 2012 at 2:56 AM, Robert Hubbard <hubbardmeister@gmail.com> 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 >
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;
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. On Thu, Nov 8, 2012 at 12:15 AM, Stefan Hajnoczi <1075252@bugs.launchpad.net > wrote: > On Tue, Nov 6, 2012 at 11:38 PM, Robert Hubbard > <hubbardmeister@gmail.com> 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 qemu-devel@nongnu.org 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 >