On 22.05.23 15:13, Markus Armbruster wrote:
Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru> writes:

On 22.05.23 12:27, Markus Armbruster wrote:
"Michael S. Tsirkin" <m...@redhat.com> writes:

On Fri, Apr 21, 2023 at 01:32:04PM +0300, Vladimir Sementsov-Ogievskiy wrote:
DEVICE_DELETED and DEVICE_UNPLUG_GUEST_ERROR has equal data, let's
refactor it to one structure. That also helps to add new events
consistently.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>

Can QAPI maintainers please review this patchset?
It's been a month.

It's been a busy month; sorry for the delay.

---
   qapi/qdev.json | 39 +++++++++++++++++++++++++++------------
   1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/qapi/qdev.json b/qapi/qdev.json
index 2708fb4e99..135cd81586 100644
--- a/qapi/qdev.json
+++ b/qapi/qdev.json
@@ -114,16 +114,37 @@
   { 'command': 'device_del', 'data': {'id': 'str'} }
##
-# @DEVICE_DELETED:
+# @DeviceAndPath:
   #
-# Emitted whenever the device removal completion is acknowledged by the guest.
-# At this point, it's safe to reuse the specified device ID. Device removal can
-# be initiated by the guest or by HMP/QMP commands.
+# In events we designate devices by both their ID (if the device has one)
+# and QOM path.
+#
+# Why we need ID? User specify ID in device_add command and in command line
+# and expects same identifier in the event data.
+#
+# Why we need QOM path? Some devices don't have ID and we still want to emit
+# events for them.
+#
+# So, we have a bit of redundancy, as QOM path for device that has ID is
+# always /machine/peripheral/ID. But that's hard to change keeping both
+# simple interface for most users and universality for the generic case.

Hmm.  I appreciate rationale, but I'm not sure it fits here.  Would
readers be worse off if we dropped it?

Is there a syntax to add comment to the QAPI structure, which doesn't go into 
compiled public documentation?

Yes!  qapi-code-gen.rst: "A multi-line comment that starts and ends with
a ``##`` line is a documentation comment."  All other comments are not,
and won't be included in generated documentation.

Good, thanks!


Example: qapi/qapi-schema.json has

     { 'include': 'pragma.json' }

     # Documentation generated with qapi-gen.py is in source order, with
     # included sub-schemas inserted at the first include directive
     # (subsequent include directives have no effect).  To get a sane and
     # stable order, it's best to include each sub-schema just once, or
     # include it first right here.

     { 'include': 'error.json' }

Not a documentation comment, thus not included in generated
documentation.

Additionally, TODO sections in documentation comments are omitted from
generated documentation.  qapi-code-gen.rst again: "TODO" sections are
not rendered at all (they are for developers, not users of QMP).

I agree that we don't need this in compiled documentation, but this place in 
the code really good for the rationale, to avoid starting the discussion from 
the beginning again.

Saving rationale so we can refer to it later is good.  We tend to use
commit messages for that.  I'd say use comments when the rationale needs
to be more visible.


Yes I can move this to the commit message. I just wasn't sure that it's an 
obvious place in our case, as that's not a commit that introduces new structure 
but just a no-logic-change refactoring.

--
Best regards,
Vladimir


Reply via email to