Re: [Qemu-devel] [PATCH v2 00/25] qmp: add async command type

["Marc-André Lureau" <mlureau@redhat.com>]

Hi

----- Original Message -----
> On Tue, Jan 24, 2017 at 01:43:17PM -0500, Marc-André Lureau wrote:
> > Hi
> > 
> > ----- Original Message -----
> > > On Mon, Jan 23, 2017 at 06:27:29AM -0500, Marc-André Lureau wrote:
> > > > ----- Original Message -----
> > > > > On Wed, Jan 18, 2017 at 08:03:07PM +0400, Marc-André Lureau wrote:
> > > > > > Hi,
> > > > > 
> > > > > CCing Jeff Cody and John Snow, who have been working on generalizing
> > > > > Block Job APIs to generic background jobs.  There is some overlap
> > > > > between async commands and background jobs.
> > > > 
> > > > If you say so :) Did I miss a proposal or a discussion for async qmp
> > > > commands?
> > > 
> > > There is no recent mailing list thread, so it's probably best to discuss
> > > here:
> > > 
> > > The goal of jobs is to support long-running operations that can be
> > > managed via QMP.  Jobs can have a more elaborate lifecycle than just
> > > start -> finish/cancel (e.g. they can be paused/resumed and may have
> > > multiple phases of execution that the client controls).  There are QMP
> > > APIs to query their state (Are they running?  How much "progress" has
> > > been made?).
> > 
> > Indeed, I mention that in my cover. Such use cases require something more
> > complete than simple async qmp commands. I don't see why it would be
> > incompatible with the usage of async qmp commands.
> > 
> > > A client reconnecting to QEMU can query running jobs.  This way a client
> > > can resume with a running QEMU process.  For commands like saving a
> > > screenshot is mostly does not matter, but for commands that modify state
> > > it's critical that clients are aware of running commands after reconnect
> > > to prevent corruption/interference.  This behavior is what I asked about
> > > in my previous mail.
> > 
> > That's what I mention in the cover, some commands are global (and
> > broadcasted events are appropriate) and some are local to the client
> > context. Some could be discarded when the client disconnects etc. It's a
> > case by case.
> > 
> > > Jobs are currently only used by the block layer and called "block jobs",
> > > but the idea is to generalize this.  They use synchronous QMP + events.
> > 
> > That pattern will have the flaws I mentioned (empty return, broadcast
> > events, id conflict, qapi semantic & documentation etc). Something new can
> > be invented, but it will likely make the protocol more complicated
> > compared to the solution I proposed (which is optional btw, and gracefully
> > fallbacks to sync processing for clients that do not support the async qmp
> > capability). However, I believe the job interface could be built on top of
> > what I propose.
> > 
> > > Jobs are more heavy-weight than async QMP commands, but pause/resume,
> > > rate-limiting, progress reporting, robust reconnect, etc are important
> > > features.  Users want to be aware of long-running operations and have
> > > the ability to control them.
> > 
> > You can't generalize such job interface to all async commands. Some may not
> > implement the ability to report progress, to cancel, to pause etc, etc. In
> > the end, it will be complicated and unneeded in many cases (what's the use
> > case to pause or to get the progress of a screendump?). What I propose is
> > simpler and compatible with job/task interfaces appropriate for various
> > domains.
> > 
> > > I suspect that if we transition synchronous QMP commands to async we'll
> > > soon have requirements for progress reporting, pause/resume, etc.  So is
> > > there a set of commands that should be async and others that should be
> > > jobs or should everything just be a job?
> > 
> > Hard to say without a concrete proposal of what "job" is. Likely,
> > everything is not going to be a "job".
> > 
> > But hopefully qmp-async and jobs can co-exist and benefit from each other.
> 
> My concern with this series is that background operations must be
> observable and there must be a way to cancel them.  Otherwise management
> tools cannot do their job and it's hard to troubleshoot a misbehaving
> system because you can't answer the question "what's going on?".  Once
> you add that then a large chunk of block jobs is duplicated.

Tracking ongoing operations can also be done at management layer. If needed, we could add qmp-commands to list on-going commands (their ids etc), and add commands to cancel them. But then again, not all operations will be cancellable, and I am not sure having requirements to list or cancel or modify all on-going operation is needed (I would say no, just like today you can't do anything while a command is running)

> 
> So then you look at block jobs and realize that no QMP wire protocol
> changes are necessary.  Despite the limitations you've listed, block
> jobs have been working successfully for years and don't require QMP
> clients to change.

Right, but we have existing bugs with functions that need to be async, at least at the qemu level. My proposal fixes this and optionally also allows commands to be async at the qmp level to solve the limitations and issues I listed.

> 
> I agree that not all background operations need to support the same set
> of management primitives to rate-limit, report progress, cancel,
> pause/resume, etc.
> 
> It would be nice for this series to evolve to a generic QMP jobs series
> so that all background operations are visible to the client and a useful
> subset of management primitives can be implemented on a per-command
> basis.  Both live migration and existing block jobs could use this code
> so that we don't have multiple copies of the same infrastructure.

Indeed, but I would need to know what proposal or requirements the block layer have here, and I would appreciate if they took time to review mine.