| Message ID | 20180601153922.11590-1-ricardo.perez_blanco@nokia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/01/2018 10:39 AM, Ricardo Perez Blanco wrote: > 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. > - Display value of the field. > > Signed-off-by: Ricardo Perez Blanco <ricardo.perez_blanco@nokia.com> > --- > +++ b/qapi/misc.json > @@ -1328,10 +1328,12 @@ > # > # @description: if specified, the description of the property. > # > -# Since: 1.2 > +# @value: if specified, the value of the property. > +# > +# Since: 2.13 Not quite. The struct was first introduced in 1.2, and only 'value' was added in 3.0 (since we've renamed 2.13 into 3.0). It should look like: # @value: if specified, the value of the property (since 3.0) # # Since: 1.2
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. There might be unmerged patches on qemu-devel related to display of certain data types. Regards, Andreas > > Signed-off-by: Ricardo Perez Blanco <ricardo.perez_blanco@nokia.com> > --- > hmp.c | 13 +++++++++++-- > qapi/misc.json | 6 ++++-- > qmp.c | 7 +++++++ > qom/object.c | 8 +++----- > 4 files changed, 25 insertions(+), 9 deletions(-) [snip]
* 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 - one reason I'm not too bothered is because we've failed to get a qom-get in multiple years of trying. > There might be unmerged patches on qemu-devel related to display > of certain data types. Which ones? Dave > Regards, > Andreas > > > > > Signed-off-by: Ricardo Perez Blanco <ricardo.perez_blanco@nokia.com> > > --- > > hmp.c | 13 +++++++++++-- > > qapi/misc.json | 6 ++++-- > > qmp.c | 7 +++++++ > > qom/object.c | 8 +++----- > > 4 files changed, 25 insertions(+), 9 deletions(-) > [snip] > > -- > SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Felix Imendörffer, Jane Smithard, Graham Norton > HRB 21284 (AG Nürnberg) -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
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 - one reason I'm not too bothered is because we've > failed to get a qom-get in multiple years of trying. > > >> 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. Regards, Andreas
> -----Original Message----- > From: Andreas Färber [mailto:afaerber@suse.de] > Sent: Friday, June 8, 2018 6:20 PM > To: Dr. David Alan Gilbert <dgilbert@redhat.com> > Cc: Perez Blanco, Ricardo (Nokia - BE/Antwerp) > <ricardo.perez_blanco@nokia.com>; Eric Blake <eblake@redhat.com>; > Markus Armbruster <armbru@redhat.com>; qemu-devel <qemu- > devel@nongnu.org> > Subject: Re: [PATCH v2] Show values and description when using "qom-list" > > 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 - one reason I'm not too bothered is because > > we've failed to get a qom-get in multiple years of trying. > > It was my first attempt. But, then I realized it was much easier and straight forward to do it in qom-list. > > > >> 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. > The original patch is already quite old. I can merge again with mainstream. I imagine that this will fix it. Am I right? > Regards, > Andreas > > -- > SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG > Nürnberg)
Thanks for the comments! Replying inline. > -----Original Message----- > From: Eric Blake [mailto:eblake@redhat.com] > Sent: Friday, June 1, 2018 6:01 PM > To: Perez Blanco, Ricardo (Nokia - BE/Antwerp) > <ricardo.perez_blanco@nokia.com> > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>; Markus Armbruster > <armbru@redhat.com>; Andreas Färber <afaerber@suse.de>; open list:All > patches CC here <qemu-devel@nongnu.org> > Subject: Re: [PATCH v2] Show values and description when using "qom-list" > > On 06/01/2018 10:39 AM, Ricardo Perez Blanco wrote: > > 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. > > - Display value of the field. > > > > Signed-off-by: Ricardo Perez Blanco <ricardo.perez_blanco@nokia.com> > > --- > > > +++ b/qapi/misc.json > > @@ -1328,10 +1328,12 @@ > > # > > # @description: if specified, the description of the property. > > # > > -# Since: 1.2 > > +# @value: if specified, the value of the property. > > +# > > +# Since: 2.13 > > Not quite. The struct was first introduced in 1.2, and only 'value' was added > in 3.0 (since we've renamed 2.13 into 3.0). It should look like: > > # @value: if specified, the value of the property (since 3.0) # # Since: 1.2 > Ok, it completely makes sense. I will fix it. > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3266 > Virtualization: qemu.org | libvirt.org
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 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
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. * 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.
* 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. > * 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? Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
"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.
* 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
> -----Original Message----- > From: Dr. David Alan Gilbert [mailto:dgilbert@redhat.com] > Sent: Friday, June 29, 2018 2:17 PM > To: Markus Armbruster <armbru@redhat.com> > Cc: Perez Blanco, Ricardo (Nokia - BE/Antwerp) > <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" > > * 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.ht > > >> > ml > > >> > > >> 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.ht > > >> > ml > > >> > https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg04261.ht > > >> > ml > > >> > https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg04267.ht > > >> > ml > > >> > > >> 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 Hi guys, This discussion is out of my field of expertise. I am still a newcomer in qemu's project : D I developed the improvement in qom-list and not in qom-get as it was easier and much more straight forward to me. Moreover, for debugging purposes, I found easier to see all values in one command and not to have to execute several times qom-get. Nonetheless, I see this is a controversial topic (and I fully understand why it is) so I let up to you to make the final decision of accepting it or not. Regarding code improvements/bugs, there are still a couple of points to be addressed: -@Erik blake pointed an error in the "since version" is not correct - I recall another colleague pointing that I should merge again with mainstream. If these are the only remarks, I will deliver a v3 patch with these and it is up to you to decide to make official or not. Kr, Ricardo
"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes: > * 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: Before I answer your questions, let me reiterate: perfect's the enemy of good. Perfect is what has denied us qom-get in HMP. > a) Does keyval output here make sense (It seems reasonable, perhaps > with some wrapping etc) I guess it wouldn't be my first choice on a green field. But this field is anything but green. The data structure we need to print is a recursive QObject. QObject is basically a JSON AST. One obvious way to print it is JSON. Comptetitors in that space include YAML and TOML. Unlike these three, "keyval" isn't recursive. Instead, it uses dotted keys to express the nesting. Moreover, mapping from JSON to "keyval" can lose information (see "bridge the gap" below). But then CLI uses "keyval" syntax extensively, and HMP is related to CLI (e.g. object_add's argument is like -object's argument). Use of the same format for printing information and for reading configuration is generally appreciated by users. QMP does that by using JSON for both. qom-get and qom-set could do that by using "keyval" for both. > b) Is this the code in util/keyval.c + qapi/qobject-input-visitor.c ? Yes. util/keyval.c provides keyval_parse() to parse QEMU's traditional KEY=VALUE,... syntax into a QDict. The really traditional part of that syntax is flat. Dotted key convention provides depth, and was grafted on later. The big comment in keyval.c spells out the flaws^Wcompromises we had to accept to bridge the gap between KEY=VALUE,... and JSON. qobject_input_visitor_new_keyval() converts a QDict made with keyval_parse() into a QAPI type. > 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? For the input visitor, I chose the former, i.e. to do the keyval visitor as a variant of the existing qobject input visitor, simply because the two share so much code. I've since found talking about the two variants awkward due to difficulties naming them. Perhaps coining distinct, pithy names for the two would help. I recommend to keep output and input visitors paired, at least at the interface.
diff --git a/hmp.c b/hmp.c index a25c7bd9a8..ff3a024cd0 100644 --- a/hmp.c +++ b/hmp.c @@ -2490,8 +2490,17 @@ void hmp_qom_list(Monitor *mon, const QDict *qdict) while (list != NULL) { ObjectPropertyInfo *value = list->value; - monitor_printf(mon, "%s (%s)\n", - value->name, value->type); + monitor_printf(mon, "%s", value->name); + if (value->has_value) { + monitor_printf(mon, "=%s", value->value); + } + monitor_printf(mon, " (%s)", value->type); + if (value->has_description) { + monitor_printf(mon, "\r\t\t\t\t\t\t\t\t\t[Description: %s]", + value->description); + } + monitor_printf(mon, "\n"); + list = list->next; } qapi_free_ObjectPropertyInfoList(start); diff --git a/qapi/misc.json b/qapi/misc.json index 5636f4a149..b4bc472a37 100644 --- a/qapi/misc.json +++ b/qapi/misc.json @@ -1328,10 +1328,12 @@ # # @description: if specified, the description of the property. # -# Since: 1.2 +# @value: if specified, the value of the property. +# +# Since: 2.13 ## { 'struct': 'ObjectPropertyInfo', - 'data': { 'name': 'str', 'type': 'str', '*description': 'str' } } + 'data': { 'name': 'str', 'type': 'str', '*description': 'str', '*value': 'str' } } ## # @qom-list: diff --git a/qmp.c b/qmp.c index f72261667f..39cf656f97 100644 --- a/qmp.c +++ b/qmp.c @@ -237,6 +237,13 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp) entry->value->name = g_strdup(prop->name); entry->value->type = g_strdup(prop->type); + if (prop->description) { + entry->value->has_description = true; + entry->value->description = g_strdup(prop->description); + } + entry->value->has_value = true; + entry->value->value = g_strdup(object_property_print(obj, + entry->value->name, true, errp)); } return props; diff --git a/qom/object.c b/qom/object.c index 467795189c..4d1606e5f9 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1359,13 +1359,11 @@ char *object_property_print(Object *obj, const char *name, bool human, v = string_output_visitor_new(human, &string); object_property_get(obj, v, name, &local_err); if (local_err) { - error_propagate(errp, local_err); - goto out; + string = g_strdup("<Error reading value>"); + } else { + visit_complete(v, &string); } - visit_complete(v, &string); - -out: visit_free(v); return string; }
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. - Display value of the field. Signed-off-by: Ricardo Perez Blanco <ricardo.perez_blanco@nokia.com> --- hmp.c | 13 +++++++++++-- qapi/misc.json | 6 ++++-- qmp.c | 7 +++++++ qom/object.c | 8 +++----- 4 files changed, 25 insertions(+), 9 deletions(-)