All of lore.kernel.org
 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 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.