qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Luiz Capitulino <lcapitulino@redhat.com>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Anthony Liguori <aliguori@us.ibm.com>,
	Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>,
	libvir-list@redhat.com, Stefan Hajnoczi <stefanha@gmail.com>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	qemu-devel@nongnu.org, Eric Blake <eblake@redhat.com>
Subject: Re: [Qemu-devel] [libvirt] QMP: Supporting off tree APIs
Date: Tue, 10 Jan 2012 18:55:38 -0200	[thread overview]
Message-ID: <20120110185538.3fddb182@doriath> (raw)
In-Reply-To: <4F0C8F11.3080602@codemonkey.ws>

On Tue, 10 Jan 2012 13:18:41 -0600
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 01/06/2012 01:42 PM, Luiz Capitulino wrote:
> > On Fri, 06 Jan 2012 09:08:19 -0600
> >> We also need to look at this interface as a public interface whether we
> >> technically committed it to or not.  The fact is, an important user is relying
> >> upon so that makes it a supported interface.  Even though I absolutely hate it,
> >> this is why we haven't changed the help output even after all of these years.
> >> Not breaking users should be one of our highest priorities.
> >
> > One thing I don't understand: how is libvirt relying on it if it doesn't
> > exist in qemu.git yet?
> 
> Because there was a discussion on qemu-devel and we agreed on an interface that 
> both sides would implement to.
> 
> I expect this to happen more often in the future too.

For future commands we either, implement it right away or ask libvirt to
wait for the command to be merged, or at least pass initial review.

> 
> >> Now we could change this command to make it a better QMP interface and we could
> >> do that in a compatible fashion.
> >>
> >> However, since I think we'll get proper async support really soon and that will
> >> involve fundamentally changing this command (along with a bunch of other
> >> commands), I don't think there's a lot of value in making cosmetic changes right
> >> now.  If we're going to break backwards compatibility, I'd rather do it once
> >> than twice.
> >
> > It goes beyond cosmetic changes. For example, will we allow other async
> > block commands to use this interface? And if we're doing this for block,
> > why not accept something similar for other subsystems if someone happen to
> > submit it?
> 
> Yes, I'm in agreement with you on direction.  But adding proper async support is 
> not a simple task, and blocking this interface based on that is a bad idea for 
> reasons previously mentioned.
> 
> > Let me take a non-cosmetic change request example. The BLOCK_JOB_COMPLETED
> > event has a 'error' field. However, it's impossible to know which error
> > happened because the 'error' field contains only the human error description.
> 
> Ok.  That's worth thinking about.
> 
> > Another problem: the event is called BLOCK_JOB_COMPLETED, but it's tied
> > to the streaming API. If we allow other commands to use it, they will likely
> > have to add fields there, making the event worse than it already is.
> 
> But aren't we going to introduce a proper async interface?  This is what has me 
> confused.

Yes, I was thinking about new block commands using this API before we get
proper async support...

> > There's more, because I skipped this review in v3 as I jumped to the
> > "proper async support" discussion...
> 
> Well let's do proper async support.  As I mentioned, I'd rather take this 
> command in as-is, introduce proper async support, and then deprecate a bunch of 
> stuff at once.

Ok, I'm willing to do this as Stefan has agreed on deprecating this as soon as
we get proper async support.

> 
> >> What I'd suggest is that we take the command in as-is and we mark it:
> >>
> >> Since: 1.1
> >> Deprecated: 1.2
> >> See Also: TBD
> >>
> >> The idea being that we'll introduce new generic async commands in 1.2 and
> >> deprecate this command.  We can figure out the removal schedule then too.  Since
> >> this command hasn't been around all that long, we can probably have a short
> >> removal schedule.
> >
> > That makes its inclusion even discussable :) A few (very honest) questions:
> >
> >   1. Is it really worth it to have the command for one or two releases?
> 
> Yes.  The most important consideration IMHO is parallelizing development.  We 
> want the block layer to evolve to the greatest extent possible independent of 
> our other subsystems.  If we don't have the right infrastructure in QMP to 
> support a block feature, that shouldn't hold up progress in the block layer to 
> the greatest extent possible.
> 
> >   2. Will we allow other block commands to use this async API?
> 
> Depends on how long it takes to do "proper async support".
> 
> >   3. Are we going to accept other ad-hoc async APIs until we have a
> >      proper one?
> 
> Yes.  So let's get serious about getting a proper interface in :-)

ACK

> 
> Regards,
> 
> Anthony Liguori
> 

  reply	other threads:[~2012-01-10 20:55 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-13 13:52 [Qemu-devel] [PATCH v3 0/9] block: generic image streaming Stefan Hajnoczi
2011-12-13 13:52 ` [Qemu-devel] [PATCH v3 1/9] coroutine: add co_sleep_ns() coroutine sleep function Stefan Hajnoczi
2011-12-13 13:52 ` [Qemu-devel] [PATCH v3 2/9] block: add BlockJob interface for long-running operations Stefan Hajnoczi
2011-12-14 13:51   ` Kevin Wolf
2011-12-15  8:50     ` Stefan Hajnoczi
2011-12-15 10:40     ` Marcelo Tosatti
2011-12-16  8:09       ` Stefan Hajnoczi
2011-12-13 13:52 ` [Qemu-devel] [PATCH v3 3/9] block: add image streaming block job Stefan Hajnoczi
2011-12-13 14:14   ` Marcelo Tosatti
2011-12-13 15:18     ` Stefan Hajnoczi
2011-12-14 13:59   ` Kevin Wolf
2011-12-13 13:52 ` [Qemu-devel] [PATCH v3 4/9] block: rate-limit streaming operations Stefan Hajnoczi
2011-12-13 13:52 ` [Qemu-devel] [PATCH v3 5/9] qmp: add block_stream command Stefan Hajnoczi
2012-01-04 12:59   ` Luiz Capitulino
2012-01-04 13:11     ` Luiz Capitulino
2012-01-05 13:48     ` Stefan Hajnoczi
2012-01-05 14:16       ` [Qemu-devel] QMP: Supporting off tree APIs Luiz Capitulino
2012-01-05 15:35         ` [Qemu-devel] [libvirt] " Eric Blake
2012-01-05 15:56           ` Anthony Liguori
2012-01-05 20:26             ` Luiz Capitulino
2012-01-06 11:06               ` Stefan Hajnoczi
2012-01-06 12:45                 ` Luiz Capitulino
2012-01-06 15:08                   ` Anthony Liguori
2012-01-06 19:42                     ` Luiz Capitulino
2012-01-10 19:18                       ` Anthony Liguori
2012-01-10 20:55                         ` Luiz Capitulino [this message]
2012-01-10 21:02                           ` Anthony Liguori
2012-01-11  1:41                             ` Luiz Capitulino
2012-01-05 14:20       ` [Qemu-devel] [PATCH v3 5/9] qmp: add block_stream command Stefan Hajnoczi
2011-12-13 13:52 ` [Qemu-devel] [PATCH v3 6/9] qmp: add block_job_set_speed command Stefan Hajnoczi
2011-12-13 13:52 ` [Qemu-devel] [PATCH v3 7/9] qmp: add block_job_cancel command Stefan Hajnoczi
2011-12-13 13:52 ` [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs Stefan Hajnoczi
2011-12-14 14:54   ` Kevin Wolf
2011-12-15  8:27     ` Stefan Hajnoczi
2011-12-15 10:34       ` Kevin Wolf
2011-12-15 11:30         ` Luiz Capitulino
2011-12-15 12:00           ` Stefan Hajnoczi
2011-12-15 12:09             ` Luiz Capitulino
2011-12-15 12:37             ` Kevin Wolf
2011-12-15 12:51               ` Stefan Hajnoczi
2012-01-04 13:17             ` Luiz Capitulino
2011-12-13 13:52 ` [Qemu-devel] [PATCH v3 9/9] test: add image streaming test cases Stefan Hajnoczi
2011-12-13 14:12 ` [Qemu-devel] [PATCH v3 0/9] block: generic image streaming Yibin Shen
2011-12-13 15:18   ` Stefan Hajnoczi
2011-12-14  1:42     ` Yibin Shen
2011-12-14 10:50       ` 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=20120110185538.3fddb182@doriath \
    --to=lcapitulino@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=anthony@codemonkey.ws \
    --cc=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=mtosatti@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --cc=stefanha@linux.vnet.ibm.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).