qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH for-2.5 0/4] Expose ErrorClass through introspection
Date: Thu, 12 Nov 2015 06:28:59 -0700	[thread overview]
Message-ID: <5644941B.9000608@redhat.com> (raw)
In-Reply-To: <8737wbtrwq.fsf@blackfin.pond.sub.org>

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

On 11/12/2015 03:48 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> I noticed that introspection was not documenting either
>> qmp_capabilities nor the ErrorClass enum.  I think this is worth
>> fixing for 2.5 when introspection is brand new, so that if we later
>> extend the ErrorClass enum or add future capability negotiation (and
>> in particular if such additions get backported in downstream builds),
>> a client will be able to use introspection to learn whether the new
>> features are supported, regardless of the qemu version.
>>
>> Note that this also adds qmp_capabilities to 'query-commands'.
>>
>> Yes, this is borderline, and you may decide that it doesn't deserve
>> to be called a bug and should wait for 2.6.

Or in other words, I posted this series precisely for this conversation.
 The mere fact that you have questions means that it is NOT appropriate
for 2.5 this late in the game.  But to answer you...

> 
> Two entirely separate issues: introspecting qmp_capabilities and
> introspecting error classes.  This reply is about the former.  I'll
> discuss the latter in a separate message.
> 
> Introspecting qmp_capabilities with query-qmp-schema is kind of
> academic, because by the time you can query-qmp-schema, you're already
> done using qmp_capabilities.  That's why docs/qmp-spec.txt nails down
> qmp_capabilities, unlike other commands.  Unfortunately, it's a bit
> screwy.  Let's have a closer look.
> 
> Right after connect, the server sends a greeting that includes a list of
> available capabilities (docs/qmp-spec.txt section 2.2 Server Greeting),
> currently none.
> 
> The only valid command then is qmp_capabilities (section 4. Capabilities
> Negotiation).  The command is meant to let clients enable capabilities
> from the greeting, but we've neglected to specify how.  Fortunately, QMP
> rejects all arguments, so we can still fix that by adding optional
> arguments, either a list of capabilities to enable, or a (boolean)
> argument for each capability.
> 
> My point is: unlike other commands, qmp_capabilities is baked into the
> protocol.  Introspection is useful to examine *variable* aspects.  With
> the hole in the protocol specification plugged, the only variable aspect
> of qmp_capabilities are the capabilities, and the practical way to
> introspect those is the server greeting.

Good point - if we ever add a capability, a client that knows how to
request the capability can easily learn whether the server will honor
the request by reading the server's advertisements.

> 
> Qapifying qmp_capabilities can make sense regardless.  But why does it
> need to be rushed into 2.5 then?

No need for the rush after all.  You are correct that qapi-fying
qmp_capabilities will NOT help the client learn anything it couldn't
have already learned from the server greeting.

And if we don't rush qmp_capabilities into 2.5 (patch 3), then we also
don't need to rush in a fix for explicitly empty types (patch 2) or a
way to detect empty types (patch 1).

As for patch 4 - we could still expose ErrorClass, via some means other
than qmp_capabilities, if desired; but that's a discussion for your
other mail.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

  reply	other threads:[~2015-11-12 13:29 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-12  1:17 [Qemu-devel] [PATCH for-2.5 0/4] Expose ErrorClass through introspection Eric Blake
2015-11-12  1:17 ` [Qemu-devel] [PATCH for-2.5 1/4] qapi: Add type.is_empty() helper Eric Blake
2015-11-12  1:17 ` [Qemu-devel] [PATCH for-2.5 2/4] qapi: Fix command with named empty argument type Eric Blake
2015-11-12  1:17 ` [Qemu-devel] [PATCH for-2.5 3/4] monitor: use qapi for qmp_capabilities command Eric Blake
2015-11-12  1:17 ` [Qemu-devel] [PATCH for-2.5 4/4] qapi: Expose ErrorClass through introspection Eric Blake
2015-11-12 10:48 ` [Qemu-devel] [PATCH for-2.5 0/4] " Markus Armbruster
2015-11-12 13:28   ` Eric Blake [this message]
2015-11-12 10:48 ` Markus Armbruster
2015-11-12 13:37   ` Eric Blake

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=5644941B.9000608@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@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).