[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH] block/mirror: fix fail to cancel w

From: Liang Li
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH] block/mirror: fix fail to cancel when VM has heavy BLK IO
Date: Mon, 29 Jan 2018 11:48:25 +0800
User-agent: Mutt/1.7.2 (2016-11-26)

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.


> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org

reply via email to

[Prev in Thread] Current Thread [Next in Thread]