qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Adam Litke <agl@us.ibm.com>
To: Eric Blake <eblake@redhat.com>
Cc: libvir-list@redhat.com, Stefan Hajnoczi <stefanha@gmail.com>,
	Anthony Liguori <aliguori@us.ibm.com>,
	qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [libvirt] virDomainBlockJobAbort and block_job_cancel
Date: Thu, 8 Dec 2011 08:55:56 -0600	[thread overview]
Message-ID: <20111208145556.GA3794@us.ibm.com> (raw)
In-Reply-To: <4EDFF066.6010302@redhat.com>

On Wed, Dec 07, 2011 at 04:01:58PM -0700, Eric Blake wrote:
> On 12/07/2011 03:35 PM, Adam Litke wrote:
> > Stefan's qemu tree has a block_job_cancel command that always acts
> > asynchronously.  In order to provide the synchronous behavior in libvirt (when
> > flags is 0), I need to wait for the block job to go away.  I see two options:
> > 
> > 1) Use the event:
> > To implement this I would create an internal event callback function and
> > register it to receive the block job events.  When the cancel event comes in, I
> > can exit the qemu job context.  One problem I see with this is that events are
> > only available when using the json monitor mode.
> 
> I like this idea.  We have internally handled events before, and limited
> it to just JSON if that made life easier: for example, virDomainReboot
> on qemu is rejected if you only have the HMP monitor, and if you have
> the JSON monitor, the implementation internally handles the event to
> change the domain state.
> 
> Can we reliably detect whether qemu is new enough to provide the event,
> and if qemu was older and did not provide the event, do we reliably know
> that abort was blocking in that case?

I think we can say that qemu will operate in one of two modes:
a) Blocking abort AND event is not emitted
b) Non-blocking abort AND event is emitted

The difficulty is in detecting which case the current qemu supports.  I don't
believe there is a way to query qemu for a list of currently-supported events.
Therefore, we'd have to use version numbers.  If we do this, how do we avoid
breaking users of qemu git versions that fall between official qemu releases?

> It's okay to make things work that used to fail, but it is a regression
> to make blocking job cancel fail where it used to work, so rejecting
> blocking job cancel with HMP monitor is not a good idea.  If we can
> guarantee that all qemu new enough to have async cancel also support the
> event, while older qemu was always blocking, and that we can always use
> the JSON monitor to newer qemu, then we're set - merely ensure that we
> use only the JSON monitor and the event.  But if we can't make the
> guarantees, and insist on supporting newer qemu via HMP monitor, then we
> may need a hybrid combination of options 1 and 2, or for less code
> maintenance, just option 2.

Is there a deprecation plan for HMP with newer qemu versions?  I really hate the
idea of needing two implementations for this: one polling and one event-based.

> > 2) Poll the qemu monitor
> > To do it this way, I would write a function that repeatedly calls
> > virDomainGetBlockJobInfo() against the disk in question.  Once the job
> > disappears from the list I can return with confidence that the job is gone.
> > This is obviously sub-optimal because I need to poll and sleep.
> 
> We've done this before, for both HMP and JSON - see
> qemuMigrationWaitForCompletion.  I agree that an event is nicer than
> polling, but we may be locked into this.
> 
> > 
> > 3) Ask Stefan to provide a synchronous mode for the qemu monitor command
> > This one is the nicest from a libvirt perspective, but I doubt qemu wants to add
> > such an interface given that it basically has broken semantics as far as qemu is
> > concerned.
> 
> Or even:
> 
> 4) Ask Stefan to make the HMP monitor command synchronous, but only
> expose the JSON command as asynchronous.  After all, it is only HMP
> where we can't wait for an event.

Stefan, how 'bout it?

> > 
> > If this is all too nasty, we could probably just change the behavior of
> > blockJobAbort and make it always synchronous with a 'cancelled' event.
> 
> No, we can't change the behavior without breaking back-compat of
> existing clients of job cancellation.

-- 
Adam Litke <agl@us.ibm.com>
IBM Linux Technology Center

  parent reply	other threads:[~2011-12-08 14:56 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-23 14:48 [Qemu-devel] virDomainBlockJobAbort and block_job_cancel Stefan Hajnoczi
2011-11-23 16:04 ` [Qemu-devel] [libvirt] " Eric Blake
2011-11-24  5:31   ` Daniel Veillard
2011-11-24  9:21     ` Stefan Hajnoczi
2011-12-07 22:35       ` Adam Litke
2011-12-07 23:01         ` Eric Blake
2011-12-08 14:55           ` Stefan Hajnoczi
2011-12-08 14:55           ` Adam Litke [this message]
2011-12-08 15:33             ` Stefan Hajnoczi

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=20111208145556.GA3794@us.ibm.com \
    --to=agl@us.ibm.com \
    --cc=aliguori@us.ibm.com \
    --cc=eblake@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    /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).