Re: [Qemu-block] [PATCH 00/17] block: transactionless incremental backup series

[Stefan Hajnoczi]

On Fri, Feb 27, 2015 at 07:38:14PM -0500, John Snow wrote:
> 
> 
> On 02/27/2015 07:34 PM, Eric Blake wrote:
> >On 02/27/2015 04:47 PM, John Snow wrote:
> >>New topic, new version, same great incremental backup taste.
> >>
> >>This series relies on [Qemu-devel] [PULL 30/69] blkdebug: fix "once" rule,
> >>part of Stefan's 69 patch pull request submitted 2015-02-27.
> >>
> >>This patchset enables the in-memory part of the incremental backup
> >>feature, without transactional support.
> >>
> >>Support for transactions will come at a later date, but getting this
> >>portion upstream will help stabilize work on bitmap persistence and
> >>bitmap migration.
> >>
> >>Transactional support is being postponed to allow more development
> >>to create a transactional callback system that will allow objects used
> >>by the drive backup routines to perform cleanup actions conditionally
> >>based on the outcome of all backups in the transaction group.
> >
> >Food for thought, and not necessarily fatal to this series:
> >
> >What if this series makes 2.3, but transactions do not?  In the past,
> >we've had the benefit that if a standalone command exists, then the
> >matching transaction command also exists in the same release, such that
> >'query-commands' was a sufficient witness for what transactions we could
> >run.  But if we delay transactional incremental backup to 2.4, we'll
> >need some sort of introspection other than 'query-commands' to learn
> >what transaction elements are supported.
> >
> >>
> >>Deletions:
> >>  - Removed Transactions, to be added later.
> >>  - Removed Transaction tests, as above.
> >>
> >>Changes:
> >>01: Indentation fixes.
> >>     Removed enable/disable documentation.
> >>     Added a note that transactions aren't implemented yet.
> >>     Removed my needless commas
> >>     Added error case documentation.
> >>
> >>07: QMP enable/disable commands are deleted.
> >>
> >>14: Some comments concerning assertions.
> >>     Scrub re-alloc memory if we expand the array.
> >>     Do not attempt to scrub memory if fix_count is 0
> >>
> >>Changes made with Reviews kept:
> >>
> >>02: Since 2.4
> >
> >Oh, I didn't read far enough in my comments above.  If this entire
> >series is waiting for 2.4, then we stand a much better chance of getting
> >transaction support in the same release, negating all my worry above :)
> >
> 
> Yeah, the feature is *so* weak without transactions that I don't really want
> to make the push without them.
> 
> I intend to send out the new transaction features ASAP, but I don't know if
> they'll get approved for 2.3 this late in the game. Heck, I can always just
> change the 'Since' text back.

Incremental backup is suitable for 2.4.  Unless there is a strong reason
to push for 2.3 with this core series, it's better to wait for 2.4 where
we can test all the pieces together.

Stefan

pgpEkT0_Ml3Uy.pgp
Description: PGP signature