Re: [Qemu-devel] [PATCH V6 0/3] Implement sync modes for drive-backup.

[Ian Main]

On Tue, Jul 23, 2013 at 01:53:51PM +0200, Stefan Hajnoczi wrote:
> On Mon, Jul 22, 2013 at 03:09:17PM -0700, Ian Main wrote:
> > This patch adds sync modes on top of the work that Stefan Hajnoczi has done.
> > 
> > These patches apply on kevin/block.
> > 
> > Hopefully all is in order as this is my first QEMU patch.  Many thanks to
> > Stephan and Fam Zheng for their help.
> > 
> > V2:
> > 
> > - No longer poll, instead use qemu_coroutine_yield().
> > - Use bdrv_co_is_allocated().
> > - Much better SYNC_MODE_NONE test.
> > 
> > V3:
> > 
> > - A few style fixes.
> > - Better commit message explaining how TOP and NONE operate.
> > - Verified using checkpatch.pl.
> > 
> > V4:
> > 
> > - Add patch to use the source as a backing hd during backup.
> > - Add patch to default sync mode none to qcow2.
> > 
> > V5:
> > 
> > - Fix qcow2 patch.  Forgot to git add final version.
> > 
> > V6:
> > 
> > - Default to requiring 'format' when mode is absolute-paths.
> > - Removed one bad hunk that was misapplying.
> > - Fixed docs, examples and tests to match changes.
> > - Added tests for format bad/missing.
> > - Added bdrv_set_in_use() to target.
> > - Default to qcow2 patch not required.
> > 
> > Ian Main (3):
> >   Implement sync modes for drive-backup.
> >   Add tests for sync modes 'TOP' and 'NONE'
> >   Add backing drive while performing backup.
> > 
> >  block/backup.c                | 107 
> > +++++++++++++++++++++++++++++------------
> >  blockdev.c                    |  36 +++++++++-----
> >  include/block/block_int.h     |   4 +-
> >  qapi-schema.json              |   4 +-
> >  qmp-commands.hx               |   2 +
> >  tests/qemu-iotests/055        | 108 
> > +++++++++++++++++++++++++++++++++++++-----
> >  tests/qemu-iotests/055.out    |   4 +-
> >  tests/qemu-iotests/group      |   2 +-
> >  tests/qemu-iotests/iotests.py |   5 ++
> >  9 files changed, 211 insertions(+), 61 deletions(-)
> 
> This patch mostly takes care of image fleecing except it does not give
> the target a device name which can be used by nbd-server-add.

It's not clear to me how to do that.  I looked through Fams series and
was hoping to find a clue there but I may have missed it.
 
> Fam's series tackles the target device name and some of the overlapping
> problems with your series.

Yes which is great.
 
> The core feature in your series is sync=top|none and that needs to be
> merged.
> 
> Now we need to figure out which patches to take and what must be
> changed.  Please see the sub-threads on Fam's series.  Perhaps we can
> reach a consensus there.

Yes I'm happy to offer whatever help this is.  It does seem like
applying this is the next logical move however.  Fam could then base his
patch on top of this one.  At some point soon we need to reconcile
everything and to me that seems the logical way.  There actually doesn't
seem to be too much overlap in effort from what I can see.

Fam, what do you think?  I'm happy to make changes to make it easier for
your patchset too.

        Ian