Re: [Qemu-devel] [PATCH 0/7] blockjobs: preliminary refactoring work, Pt 1

[Jeff Cody]

On Fri, Oct 14, 2016 at 02:32:55PM -0400, John Snow wrote:
> 
> 
> On 10/13/2016 06:56 PM, John Snow wrote:
> >This is a follow-up to patches 1-6 of:
> >[PATCH v2 00/11] blockjobs: Fix transactional race condition
> >
> >That series started trying to refactor blockjobs with the goal of
> >internalizing BlockJob state as a side effect of having gone through
> >the effort of figuring out which commands were "safe" to call on
> >a Job that has no coroutine object.
> >
> >I've split out the less contentious bits so I can move forward with my
> >original work of focusing on the transactional race condition in a
> >different series.
> >
> >Functionally the biggest difference here is the presence of "internal"
> >block jobs, which do not emit QMP events or show up in block query
> >requests. This is done for the sake of replication jobs, which should
> >not be interfering with the public jobs namespace.
> >
> 
> I have v2 ready to send out correcting Kevin's comments in patch #01, but
> I'd like to have the Replication maintainers at Fujitsu take a look at how I
> have modified replication and at least 'ACK' the change.
> 
> As a recap: I am creating "internal" block jobs that have no ID and
> therefore do not collide with the user-specified jobs namespace. This way
> users cannot query, cancel, pause, or otherwise accidentally interfere with
> the replication job lifetime.
> 
> It also means that management layers such as libvirt will not be aware of
> the presence of such "internal" jobs.
> 
> Relevant patches are 1-3. Please let me know if you have questions.
> 
> Thanks,
> --John Snow
>

Looks good to me, once you address Kevin's comments in patch 1.

> 
> >________________________________________________________________________________
> >
> >For convenience, this branch is available at:
> >https://github.com/jnsnow/qemu.git branch job-refactor-pt1
> >https://github.com/jnsnow/qemu/tree/job-refactor-pt1
> >
> >This version is tagged job-refactor-pt1-v1:
> >https://github.com/jnsnow/qemu/releases/tag/job-refactor-pt1-v1
> >
> >John Snow (7):
> >  blockjobs: hide internal jobs from management API
> >  blockjobs: Allow creating internal jobs
> >  Replication/Blockjobs: Create replication jobs as internal
> >  blockjob: centralize QMP event emissions
> >  Blockjobs: Internalize user_pause logic
> >  blockjobs: split interface into public/private, Part 1
> >  blockjobs: fix documentation
> >
> > block/backup.c               |   5 +-
> > block/commit.c               |  10 +-
> > block/mirror.c               |  28 +++--
> > block/replication.c          |  14 +--
> > block/stream.c               |   9 +-
> > block/trace-events           |   5 +-
> > blockdev.c                   |  74 +++++--------
> > blockjob.c                   | 109 ++++++++++++++----
> > include/block/block.h        |   3 +-
> > include/block/block_int.h    |  26 ++---
> > include/block/blockjob.h     | 257 
> > +++++++------------------------------------
> > include/block/blockjob_int.h | 232 ++++++++++++++++++++++++++++++++++++++
> > qemu-img.c                   |   5 +-
> > tests/test-blockjob-txn.c    |   5 +-
> > tests/test-blockjob.c        |   4 +-
> > 15 files changed, 443 insertions(+), 343 deletions(-)
> > create mode 100644 include/block/blockjob_int.h
> >