From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46012) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bzGCO-00063a-QM for qemu-devel@nongnu.org; Wed, 26 Oct 2016 00:52:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bzGCN-00052p-PR for qemu-devel@nongnu.org; Wed, 26 Oct 2016 00:52:24 -0400 Date: Wed, 26 Oct 2016 00:52:16 -0400 From: Jeff Cody Message-ID: <20161026045216.GH2677@localhost.localdomain> References: <1476399422-8028-1-git-send-email-jsnow@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 0/7] blockjobs: preliminary refactoring work, Pt 1 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: qemu-block@nongnu.org, kwolf@redhat.com, xiecl.fnst@cn.fujitsu.com, wency@cn.fujitsu.com, qemu-devel@nongnu.org, pbonzini@redhat.com 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 > >