Re: [Qemu-devel] [PATCH v8 00/19] VHDX log replay and write support, .bdrv_create()

[Kevin Wolf]

Am 01.11.2013 um 16:32 hat Jeff Cody geschrieben:
> On Thu, Oct 31, 2013 at 02:10:48PM +0100, Stefan Hajnoczi wrote:
> > On Wed, Oct 30, 2013 at 10:44:37AM -0400, Jeff Cody wrote:
> > > This patch series contains the initial VHDX log parsing, replay,
> > > write support, and image creation.
> > > 
> > > === v8 changes ===
> > > https://github.com/codyprime/qemu-kvm-jtc/tree/vhdx-write-v7-upstream
> > > 
> > > Rebased to latest qemu/master
> > > 
> > > Patch 10/19: * Added comments for bdrv_flush() (Stefan)
> > > 
> > > Patch 11/19: * Added qemu_iovec_destroy(&hd_qiov) (Stefan)
> > >              * On certain _writev errors, restore BAT cache (Stefan)
> > > 
> > > Patch 16/19: * Replaced fprintf(stderr,...) with error_setg_errno() 
> > > (Stefan)
> > > 
> > > Patch 18/19: * Added filter for block_state_zero in qemu-iotest/common.rc
> > > 
> > > Patch 19/19: * Moved log replay test name to 068 (part of rebase to 
> > > master)
> > > 
> > > === v7 changes ===
> > > https://github.com/codyprime/qemu-kvm-jtc/tree/vhdx-write-v7-upstream
> > > 
> > > Rebased to latest qemu/master (picked up vhdx r/o tests, migration 
> > > blocker)
> > > 
> > > Patch  8/19: * validate log descriptor_count (Stefan)
> > >              * fix typos in comments (Stefan)
> > >              * Removed unneccessary initialization (Stefan)
> > >              * Replay log prior to metadata (Stefan)
> > >              * In vhdx_log_flush(), call bdrv_flush() prior to zeroing
> > >                out the log guid in the header.
> > >              * In vhdx_close(), set freed pointers to NULL
> > >              
> > > Patch  9/19: * correct logic for region overlap (Stefan)
> > >              
> > > Patch 10/19: * add missing goto exit in error case (Stefan)
> > >              * add bdrv_flush() to ensure data is stable on disk (Stefan)
> > > 
> > > Patch 11/19: * fixed typos in comments (Stefan)
> > >              * QEMU coding style changes (Stefan)
> > >              * rename bat_entry to bat_entry_le for clarity (Stefan)
> > >              * Add PAYLOAD_BLOCK_ZERO explicit zero padding for
> > >                protocols that do not support zero init (Stefan)
> > >              * rename PAYLOAD_BLOCK_FULL_PRESENT to 
> > >                PAYLOAD_BLOCK_FULLY_PRESENT (Stefan)
> > > 
> > > Patch 13/19: * Fixed typo in commit message (Stefan)
> > > 
> > > Patch 19/19: * New, adds qemu-io test for log replay of data sector
> > > 
> > > v6 Patch 17/20: * Dropped (already upstream)
> > > v6 Patch 18/20: * Dropped (already upstream)
> > > 
> > > 
> > > 
> > > === v6 changes ===
> > > https://github.com/codyprime/qemu-kvm-jtc/tree/vhdx-write-v6-upstream
> > > 
> > > Rebased to latest qemu/master:
> > > 
> > > Patch 16/20: .bdrv_create() propagates Error, and bdrv_unref() used
> > >              instead of bdrv_delete().
> > > 
> > > Patch 17 & 18 are already included in another series:
> > >     [PATCH v3 0/3] qemu-iotests with sample images, vhdx test, cleanup
> > > 
> > > They are included here to provide a base for patches 19 & 20.  If the 
> > > above
> > > series is applied before this series, then patches 17 and 18 can be 
> > > ignored.
> > > 
> > > Patch 19/20: In qemu-io tests _make_test_img(), filter out vhdx-specific
> > >              options for .bdrv_create().
> > > 
> > > Patch 20/20: Add VHDX write test case to 064.
> > > 
> > > 
> > > === v5 changes ===
> > > 
> > > v5 is also available for testing from:
> > > https://github.com/codyprime/qemu-kvm-jtc/tree/vhdx-write-v5-upstream
> > > 
> > > Most of the patches from v4 -> v5 are the same, but there are a few 
> > > differences
> > > and a few new patches.  Here is a summary of which patches are different 
> > > and/or
> > > new:
> > > 
> > > Patch highlights:
> > > 
> > > Patch 7  just some minor code movement, in prep for changes in patch 8
> > > 
> > > Patch 8  incorporates review feedback from Stefan, for the previous Patch 
> > > 7
> > >          in v4.
> > > 
> > > Patch 9  adds region checking for log, region table, and metadata tables, 
> > > per
> > >          suggestion from Stefan.
> > > 
> > > Patch 10 minor change from changes made in 8/16 (vhdx_guid_is_zero() is 
> > > gone)
> > > 
> > > Patch 12 is just some minor housekeeping, to get rid of bit shifting that
> > >          doesn't need to happen.
> > > 
> > > 
> > > 
> > > === v4 changes ===  
> > > 
> > > v4 patches are available from github as well, on branch 
> > > vhdx-write-v4-upstream:
> > > https://github.com/codyprime/qemu-kvm-jtc/tree/vhdx-write-v4-upstream
> > > https://github.com/codyprime/qemu-kvm-jtc.git
> > > 
> > > Those in the midst of reviewing v3, don't fear - the only changes with v4 
> > > is
> > > the addition of patches on the end of the series (patches 10-13).  These
> > > patches enable creating VHDX images.  Image files created have been
> > > (briefly & lightly) tested on Hyper-V running on Windows Server 2012.
> > > 
> > > Some of the new patches could be squashed with earlier patches in the 
> > > series,
> > > but I refrained from doing so, since some of the patches have already been
> > > reviewed, and others are in the midst of review.  I want to make it as 
> > > easy
> > > as possible on those currently reviewing. There is nothing critical
> > > that needs to be pushed into the earlier patches.
> > > 
> > > New patches:
> > > 
> > > Patch 10:  Breaks out some more endian translation functions
> > >             (likely squashable into patch 5)
> > > 
> > > Patch 11:  Break out some operations into seperate helper functions
> > > 
> > > Patch 12:  More comment typos and header fixes in vhdx.h
> > >             (likely squashable into patch 1)
> > > 
> > > Patch 13:  Adds .bdrv_create() for vhdx.  VHDX images are can be created 
> > > for
> > >            Fixed or Dynamic images.
> > > 
> > > Patches 1-9 are unchanged.
> > > 
> > > === end v4 changelog ===
> > > 
> > > === v3 changes ===  
> > > 
> > > Thank you Kevin & Stefan for the feedback; incoporated in v3:
> > > 
> > > Patch 1: --- nil ---
> > > 
> > > Patch 2: * use sizeof(crc) instead of 4
> > >          * remove outdated comment
> > >          * use sizeof(MSGUID) instead of 16
> > >          * direct assignment of guid structs rather than memcpy
> > >          * rename 'rw' to 'generate_data_write_guid'
> > >          * use offsetof() instead of 4
> > >          * comment typos
> > >          * add missing error checking
> > >          * MSGUID is now QEMU_PACKED
> > >          * configure enable for vhdx is now correct and not braindead
> > > 
> > > Patch 3: --- nil ---
> > > 
> > > Patch 4: * code style fixes
> > >          * removed unused struct (VHDXLogEntryInfo)
> > > 
> > > Patch 5: * more direct assignment of guid rather than memcpy
> > >          * order of operation in export/import the same now
> > >          * became less generous with newlines (bah-humbug!)
> > > 
> > > Patch 6: * more direct assignment of guid rather than memcpy     
> > >          * add error check in vhdx_user_visible_write(), now returns int
> > > 
> > > Patch 7: * check error return now of vhdx_user_visible_write()
> > > 
> > > Patch 8: * check error return now of vhdx_user_visible_write()
> > >          * vhdx_log_write_sectors() uses bdrv_pwrite() vs 
> > > bdrv_pwrite_sync()
> > >          * more direct assignment of guid rather than memcpy     
> > >          * use offsetof() instead of 4
> > > 
> > > Patch 9: --- nil ---
> > > 
> > > === end v3 changelog ===
> > > 
> > > v2 changes:  Incorporated Fam's review feedback - Thank you Fam for the 
> > > feedback
> > > 
> > > === end v2 changelog ===
> > > 
> > > This will allow an existing log in a VHDX image to be replayed (e.g., a 
> > > VHDX
> > > image from a Hyper-V host that crashed).  In addition, metadata writes are
> > > enabled through the log.  This allows write support to be enabled for 
> > > VHDX,
> > > as the BAT can be updated safely via the log journal.
> > > 
> > > These exact patches are available from github, for testing:
> > > https://github.com/codyprime/qemu-kvm-jtc/tree/vhdx-write-v2-upstream
> > > 
> > > The latest vhdx work (including anything beyond these patches, such as
> > > backing/parent file support) can be found at:
> > > https://github.com/codyprime/qemu-kvm-jtc/tree/jtc-vhdx-latest
> > > 
> > > Jeff Cody (19):
> > >   block: vhdx - minor comments and typo correction.
> > >   block: vhdx - add header update capability.
> > >   block: vhdx code movement - VHDXMetadataEntries and BDRVVHDXState to
> > >     header.
> > >   block: vhdx - log support struct and defines
> > >   block: vhdx - break endian translation functions out
> > >   block: vhdx - update log guid in header, and first write tracker
> > >   block: vhdx code movement - move vhdx_close() above vhdx_open()
> > >   block: vhdx - log parsing, replay, and flush support
> > >   block: vhdx - add region overlap detection for image files
> > >   block: vhdx - add log write support
> > >   block: vhdx write support
> > >   block: vhdx - remove BAT file offset bit shifting
> > >   block: vhdx - move more endian translations to vhdx-endian.c
> > >   block: vhdx - break out code operations to functions
> > >   block: vhdx - fix comment typos in header, fix incorrect struct fields
> > >   block: vhdx - add .bdrv_create() support
> > >   block: vhdx - update _make_test_img() to filter out vhdx options
> > >   block: qemu-iotests for vhdx, add write test support
> > >   block: vhdx qemu-iotest - log replay of data sector
> > > 
> > >  block/Makefile.objs                                |    2 +-
> > >  block/vhdx-endian.c                                |  216 ++++
> > >  block/vhdx-log.c                                   | 1010 
> > > ++++++++++++++++
> > >  block/vhdx.c                                       | 1220 
> > > +++++++++++++++++---
> > >  block/vhdx.h                                       |  178 ++-
> > >  configure                                          |   24 +
> > >  tests/qemu-iotests/064                             |   11 +
> > >  tests/qemu-iotests/064.out                         |   14 +
> > >  tests/qemu-iotests/068                             |   67 ++
> > >  tests/qemu-iotests/068.out                         |    8 +
> > >  tests/qemu-iotests/common                          |    1 -
> > >  tests/qemu-iotests/common.rc                       |    5 +-
> > >  tests/qemu-iotests/group                           |    1 +
> > >  .../sample_images/iotest-dirtylog-10G-4M.vhdx.bz2  |  Bin 0 -> 4490 bytes
> > >  14 files changed, 2573 insertions(+), 184 deletions(-)
> > >  create mode 100644 block/vhdx-endian.c
> > >  create mode 100644 block/vhdx-log.c
> > >  create mode 100755 tests/qemu-iotests/068
> > >  create mode 100644 tests/qemu-iotests/068.out
> > >  create mode 100644 
> > > tests/qemu-iotests/sample_images/iotest-dirtylog-10G-4M.vhdx.bz2
> > 
> > Reviewed-by: Stefan Hajnoczi <address@hidden>
> >
> 
> Kevin,
> 
> Do you think it is possible for this to get in for 1.7.0-rc0 or rc1?

Stefan is handling block patches this week. But more importantly, I have
no idea what the plan for 1.7 is now. In theory, -rc1 should have been
released on Friday, but in fact we don't even have -rc0 yet.

I was expecting my pull request from Thursday to be the last non-bugfix
pull request for 1.7, but if it gets further delayed, there may be
another chance of getting it in.

Anthony, any opinion on this?

Kevin