[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block/mirror: fix fail to cancel when VM has he
From: |
Liang Li |
Subject: |
Re: [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.
Thanks!
Liang
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
>
Re: [Qemu-devel] [Qemu-block] [PATCH] block/mirror: fix fail to cancel when VM has heavy BLK IO, John Snow, 2018/01/26