All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: "Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: "Markus Armbruster" <armbru@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Peter Krempa" <pkrempa@redhat.com>,
	eduardo@habkost.net, antonkuchin@yandex-team.ru, mst@redhat.com,
	"reviewer:Incompatible changes" <libvir-list@redhat.com>,
	qemu-devel@nongnu.org,
	"Vladimir Sementsov-Ogievskiy" <vsementsov@yandex-team.ru>,
	den-plotnikov@yandex-team.ru, marcel.apfelbaum@gmail.com,
	pbonzini@redhat.com, eblake@redhat.com
Subject: Re: [PATCH v4 14/16] qapi: deprecate "device" field of DEVICE_* events
Date: Tue, 14 Feb 2023 17:18:18 +0100	[thread overview]
Message-ID: <875yc4gr05.fsf@pond.sub.org> (raw)
In-Reply-To: <9509eb04-16a3-9b29-f676-940f5d51d263@linaro.org> ("Philippe Mathieu-Daudé"'s message of "Tue, 14 Feb 2023 14:56:00 +0100")

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 14/2/23 13:17, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>> 
>>> On 14/2/23 12:49, Markus Armbruster wrote:
>>>> Daniel P. Berrangé <berrange@redhat.com> writes:
>> 
>> [...]
>> 
>>>>> What's the documented way to construct a QOM path, given only an ID  as
>>>>> input ?
>>>>
>>>> QOM paths a gap in our documentation, even though the composition tree
>>>> structure has been stable since day one, and is de facto ABI.
>>>>
>>>> Short answer: "/machine/peripheral/ID".
>>>>
>>>> Long answer follows.
>>>>
>>>> We have three "containers" under /machine that serve as parents for
>>>> devices:
>>>>
>>>> * /machine/peripheral/
>>>>
>>>>    Parent of user-created devices with ID.  Children are named "ID".
>>>>
>>>>    Put there by qdev_set_id(), called from qdev_device_add_from_qdict().
>>>>
>>>>    On "user-created": Nothing stops board code to abuse qdev_set_id() for
>>>>    onboard devices, directly or indirectly, but it really, really
>>>>    shouldn't.
>>>>
>>>> * /machine/peripheral-anon/
>>>>
>>>>    Parent of user-created devices without ID.  Children are named
>>>>    "device[N]", where N counts up from zero.
>>>>
>>>>    Put there by qdev_set_id(), called from qdev_device_add_from_qdict().
>>>>
>>>>    Again, abuse by board code is possible, but would be wrong.
>>>>
>>>>    Beware: a particular device's N changes when the set of devices
>>>>    created before it grows or shrinks.  Messing with the machine type can
>>>>    change it (different onboard devices).
>>>>
>>>> * /machine/unattached/
>>>>
>>>>    Surrogate parent of onboard devices created without a parent.
>>>>
>>>>    Put there by device_set_realized() (general case),
>>>>    qdev_connect_gpio_out_named() (input pins) , memory_region_do_init()
>>>>    (memory regions), qemu_create_machine() (the main sysbus).
>>>>
>>>>    I believe this container was created as a convenience, so we don't
>>>>    have to retrofit parents to existing code.  Probably abused ever
>>>>    since.
>>>
>>> Are you suggesting this is a stable interface and we can not move
>>> devices (like from /machine/unattached/ to /machine/peripheral/)
>>> without going thru the deprecation process?
>> 
>> Difficult question!
>> 
>> The point of not changing interfaces incompatibly without a grace period
>> / deprecation process is not breaking users of the interface.
>> 
>> When an interface has always worked a certain way, its users may well
>> depend on it, whether it's documented or not.
>> 
>> The question to ask is always "will this break users?"
>> 
>> For documented aspects, we generally assume it will.  Doesn't mean we
>> can simply assume "won't" for undocumented aspects.
>> 
>> Does this make sense?
>
> Yes, but I never considered the QOM paths as a stable interface...
> I'm very surprised.

I think it's a gray area.

For a good part of the QMP interface, we make an effort to review and
document, and to spell out what is stable and what isn't.  Sadly, QOM
and qdev are exceptions.

Properties are an essential part of the QMP interface, yet they are
virtually undocumented: closest we have is output of "-device
TYPENAME,help", which is utterly inadequate.  There is no systematic
review.  We've never been quite clear on which properties are part of
the stable interface.

QOM paths are a much less prominent part of the QMP interface, but they
are a part.  The structure of the QOM composition tree is undocumented.
Are they part of the stable interface?  Anybody's guess.  I figure they
weren't intended to be stable interace.  But then a QOM path is the only
way to device_del a device without ID.  Gray area.

Moreover, Hyrum's law[*] can catch up with us any time.

> "Automatically assigned to /machine/unattached/" doesn't seem
> quite stable...

The practical difficulties in (ab)using these push them towards the
unstable end of the gray area.


[*] "With a sufficient number of users of an API, it does not matter
what you promise in the contract: all observable behaviors of your
system will be depended on by somebody."



  reply	other threads:[~2023-02-14 16:18 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-13 14:00 [PATCH v4 00/16] pci hotplug tracking Vladimir Sementsov-Ogievskiy
2023-02-13 14:00 ` [PATCH v4 01/16] pci/shpc: set attention led to OFF on reset Vladimir Sementsov-Ogievskiy
2023-02-13 14:16   ` Philippe Mathieu-Daudé
2023-02-14  9:49     ` Vladimir Sementsov-Ogievskiy
2023-02-14 19:31   ` Anton Kuchin
2023-02-13 14:00 ` [PATCH v4 02/16] pci/shpc: change shpc_get_status() return type to uint8_t Vladimir Sementsov-Ogievskiy
2023-02-14 19:32   ` Anton Kuchin
2023-02-13 14:00 ` [PATCH v4 03/16] pci/shpc: shpc_slot_command(): handle PWRONLY -> ENABLED transition Vladimir Sementsov-Ogievskiy
2023-02-14 19:33   ` Anton Kuchin
2023-02-13 14:00 ` [PATCH v4 04/16] pci/shpc: more generic handle hot-unplug in shpc_slot_command() Vladimir Sementsov-Ogievskiy
2023-02-14 19:34   ` Anton Kuchin
2023-02-13 14:00 ` [PATCH v4 05/16] pci/shpc: pass PCIDevice pointer to shpc_slot_command() Vladimir Sementsov-Ogievskiy
2023-02-14 19:35   ` Anton Kuchin
2023-02-13 14:00 ` [PATCH v4 06/16] pci/shpc: refactor shpc_device_plug_common() Vladimir Sementsov-Ogievskiy
2023-02-14 19:36   ` Anton Kuchin
2023-02-13 14:00 ` [PATCH v4 07/16] pcie: pcie_cap_slot_write_config(): use correct macro Vladimir Sementsov-Ogievskiy
2023-02-14 19:37   ` Anton Kuchin
2023-02-13 14:00 ` [PATCH v4 08/16] pcie_regs: drop duplicated indicator value macros Vladimir Sementsov-Ogievskiy
2023-02-14 19:39   ` Anton Kuchin
2023-02-13 14:00 ` [PATCH v4 09/16] pcie: drop unused PCIExpressIndicator Vladimir Sementsov-Ogievskiy
2023-02-14 19:40   ` Anton Kuchin
2023-02-13 14:00 ` [PATCH v4 10/16] pcie: pcie_cap_slot_enable_power() use correct helper Vladimir Sementsov-Ogievskiy
2023-02-14 19:41   ` Anton Kuchin
2023-02-13 14:00 ` [PATCH v4 11/16] pcie: introduce pcie_sltctl_powered_off() helper Vladimir Sementsov-Ogievskiy
2023-02-14 19:41   ` Anton Kuchin
2023-02-13 14:00 ` [PATCH v4 12/16] pcie: set power indicator to off on reset by default Vladimir Sementsov-Ogievskiy
2023-02-14 19:42   ` Anton Kuchin
2023-02-13 14:01 ` [PATCH v4 13/16] pci: introduce pci_find_the_only_child() Vladimir Sementsov-Ogievskiy
2023-02-14 19:43   ` Anton Kuchin
2023-02-13 14:01 ` [PATCH v4 14/16] qapi: deprecate "device" field of DEVICE_* events Vladimir Sementsov-Ogievskiy
2023-02-13 14:13   ` Daniel P. Berrangé
2023-02-14  8:54     ` Markus Armbruster
2023-02-14  9:25       ` Peter Krempa
2023-02-14 11:14         ` Daniel P. Berrangé
2023-02-14 11:49           ` Markus Armbruster
2023-02-14 11:53             ` Philippe Mathieu-Daudé
2023-02-14 12:17               ` Markus Armbruster
2023-02-14 13:56                 ` Philippe Mathieu-Daudé
2023-02-14 16:18                   ` Markus Armbruster [this message]
2023-02-14 11:53             ` Markus Armbruster
2023-02-14 11:13       ` Daniel P. Berrangé
2023-02-14 11:57         ` Markus Armbruster
2023-02-14 13:51           ` Vladimir Sementsov-Ogievskiy
2023-02-14 13:59           ` Daniel P. Berrangé
2023-02-14 16:28             ` Markus Armbruster
2023-02-15 21:00               ` Vladimir Sementsov-Ogievskiy
2023-02-16  0:34                 ` Markus Armbruster
2023-02-13 14:01 ` [PATCH v4 15/16] qapi: add HOTPLUG_STATE event Vladimir Sementsov-Ogievskiy
2023-02-13 14:10   ` Philippe Mathieu-Daudé
2023-02-14  8:56     ` Markus Armbruster
2023-02-14  9:52       ` Vladimir Sementsov-Ogievskiy
2023-02-13 14:01 ` [PATCH v4 16/16] qapi: introduce DEVICE_ON event Vladimir Sementsov-Ogievskiy
2023-02-13 14:12   ` Philippe Mathieu-Daudé
2023-02-14  8:58     ` Markus Armbruster
2023-02-14  9:56       ` Vladimir Sementsov-Ogievskiy

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=875yc4gr05.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=antonkuchin@yandex-team.ru \
    --cc=berrange@redhat.com \
    --cc=den-plotnikov@yandex-team.ru \
    --cc=eblake@redhat.com \
    --cc=eduardo@habkost.net \
    --cc=libvir-list@redhat.com \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=pkrempa@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@yandex-team.ru \
    /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.