All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>,
	Eric Blake <eblake@redhat.com>,
	qemu-devel@nongnu.org,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [PATCH 1/5] docs/devel: document expectations for QAPI data modelling for QMP
Date: Fri, 10 Sep 2021 13:46:59 +0100	[thread overview]
Message-ID: <YTtTwwXjG9/u653o@redhat.com> (raw)
In-Reply-To: <87tuium6u7.fsf@dusky.pond.sub.org>

On Thu, Sep 09, 2021 at 11:33:20AM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > Traditionally we have required that newly added QMP commands will model
> > any returned data using fine grained QAPI types. This is good for
> > commands that are intended to be consumed by machines, where clear data
> > representation is very important. Commands that don't satisfy this have
> > generally been added to HMP only.
> >
> > In effect the decision of whether to add a new command to QMP vs HMP has
> > been used as a proxy for the decision of whether the cost of designing a
> > fine grained QAPI type is justified by the potential benefits.
> >
> > As a result the commands present in QMP and HMP are non-overlapping
> > sets, although HMP comamnds can be accessed indirectly via the QMP
> > command 'human-monitor-command'.
> >
> > One of the downsides of 'human-monitor-command' is that the QEMU monitor
> > APIs remain tied into various internal parts of the QEMU code. For
> > example any exclusively HMP command will need to use 'monitor_printf'
> > to get data out. It would be desirable to be able to fully isolate the
> > monitor implementation from QEMU internals, however, this is only
> > possible if all commands are exclusively based on QAPI with direct
> > QMP exposure.
> >
> > The way to achieve this desired end goal is to finese the requirements
> > for QMP command design. For cases where the output of a command is only
> > intended for human consumption, it is reasonable to want to simplify
> > the implementation by returning a plain string containing formatted
> > data instead of designing a fine grained QAPI data type. This can be
> > permitted if-and-only-if the command is exposed under the 'x-' name
> > prefix. This indicates that the command data format is liable to
> > future change and that it is not following QAPI design best practice.
> >
> > The poster child example for this would be the 'info registers' HMP
> > command which returns printf formatted data representing CPU state.
> > This information varies enourmously across target architectures and
> > changes relatively frequently as new CPU features are implemented.
> > It is there as debugging data for human operators, and any machine
> > usage would treat it as an opaque blob. It is thus reasonable to
> > expose this in QMP as 'x-query-registers' returning a 'str' field.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  docs/devel/writing-qmp-commands.rst | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)


> > +QAPI types. As a general guide, a caller of the QMP command should never need
> > +to parse individual returned data fields. If a field appears to need parsing,
> > +them it should be split into separate fields corresponding to each distinct
> > +data item. This should be the common case for any new QMP command that is
> > +intended to be used by machines, as opposed to exclusively human operators.
> > +
> > +Some QMP commands, however, are only intended as adhoc debugging aids for human
> > +operators. While they may return large amounts of formatted data, it is not
> > +expected that machines will need to parse the result. The overhead of defining
> > +a fine grained QAPI type for the data may not be justified by the potential
> > +benefit. In such cases, it is permitted to have a command return a simple string
> 
> There are many existing long lines in this file, so I'm not flagging
> yours, except for this one, because it increases the maximum.

This line is at exactly 80 characters so checkstyle is happy with it.
We don't have any requirement for a differenet line limit for docs
afair ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  reply	other threads:[~2021-09-10 12:54 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-08 10:37 [PATCH 0/5] Stop adding HMP-only commands, allow QMP for all Daniel P. Berrangé
2021-09-08 10:37 ` [PATCH 1/5] docs/devel: document expectations for QAPI data modelling for QMP Daniel P. Berrangé
2021-09-08 17:42   ` Eric Blake
2021-09-09  9:33   ` Markus Armbruster
2021-09-10 12:46     ` Daniel P. Berrangé [this message]
2021-09-10 13:45       ` Markus Armbruster
2021-09-10 13:52         ` Daniel P. Berrangé
2021-09-08 10:37 ` [PATCH 2/5] hw/core: introduce 'format_state' callback to replace 'dump_state' Daniel P. Berrangé
2021-09-08 10:37 ` [PATCH 3/5] target/i386: convert to use format_state instead of dump_state Daniel P. Berrangé
2021-09-08 12:17   ` Ján Tomko
2021-09-08 18:05   ` Eric Blake
2021-09-08 22:06     ` Daniel P. Berrangé
2021-09-09  9:55       ` Daniel P. Berrangé
2021-09-08 10:37 ` [PATCH 4/5] qapi: introduce x-query-registers QMP command Daniel P. Berrangé
2021-09-08 18:06   ` Eric Blake
2021-09-09  9:05   ` Markus Armbruster
2021-09-09 10:01     ` Daniel P. Berrangé
2021-09-08 10:37 ` [PATCH 5/5] monitor: rewrite 'info registers' in terms of 'x-query-registers' Daniel P. Berrangé
2021-09-08 11:01   ` Philippe Mathieu-Daudé
2021-09-08 11:02     ` Daniel P. Berrangé
2021-09-08 12:18 ` [PATCH 0/5] Stop adding HMP-only commands, allow QMP for all Ján Tomko
2021-09-08 15:09 ` Markus Armbruster
2021-09-08 15:24   ` Daniel P. Berrangé
2021-09-09  4:48     ` Markus Armbruster
2021-09-09  8:13       ` Markus Armbruster
2021-09-09  8:40       ` Daniel P. Berrangé
2021-09-08 16:15   ` Philippe Mathieu-Daudé
2021-09-09  6:15   ` Paolo Bonzini

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=YTtTwwXjG9/u653o@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=eblake@redhat.com \
    --cc=ehabkost@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.