qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: "Ricardo Perez Blanco" <ricardo.perez_blanco@nokia.com>,
	"Andreas Färber" <afaerber@suse.de>,
	"Dongli Zhang" <dongli.zhang@oracle.com>,
	qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH v2] Show values and description when using "qom-list"
Date: Fri, 29 Jun 2018 13:17:02 +0100	[thread overview]
Message-ID: <20180629121701.GI2568@work-vm> (raw)
In-Reply-To: <874lhqr9lg.fsf@dusky.pond.sub.org>

* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> 
> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> Markus Armbruster <armbru@redhat.com> writes:
> >> 
> >> > Andreas Färber <afaerber@suse.de> writes:
> >> >
> >> >> Am 08.06.2018 um 11:41 schrieb Dr. David Alan Gilbert:
> >> >>> * Andreas Färber (afaerber@suse.de) wrote:
> >> >>>> Am 01.06.2018 um 17:39 schrieb Ricardo Perez Blanco:
> >> >>>>> For debugging purposes it is very useful to:
> >> >>>>>  - See the description of the field. This information is already filled
> >> >>>>>    in but not shown in "qom-list" command.
> >> >>>>
> >> >>>> No objection on this part.
> >> >>>>
> >> >>>>>  - Display value of the field.
> >> >>>>
> >> >>>> That is by definition the qom-get operation, not qom-list. Just like the
> >> >>>> ls command does not show file contents, there's cat etc. for that. For
> >> >>>> debugging purposes we had a qom-tree (?) command that would combine
> >> >>>> both.
> >> >>>
> >> >>> I'm not too bothered about distinguishing between the two commands;
> >> >>> but it would be nice
> >> >
> >> > When an HMP and QMP both have a command with the same name, they should
> >> > do the same.
> >> >
> >> > HMP may add convenience features that aren't wanted in QMP, but I feel
> >> > extending an operation to list objects to also show their contents goes
> >> > beyond that.  If we want an HMP command that does both, it should be
> >> > named differently.  Perhaps that might even be more appropriate for HMP
> >> > than low-level commands qom-list and qom-get, but I leave that to the
> >> > HMP maintainer to decide.
> >> >
> >> >>>                      - one reason I'm not too bothered is because we've
> >> >>> failed to get a qom-get in multiple years of trying.
> >> >
> >> > We clearly haven't tried hard enough.
> >> >
> >> > If we can figure out how to show values in qom-list, surely we can
> >> > figure out how to show them in qom-get.
> >> >
> >> >>>>       There might be unmerged patches on qemu-devel related to display
> >> >>>> of certain data types.
> >> >>> 
> >> >>> Which ones?
> >> >>
> >> >> My original qom-info series needed StringOutputVisitor changes for enums
> >> >> (test case: rtc) that did not get accepted immediately and thus some
> >> >> part of HMP qom-info/qom-get got stuck due to risking assertions for
> >> >> qom-info / otherwise; QMP was not affected IIRC.
> >> >
> >> > Here's the last try I can find:
> >> > [PATCH v2] qom: Implement qom-get HMP command
> >> > Message-Id: <1473157086-12062-1-git-send-email-dgilbert@redhat.com>
> >> > https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg01041.html
> >> 
> >> Stalled on output format and consistency with qom-set.  I wrote back
> >> then "We can take qom-get as is and improve its output later."  I'd like
> >> to encourage you to dust it off.  Perfect's the enemy of good.
> >> 
> >> Wanted improvements include:
> >> 
> >> * Prettier output format.  I'd suggest creating a keyval variant of the
> >>   output visitor.
> >
> > I'm not too bothered about pretty at first, as long as we don't stop
> > anyway of making it pretty later; especially if non-compound types work
> > well.
> 
> We've waited so long for "someone" to post a solution that satisfies all
> wants.  We should take a solution that is useful and can grow.
> 
> >> * Make qom-set input format consistent by switching to the matching
> >>   input visitor.
> >> 
> >> > Its v1 tries a different approach:
> >> > [PATCH 0/2] qom-get [for 2.8]
> >> > Message-Id: <1472117833-10236-1-git-send-email-dgilbert@redhat.com>
> >> > Unfortunately the mailing list archive doesn't show the full thread, so
> >> > you get to follow three links:
> >> > https://lists.gnu.org/archive/html/qemu-devel/2016-08/msg03815.html
> >> > https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg04261.html
> >> > https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg04267.html
> >> 
> >> This one stalled on string visitor limitations.  You didn't feel like
> >> addressing them just to get qom-get working.  Understandable.
> >
> > Have there been any changes in the last 2 years that have helped?
> 
> There's been progress, but there are gaps left.
> 
> Back then, we had the choice of string input/output visitors, and the
> options visitor, all (severely) restricted to certain kinds of data.
> 
> Now we have the qobject keyval input visitor, which is almost general.
> There is no matching output visitor, yet, simply because we haven't had
> the need.

That's potentially a big help; is:
   a) Does keyval output here make sense (It seems reasonable, perhaps
      with some wrapping etc)
   b) Is this the code in util/keyval.c + qapi/qobject-input-visitor.c ?
   c) What's the right way to build the output - is it to use the
      existing qobject_output_visitor and add a qobject_to_keyval - or
      is it a job for a new visitor?

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

  reply	other threads:[~2018-06-29 12:17 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-01 15:39 [Qemu-devel] [PATCH v2] Show values and description when using "qom-list" Ricardo Perez Blanco
2018-06-01 16:01 ` Eric Blake
2018-06-11  7:18   ` Perez Blanco, Ricardo (Nokia - BE/Antwerp)
2018-06-02  7:20 ` Andreas Färber
2018-06-08  9:41   ` Dr. David Alan Gilbert
2018-06-08 16:19     ` Andreas Färber
2018-06-11  7:17       ` Perez Blanco, Ricardo (Nokia - BE/Antwerp)
2018-06-12  6:02       ` Markus Armbruster
2018-06-25 11:32         ` Markus Armbruster
2018-06-25 15:00           ` Dr. David Alan Gilbert
2018-06-25 18:42             ` Markus Armbruster
2018-06-29 12:17               ` Dr. David Alan Gilbert [this message]
2018-06-29 12:39                 ` Perez Blanco, Ricardo (Nokia - BE/Antwerp)
2018-07-02  6:36                 ` Markus Armbruster

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=20180629121701.GI2568@work-vm \
    --to=dgilbert@redhat.com \
    --cc=afaerber@suse.de \
    --cc=armbru@redhat.com \
    --cc=dongli.zhang@oracle.com \
    --cc=qemu-devel@nongnu.org \
    --cc=ricardo.perez_blanco@nokia.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).