Re: [PATCH 1/5] docs/devel: document expectations for QAPI data modelling for QMP
On Fri, Sep 10, 2021 at 03:45:11PM +0200, Markus Armbruster wrote: > Daniel P. Berrangé writes: > > > On Thu, Sep 09, 2021 at 11:33:20AM +0200, Markus Armbruster wrote: > > [...] > > >> 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 ? > > We don't. I'm asking for a favour. > > Humans tend to have trouble following long lines with our eyes (I sure > do). Typographic manuals suggest to limit columns to roughly 60 > characters for exactly that reason[1]. > > 70 is a reasonable compromise between legibility and "writability". For > code, we use 80-90, because there the actual width should still be fine > due to indentation. > > checkpatch.pl doesn't differentiate betwen code and prose. > > This file is already hard to read for me. Please consider not making it > harder. Ok, no worries, I just didn't understand the rationale. 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 :|
Re: [PATCH 1/5] docs/devel: document expectations for QAPI data modelling for QMP
Daniel P. Berrangé writes: > On Thu, Sep 09, 2021 at 11:33:20AM +0200, Markus Armbruster wrote: [...] >> 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 ? We don't. I'm asking for a favour. Humans tend to have trouble following long lines with our eyes (I sure do). Typographic manuals suggest to limit columns to roughly 60 characters for exactly that reason[1]. 70 is a reasonable compromise between legibility and "writability". For code, we use 80-90, because there the actual width should still be fine due to indentation. checkpatch.pl doesn't differentiate betwen code and prose. This file is already hard to read for me. Please consider not making it harder. [1] https://en.wikipedia.org/wiki/Column_(typography)#Typographic_style
Re: [PATCH 1/5] docs/devel: document expectations for QAPI data modelling for QMP
On Thu, Sep 09, 2021 at 11:33:20AM +0200, Markus Armbruster wrote: > Daniel P. Berrangé 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é > > --- > > 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 :|
Re: [PATCH 1/5] docs/devel: document expectations for QAPI data modelling for QMP
Daniel P. Berrangé 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é > --- > docs/devel/writing-qmp-commands.rst | 25 + > 1 file changed, 25 insertions(+) > > diff --git a/docs/devel/writing-qmp-commands.rst > b/docs/devel/writing-qmp-commands.rst > index 6a10a06c48..d032daa62d 100644 > --- a/docs/devel/writing-qmp-commands.rst > +++ b/docs/devel/writing-qmp-commands.rst > @@ -350,6 +350,31 @@ In this section we will focus on user defined types. > Please, check the QAPI > documentation for information about the other types. > > > +Modelling data in QAPI > +~~ Outline now: How to write QMP commands using the QAPI framework Overview Testing Writing a command that doesn't return data Arguments Errors Command Documentation Implementing the HMP command Writing a command that returns data --> Modelling data in QAPI User Defined Types The HMP command Returning Lists Awkward. I guess you wanted it next to "User Defined Types", which makes some sense. Perhaps minor tweaks (headlines, maybe a bit of text as well) would suffice: How to write QMP commands using the QAPI framework Overview Testing Writing a simple command: hello-world Arguments Errors Command Documentation Implementing the HMP command Writing more complex commands Modelling data in QAPI User Defined Types The HMP command Returning Lists > + > +For a QMP command that to be considered stable and supported long term there "term, there" > +is a requirement returned data should be explicitly modelled using fine > grained "fine-grained", I think. > +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
Re: [PATCH 1/5] docs/devel: document expectations for QAPI data modelling for QMP
On Wed, Sep 08, 2021 at 11:37:07AM +0100, Daniel P. Berrangé wrote: > 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é > --- > docs/devel/writing-qmp-commands.rst | 25 + > 1 file changed, 25 insertions(+) > > diff --git a/docs/devel/writing-qmp-commands.rst > b/docs/devel/writing-qmp-commands.rst > index 6a10a06c48..d032daa62d 100644 > --- a/docs/devel/writing-qmp-commands.rst > +++ b/docs/devel/writing-qmp-commands.rst > @@ -350,6 +350,31 @@ In this section we will focus on user defined types. > Please, check the QAPI > documentation for information about the other types. > > > +Modelling data in QAPI > +~~ > + > +For a QMP command that to be considered stable and supported long term there > +is a requirement returned data should be explicitly modelled using fine > grained > +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 then > +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 ad hoc > +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 > +that contains formatted data, however, it is mandatory for the command to use > +the 'x-' name prefix. This indicates that the command is not guaranteed to be > +long term stable / liable to change in future and is not following QAPI > design > +best practices. An example where this approach is taken is the QMP command > +"x-query-registers". This returns a printf formatted dump of the architecture > +specific CPU state. The way the data is formatted varies across QEMU targets, > +is liable to change over time, and is only intended to be consumed as an > opaque > +string by machines. > + > User Defined Types > ~~ > > -- > 2.31.1 > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[PATCH 1/5] docs/devel: document expectations for QAPI data modelling for QMP
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é --- docs/devel/writing-qmp-commands.rst | 25 + 1 file changed, 25 insertions(+) diff --git a/docs/devel/writing-qmp-commands.rst b/docs/devel/writing-qmp-commands.rst index 6a10a06c48..d032daa62d 100644 --- a/docs/devel/writing-qmp-commands.rst +++ b/docs/devel/writing-qmp-commands.rst @@ -350,6 +350,31 @@ In this section we will focus on user defined types. Please, check the QAPI documentation for information about the other types. +Modelling data in QAPI +~~ + +For a QMP command that to be considered stable and supported long term there +is a requirement returned data should be explicitly modelled using fine grained +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 +that contains formatted data, however, it is mandatory for the command to use +the 'x-' name prefix. This indicates that the command is not guaranteed to be +long term stable / liable to change in future and is not following QAPI design +best practices. An example where this approach is taken is the QMP command +"x-query-registers". This returns a printf formatted dump of the architecture +specific CPU state. The way the data is formatted varies across QEMU targets, +is liable to change over time, and is only intended to be consumed as an opaque +string by machines. + User Defined Types ~~ -- 2.31.1
