qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Liang Li <liliang.opensource@gmail.com>
Cc: Jeff Cody <jcody@redhat.com>, Kevin Wolf <kwolf@redhat.com>,
	Max Reitz <mreitz@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Huaitong Han <hanhuaitong@didichuxing.com>,
	qemu-block@nongnu.org, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] block/mirror: fix fail to cancel when VM has heavy BLK IO
Date: Fri, 26 Jan 2018 08:04:08 -0600	[thread overview]
Message-ID: <b6f06c50-8b50-809c-0c7d-770db232d762@redhat.com> (raw)
In-Reply-To: <20180126064647.GA12068@localhost>

[-- Attachment #1: Type: text/plain, Size: 2147 bytes --]

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.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

  reply	other threads:[~2018-01-26 16:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-24  6:17 [Qemu-devel] [PATCH] block/mirror: fix fail to cancel when VM has heavy BLK IO Liang Li
2018-01-24 19:16 ` Eric Blake
2018-01-25  4:59   ` Liang Li
2018-01-25 14:48     ` Eric Blake
2018-01-26  6:46       ` Liang Li
2018-01-26 14:04         ` Eric Blake [this message]
2018-01-29  3:48           ` Liang Li
2018-01-26 17:23   ` [Qemu-devel] [Qemu-block] " John Snow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b6f06c50-8b50-809c-0c7d-770db232d762@redhat.com \
    --to=eblake@redhat.com \
    --cc=hanhuaitong@didichuxing.com \
    --cc=jcody@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=liliang.opensource@gmail.com \
    --cc=mreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).