qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-devel@nongnu.org, Luiz Capitulino <lcapitulino@redhat.com>,
	Adam Litke <agl@us.ibm.com>,
	Markus Armbruster <armbru@redhat.com>,
	Avi Kivity <avi@redhat.com>
Subject: Re: [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2)
Date: Thu, 17 Mar 2011 10:49:06 -0500	[thread overview]
Message-ID: <4D822D72.20406@codemonkey.ws> (raw)
In-Reply-To: <4D8214EB.1060303@redhat.com>

On 03/17/2011 09:04 AM, Kevin Wolf wrote:
>
>> No, the problem with the old events is that they aren't
>> registered/maskable.  So even if you don't care about BLOCK_IO_ERROR,
>> you're getting the notification.  Plus, we'd like to add the ability to
>> add a tag to events when we register them.
> What's the problem with registering them?

You could add a simple mask/unmask event to the current protocol.  But 
that doesn't let you filter events for a given object.  As I mentioned 
in the previous note, I would like to be able to only receive 
BLOCK_IO_ERROR for certain devices.

>   If you want to stop client
> from doing this you must introduce a special case for obsolete events
> that cannot be registered. Do we gain anything from this?

Actually, the old style events are distinct from the new signals.  They 
cannot be registered/unregistered.  My intention is for new clients to 
just ignore that they exist.

>> The other problem is that events are all global today.  BLOCK_IO_ERROR
>> is a good example of this.  It's really an error that's specific to a
>> block device and it passes the name of the block device that it's
>> specific to as an argument.  But if we have a masking mechanism it could
>> only globally enable/disable BLOCK_IO_ERROR for all devices.
>>
>> It would be much nicer to be able to enable the event for specific block
>> devices.  This requires some protocol visible changes.  I'm still
>> writing up these changes but hope to have something for review soon.
> I wonder if the old, more generic event couldn't be generated
> automatically if the more specific signal is triggered in the block code.

Yes, this is what I'm going to try to do.  This is already how the 
patches I sent out work but now that I have the new signal model, it 
requires a little more tweaking.

But this is the advantage of being able to consume interfaces within 
QEMU.  As long as the block layer exposes a signal mechanism that 
provides the same information, we can hide the old style events entirely 
within the QMP layer by registering for the new style signals and 
created old style events.

>>>    but just that it
>>> doesn't map to the C interface in the way you like.
>> I think I've maybe been using "C interface" to much.  The current event
>> wire protocol doesn't map to any client interface well.
> If you mean their broadcast style, that's not really related to nesting
> or struct vs. argument.

I meant structs vs. arguments too.

I'd like to do:

import qmp

def io_error(blockdev, operation, action):
    if operation == 'stop':
        ....

srv = qmp.ServerProxy(...)

srv.connect('IO_ERROR', io_error)

Python has the same problem re: arguments.  If we add a new argument to 
a signal, then it breaks the Python client library.  At the same time, 
when possible, it's nice to use argument syntax because the alternative is:

import qmp

def io_error(args):
     if args['action'] == 'stop':
        ....

Which isn't quite as nice to me.  So I don't want to preclude having the 
ability to have a signal with a few arguments but we just have to be 
careful that we're not goign to need to add more arguments later.  If we 
do, then we should make sure there's a structure there too.

Regards,

Anthony Liguori

> Kevin
>

      reply	other threads:[~2011-03-17 15:49 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-11 23:05 [Qemu-devel] [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 01/15] qapi: add code generator for qmp-types (v2) Anthony Liguori
2011-03-11 23:12   ` [Qemu-devel] " Anthony Liguori
2011-03-12 11:29   ` [Qemu-devel] " Blue Swirl
2011-03-12 15:00     ` Anthony Liguori
2011-03-18 14:18       ` Luiz Capitulino
2011-03-18 14:14   ` [Qemu-devel] " Luiz Capitulino
2011-03-11 23:05 ` [Qemu-devel] [PATCH 02/15] qapi: add code generator for type marshallers Anthony Liguori
2011-03-18 15:13   ` [Qemu-devel] " Luiz Capitulino
2011-03-11 23:05 ` [Qemu-devel] [PATCH 03/15] qapi: add core QMP server support (v2) Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 04/15] qapi: add signal support to core QMP server Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 05/15] qapi: add QAPI module type Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 06/15] qapi: add code generators for QMP command marshaling Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 07/15] qapi: add query-version QMP command Anthony Liguori
2011-03-12 11:19   ` Blue Swirl
2011-03-12 15:06     ` Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 08/15] qapi: add new QMP server that uses CharDriverState (v2) Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 09/15] vl: add a new -qmp2 option to expose experimental QMP server Anthony Liguori
2011-03-11 23:14   ` [Qemu-devel] " Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 10/15] qapi: add QMP quit command Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 11/15] qapi: add QMP qmp_capabilities command Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 12/15] qapi: add QMP put-event command Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 13/15] qapi: add code generator for libqmp (v2) Anthony Liguori
2011-03-12 11:10   ` Blue Swirl
2011-03-12 14:53     ` Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 14/15] qapi: add test-libqmp Anthony Liguori
2011-03-12 11:23   ` Blue Swirl
2011-03-12 14:59     ` Anthony Liguori
2011-03-11 23:05 ` [Qemu-devel] [PATCH 15/15] qapi: generate HTML report for test-libqmp Anthony Liguori
2011-03-16 14:34 ` [Qemu-devel] Re: [PATCH 00/15] QAPI Round 1 (core code generator) (v2) Luiz Capitulino
2011-03-16 14:49   ` Paolo Bonzini
2011-03-16 15:00     ` Luiz Capitulino
2011-03-16 16:06       ` Anthony Liguori
2011-03-16 16:03     ` Anthony Liguori
2011-03-16 16:31       ` Paolo Bonzini
2011-03-16 18:06         ` Anthony Liguori
2011-03-16 15:59   ` Anthony Liguori
2011-03-16 18:09     ` Luiz Capitulino
2011-03-16 18:32       ` Anthony Liguori
2011-03-16 19:27         ` Luiz Capitulino
2011-03-16 20:00           ` Anthony Liguori
2011-03-18 14:10             ` Luiz Capitulino
2011-03-18 14:22               ` Anthony Liguori
2011-03-17 12:21     ` Kevin Wolf
2011-03-17 12:46       ` Anthony Liguori
2011-03-17 13:15         ` Kevin Wolf
2011-03-17 13:28           ` Anthony Liguori
2011-03-17 14:04             ` Kevin Wolf
2011-03-17 15:49               ` Anthony Liguori [this message]

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=4D822D72.20406@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=agl@us.ibm.com \
    --cc=armbru@redhat.com \
    --cc=avi@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lcapitulino@redhat.com \
    --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).