From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39187) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eg0Qy-0008PS-B9 for qemu-devel@nongnu.org; Sun, 28 Jan 2018 22:48:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eg0Qx-0006Jn-8c for qemu-devel@nongnu.org; Sun, 28 Jan 2018 22:48:40 -0500 From: Liang Li Date: Mon, 29 Jan 2018 11:48:25 +0800 Message-ID: <20180129034823.GA20875@liangdeMacBook-Pro.local> References: <20180124061728.GA99621@localhost> <20180125045926.GA4720@localhost> <20180126064647.GA12068@localhost> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH] block/mirror: fix fail to cancel when VM has heavy BLK IO List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Liang Li , Jeff Cody , Kevin Wolf , Max Reitz , Paolo Bonzini , Huaitong Han , qemu-block@nongnu.org, qemu-devel@nongnu.org On Fri, Jan 26, 2018 at 08:04:08AM -0600, Eric Blake wrote: > On 01/26/2018 12:46 AM, Liang Li wrote: > > The current QMP command is: > > > > { 'command': 'block-job-cancel', 'data': { 'device': 'str', '*force': 'bool' } } > > > > 'force' has other meaning which is not used by libvirt, for the change, there > > are 3 options: > > > > a. Now that 'force' is not used by libvirt and it current semantic is not very useful, > > we can change it's semantic to force-quit without syncing. > > The current semantics are: > > # @force: whether to allow cancellation of a paused job (default > # false). Since 1.3. > > You are right that libvirt is not using it at the moment; but that > doesn't tell us whether someone else is using it. On the other hand, it > is a fairly easy argument to make that "a job which is paused is not > complete, so forcing it to cancel means an unclean image left behind", > which can then be reformulated as "the force flag says to cancel > immediately, whether the job is paused or has pending data, and thus > leave an unclean image behind". In other words, I don't think it is too > bad to just tidy up the wording, and allow the existing 'force':true > parameter to be enabled to quit a job that won't converge. > > > > > b. change 'force' from bool to flag, and bit 0 is used for it's original meaning. > > Not possible. You can't change from 'force':true to 'force':1 in JSON, > at least not without rewriting the command to use an alternate that > accepts both bool and int (actually, I seem to recall that we tightened > QAPI to not permit alternates that might be ambiguous when parsed by > QemuOpts, which may mean that is not even possible - although I haven't > tried to see if it works or gives an error). > > > > > c. add another bool parameter. > > Also doable, if we are concerned that existing semantics of 'force' > affecting only paused jobs must be preserved. > > > > > > > which is the best one? > > 1 is slightly less code, but 3 is more conservative. I'd be okay with > option 1 if no one else can provide a reason why it would break something. > OK. I will send a patch based on the first option. Thanks! Liang > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3266 > Virtualization: qemu.org | libvirt.org >