[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 00/14] qemu-img map: Allow driver to return f
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH v4 00/14] qemu-img map: Allow driver to return file of the allocated block |
Date: |
Tue, 5 Jan 2016 16:09:54 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Mon, 01/04 22:02, Max Reitz wrote:
> On 24.12.2015 06:50, Fam Zheng wrote:
> > v4: Rebase and resend, adding Eric's and Stefan's reviewed-by.
> >
> > Fix one typo in patch 13.
> >
> > Drop previous patch 14 for a later rework because it is not a hard
> > requirement, but it is pending on Eric's QAPI-to-JSON visitor series:
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg03929.html
> >
> > v3: Add Eric's rev-by in patches 6, 7, 13, 14.
> > 12: New, split out from the previous 13.
> > 12->13: Refactor "entry_mergable" from imp_map().
> > Don't mess the merge conditions. [Paolo]
> > Address Eric's comments:
> > - Check has_foo before using foo.
> > - Remove blank line between comments and definition in schema.
> > - Use PRId64 instead of %ld.
> > - Merge short lines.
> >
> > v2: Add Eric's rev-by in patches 2, 4, 5.
> > 01: Refering -> referring in commit message. [Eric]
> > Recurse to "file" for sensible "zero" flag. [Paolo]
> > 12: New. Make MapEntry a QAPI struct. [Paolo, Markus]
> >
> > Original cover letter
> > ---------------------
> >
> > I stumbled upon this when looking at external bitmap formats.
> >
> > Current "qemu-img map" command only displays filename if the data is
> > allocated
> > in bs (bs->file) itself, or in the backing chain. Otherwise, it displays an
> > unfriendly error message:
> >
> > $ qemu-img create -f vmdk -o subformat=monolithicFlat /tmp/test.vmdk 1G
> >
> > $ qemu-img map /tmp/test.vmdk
> > Offset Length Mapped to File
> > qemu-img: File contains external, encrypted or compressed clusters.
> >
> > This can be improved. This series extends the .bdrv_co_get_block_status
> > callback, to let block driver return the BDS of file; then updates all
> > driver
> > to implement it; and lastly, it changes qemu-img to use this information in
> > "map" command:
> >
> >
> > $ qemu-img map /tmp/test.vmdk
> > Offset Length Mapped to File
> > 0 0x40000000 0 /tmp/test-flat.vmdk
> >
> > $ qemu-img map --output json /tmp/test.vmdk
> > [{"length": 1073741824, "start": 0, "zero": false, "offset": 0,
> > "depth": 0,
> > "file": "/tmp/test-flat.vmdk", "data": true}
> > ]
> >
> > Fam Zheng (14):
> > block: Add "file" output parameter to block status query functions
> > qcow: Assign bs->file->bs to file in qcow_co_get_block_status
> > qcow2: Assign bs->file->bs to file in qcow2_co_get_block_status
>
> Minor comment: I'd swap these two patches (2 and 3). Patch 1 breaks test
> 102, patch 3 fixes it again. It would be better to break it for as short
> a time as possible.
>
> Alternatively, in order not to break 102 at all, patch 1 would need to
> leave the "if (bs->file &&" part of bdrv_co_get_block_status()
> (@@ -1544,13 +1550,14 @@) as-is and change it only after the format
> block drivers set *file.
Yes, this is better.
Fam