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?

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.


  #
  # @device: the device's ID if it has one
  #
  # @path: the device's QOM path
  #
+# Since: 8.0
+##
+{ 'struct': 'DeviceAndPath',
+  'data': { '*device': 'str', 'path': 'str' } }
+

Should be Since: 8.1 no?

Yes.

Please format like

    ##
    # @DeviceAndPath:
    #
    # 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.
    #
    # @device: the device's ID if it has one
    #
    # @path: the device's QOM path
    #
    # Since: 8.0
    ##

to blend in with recent commit a937b6aa739 (qapi: Reformat doc comments
to conform to current conventions).

+##
+# @DEVICE_DELETED:
+#
+# 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.
+#

Conflict resolution:

     # 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.

  # Since: 1.5
  #
  # Example:
@@ -134,18 +155,13 @@
  #      "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
  #
  ##
-{ 'event': 'DEVICE_DELETED',
-  'data': { '*device': 'str', 'path': 'str' } }
+{ 'event': 'DEVICE_DELETED', 'data': 'DeviceAndPath' }
##
  # @DEVICE_UNPLUG_GUEST_ERROR:
  #
  # Emitted when a device hot unplug fails due to a guest reported error.
  #
-# @device: the device's ID if it has one
-#
-# @path: the device's QOM path
-#
  # Since: 6.2
  #
  # Example:
@@ -156,5 +172,4 @@
  #      "timestamp": { "seconds": 1615570772, "microseconds": 202844 } }
  #
  ##
-{ 'event': 'DEVICE_UNPLUG_GUEST_ERROR',
-  'data': { '*device': 'str', 'path': 'str' } }
+{ 'event': 'DEVICE_UNPLUG_GUEST_ERROR', 'data': 'DeviceAndPath' }
--
2.34.1


--
Best regards,
Vladimir


Reply via email to