From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35840) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fM7ee-00064L-Hd for qemu-devel@nongnu.org; Fri, 25 May 2018 04:00:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fM7ea-00005K-J6 for qemu-devel@nongnu.org; Fri, 25 May 2018 04:00:52 -0400 Date: Fri, 25 May 2018 10:00:35 +0200 From: Kevin Wolf Message-ID: <20180525080035.GA5562@localhost.localdomain> References: <20180518132114.4070-1-kwolf@redhat.com> <20180518132114.4070-22-kwolf@redhat.com> <26778cd3-3150-734b-d8c6-afa6e41f0215@redhat.com> <20180524082412.GC4008@localhost.localdomain> <1988292d-467a-9c63-e64d-035b0e93348a@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1988292d-467a-9c63-e64d-035b0e93348a@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 21/40] job: Convert block_job_cancel_async() to Job List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, jcody@redhat.com, armbru@redhat.com, mreitz@redhat.com Am 24.05.2018 um 19:42 hat John Snow geschrieben: > > > On 05/24/2018 04:24 AM, Kevin Wolf wrote: > > Am 24.05.2018 um 01:18 hat John Snow geschrieben: > >>> diff --git a/include/qemu/job.h b/include/qemu/job.h > >>> index 3e817beee9..2648c74281 100644 > >>> --- a/include/qemu/job.h > >>> +++ b/include/qemu/job.h > >>> @@ -97,6 +97,12 @@ typedef struct Job { > >>> */ > >>> bool cancelled; > >>> > >>> + /** > >>> + * Set to true if the job should abort immediately without waiting > >>> + * for data to be in sync. > >>> + */ > >>> + bool force_cancel; > >>> + > >> > >> Does this comment need an update now, though? > >> > >> Actually, in terms of "new jobs" API, it'd be really nice if cancel > >> *always meant cancel*. > >> > >> I think "cancel" should never be used to mean "successful completion, > >> but different from the one we'd get if we used job_complete." > >> > >> i.e., either we need a job setting: > >> > >> job-set completion-mode=[pivot|no-pivot] > >> > >> or optional parameters to pass to job-complete: > >> > >> job-complete mode=[understood-by-job-type] > >> > >> or some other mechanism that accomplishes the same type of behavior. It > >> would be nice if it did not have to be determined at job creation time > >> but instead could be determined later. > > > > I agree. We already made sure that job-cancel really means cancel on the > > QAPI level, so we're free to do that. We just need to keep supporting > > block-job-cancel with the old semantics, so what I have is the easy > > conversion. We can change the internal implementation when we actually > > implement the selection of a completion mode. > > > > Kevin > > > > We need this before 3.0 though, yeah? unless we make job-cancel > x-job-cancel or some other warning that the way it works might change, yeah? > > Or do I misunderstand our leeway to change this at a later point in time? > > (can job-cancel apply to block jobs created with the legacy > infrastructure? My reading was "yes.") It can, and it already has its final semantics, so nothing has to change before 3.0. job-cancel is equivalent to block-job-cancel with fixed force=true. If you want the complete-by-cancel behaviour of mirror, you have to use block-job-cancel for now, because job-cancel doesn't provide that functionality. So what we can change later is adding a way to initiate this secondary completion mode with a job-* command (probably with a new option for job-complete). But we wouldn't change the semantics of exisiting commands. Kevin