hw/core/hotplug.c | 12 +++ hw/pci-bridge/pci_bridge_dev.c | 14 +++ hw/pci-bridge/pcie_pci_bridge.c | 1 + hw/pci/pcie.c | 83 +++++++++++++++ hw/pci/pcie_port.c | 1 + hw/pci/shpc.c | 86 +++++++++++++++ include/hw/hotplug.h | 11 ++ include/hw/pci/pci_bridge.h | 2 + include/hw/pci/pcie.h | 2 + include/hw/pci/shpc.h | 2 + include/hw/qdev-core.h | 7 ++ include/monitor/qdev.h | 6 ++ qapi/qdev.json | 178 +++++++++++++++++++++++++++++--- softmmu/qdev-monitor.c | 58 +++++++++++ 14 files changed, 451 insertions(+), 12 deletions(-)
Hi all! Main thing this series does is DEVICE_ON event - a counter-part to DEVICE_DELETED. A guest-driven event that device is powered-on. Details are in patch 2. The new event is paried with corresponding command query-hotplug. v8: - improve naming, wording and style - make new QMP interface experimental Vladimir Sementsov-Ogievskiy (4): qapi/qdev.json: unite DEVICE_* event data into single structure qapi: add DEVICE_ON and query-hotplug infrastructure shpc: implement DEVICE_ON event and query-hotplug pcie: implement DEVICE_ON event and query-hotplug hw/core/hotplug.c | 12 +++ hw/pci-bridge/pci_bridge_dev.c | 14 +++ hw/pci-bridge/pcie_pci_bridge.c | 1 + hw/pci/pcie.c | 83 +++++++++++++++ hw/pci/pcie_port.c | 1 + hw/pci/shpc.c | 86 +++++++++++++++ include/hw/hotplug.h | 11 ++ include/hw/pci/pci_bridge.h | 2 + include/hw/pci/pcie.h | 2 + include/hw/pci/shpc.h | 2 + include/hw/qdev-core.h | 7 ++ include/monitor/qdev.h | 6 ++ qapi/qdev.json | 178 +++++++++++++++++++++++++++++--- softmmu/qdev-monitor.c | 58 +++++++++++ 14 files changed, 451 insertions(+), 12 deletions(-) -- 2.34.1
On Thu, Oct 05, 2023 at 12:29:22PM +0300, Vladimir Sementsov-Ogievskiy wrote: > Hi all! > > Main thing this series does is DEVICE_ON event - a counter-part to > DEVICE_DELETED. A guest-driven event that device is powered-on. > Details are in patch 2. The new event is paried with corresponding > command query-hotplug. Several things questionable here: 1. depending on guest activity you can get as many DEVICE_ON events as you like 2. it's just for shpc and native pcie - things are confusing enough for management, we should make sure it can work for all devices 3. what about non hotpluggable devices? do we want the event for them? I feel this needs actual motivation so we can judge what's the right way to do it. > > v8: > - improve naming, wording and style > - make new QMP interface experimental > > > Vladimir Sementsov-Ogievskiy (4): > qapi/qdev.json: unite DEVICE_* event data into single structure > qapi: add DEVICE_ON and query-hotplug infrastructure > shpc: implement DEVICE_ON event and query-hotplug > pcie: implement DEVICE_ON event and query-hotplug > > hw/core/hotplug.c | 12 +++ > hw/pci-bridge/pci_bridge_dev.c | 14 +++ > hw/pci-bridge/pcie_pci_bridge.c | 1 + > hw/pci/pcie.c | 83 +++++++++++++++ > hw/pci/pcie_port.c | 1 + > hw/pci/shpc.c | 86 +++++++++++++++ > include/hw/hotplug.h | 11 ++ > include/hw/pci/pci_bridge.h | 2 + > include/hw/pci/pcie.h | 2 + > include/hw/pci/shpc.h | 2 + > include/hw/qdev-core.h | 7 ++ > include/monitor/qdev.h | 6 ++ > qapi/qdev.json | 178 +++++++++++++++++++++++++++++--- > softmmu/qdev-monitor.c | 58 +++++++++++ > 14 files changed, 451 insertions(+), 12 deletions(-) > > -- > 2.34.1
On 02.11.23 14:31, Michael S. Tsirkin wrote: > On Thu, Oct 05, 2023 at 12:29:22PM +0300, Vladimir Sementsov-Ogievskiy wrote: >> Hi all! >> >> Main thing this series does is DEVICE_ON event - a counter-part to >> DEVICE_DELETED. A guest-driven event that device is powered-on. >> Details are in patch 2. The new event is paried with corresponding >> command query-hotplug. > > Several things questionable here: > 1. depending on guest activity you can get as many > DEVICE_ON events as you like No, I've made it so it may be sent only once per device > 2. it's just for shpc and native pcie - things are > confusing enough for management, we should make sure > it can work for all devices Agree, I'm thinking about it > 3. what about non hotpluggable devices? do we want the event for them? > I think, yes, especially if we make async=true|false flag for device_add, so that successful device_add must be always followed by DEVICE_ON - like device_del is followed by DEVICE_DELETED. Maybe, to generalize, it should be called not DEVICE_ON (which mostly relate to hotplug controller statuses) but DEVICE_ADDED - a full counterpart for DEVICE_DELETED. > > I feel this needs actual motivation so we can judge what's the > right way to do it. My first motivation for this series was the fact that successful device_add doesn't guarantee that hard disk successfully hotplugged to the guest. It relates to some problems with shpc/pcie hotplug we had in the past, and they are mostly fixed. But still, for management tool it's good to understand that all actions related to hotplug controller are done and we have "green light". Recently new motivation come, as I described in my "ping" letter <6bd19a07-5224-464d-b54d-1d738f5ba8f7@yandex-team.ru>, that we have a performance degradation because of 7bed89958bfbf40df, which introduces drain_call_rcu() in device_add, to make it more synchronous. So, my suggestion is make it instead more asynchronous (probably with special flag) and rely on DEVICE_ON event. > > >> >> v8: >> - improve naming, wording and style >> - make new QMP interface experimental >> >> >> Vladimir Sementsov-Ogievskiy (4): >> qapi/qdev.json: unite DEVICE_* event data into single structure >> qapi: add DEVICE_ON and query-hotplug infrastructure >> shpc: implement DEVICE_ON event and query-hotplug >> pcie: implement DEVICE_ON event and query-hotplug >> >> hw/core/hotplug.c | 12 +++ >> hw/pci-bridge/pci_bridge_dev.c | 14 +++ >> hw/pci-bridge/pcie_pci_bridge.c | 1 + >> hw/pci/pcie.c | 83 +++++++++++++++ >> hw/pci/pcie_port.c | 1 + >> hw/pci/shpc.c | 86 +++++++++++++++ >> include/hw/hotplug.h | 11 ++ >> include/hw/pci/pci_bridge.h | 2 + >> include/hw/pci/pcie.h | 2 + >> include/hw/pci/shpc.h | 2 + >> include/hw/qdev-core.h | 7 ++ >> include/monitor/qdev.h | 6 ++ >> qapi/qdev.json | 178 +++++++++++++++++++++++++++++--- >> softmmu/qdev-monitor.c | 58 +++++++++++ >> 14 files changed, 451 insertions(+), 12 deletions(-) >> >> -- >> 2.34.1 > -- Best regards, Vladimir
Ping.
And some addition. We have the case, when the commit
commit 7bed89958bfbf40df9ca681cefbdca63abdde39d
Author: Maxim Levitsky <mlevitsk@redhat.com>
Date: Tue Oct 6 14:38:58 2020 +0200
device_core: use drain_call_rcu in in qmp_device_add
Soon, a device removal might only happen on RCU callback execution.
This is okay for device-del which provides a DEVICE_DELETED event,
but not for the failure case of device-add. To avoid changing
monitor semantics, just drain all pending RCU callbacks on error.
sensibly slows down vm initialization (several calls to device_add of pc-dimm).
And looking at commit message, I see that what I do in the series is exactly a suggestion to change monitor semantics.
What do you think?
Maybe we need a boolean "async" parameter for device_add, which will turn off drain_call_rcu() call and rely on user to handle DEVICE_ON?
On 05.10.23 12:29, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
>
> Main thing this series does is DEVICE_ON event - a counter-part to
> DEVICE_DELETED. A guest-driven event that device is powered-on.
> Details are in patch 2. The new event is paried with corresponding
> command query-hotplug.
>
>
> v8:
> - improve naming, wording and style
> - make new QMP interface experimental
>
>
> Vladimir Sementsov-Ogievskiy (4):
> qapi/qdev.json: unite DEVICE_* event data into single structure
> qapi: add DEVICE_ON and query-hotplug infrastructure
> shpc: implement DEVICE_ON event and query-hotplug
> pcie: implement DEVICE_ON event and query-hotplug
>
> hw/core/hotplug.c | 12 +++
> hw/pci-bridge/pci_bridge_dev.c | 14 +++
> hw/pci-bridge/pcie_pci_bridge.c | 1 +
> hw/pci/pcie.c | 83 +++++++++++++++
> hw/pci/pcie_port.c | 1 +
> hw/pci/shpc.c | 86 +++++++++++++++
> include/hw/hotplug.h | 11 ++
> include/hw/pci/pci_bridge.h | 2 +
> include/hw/pci/pcie.h | 2 +
> include/hw/pci/shpc.h | 2 +
> include/hw/qdev-core.h | 7 ++
> include/monitor/qdev.h | 6 ++
> qapi/qdev.json | 178 +++++++++++++++++++++++++++++---
> softmmu/qdev-monitor.c | 58 +++++++++++
> 14 files changed, 451 insertions(+), 12 deletions(-)
>
--
Best regards,
Vladimir
[cc Peter, Nikolay and libvirt list] On 02.11.23 11:06, Vladimir Sementsov-Ogievskiy wrote: > Ping. > > And some addition. We have the case, when the commit > > commit 7bed89958bfbf40df9ca681cefbdca63abdde39d > Author: Maxim Levitsky <mlevitsk@redhat.com> > Date: Tue Oct 6 14:38:58 2020 +0200 > > device_core: use drain_call_rcu in in qmp_device_add > Soon, a device removal might only happen on RCU callback execution. > This is okay for device-del which provides a DEVICE_DELETED event, > but not for the failure case of device-add. To avoid changing > monitor semantics, just drain all pending RCU callbacks on error. > > sensibly slows down vm initialization (several calls to device_add of pc-dimm). > > And looking at commit message, I see that what I do in the series is exactly a suggestion to change monitor semantics. > > What do you think? > > Maybe we need a boolean "async" parameter for device_add, which will turn off drain_call_rcu() call and rely on user to handle DEVICE_ON? > > On 05.10.23 12:29, Vladimir Sementsov-Ogievskiy wrote: >> Hi all! >> >> Main thing this series does is DEVICE_ON event - a counter-part to >> DEVICE_DELETED. A guest-driven event that device is powered-on. >> Details are in patch 2. The new event is paried with corresponding >> command query-hotplug. >> >> >> v8: >> - improve naming, wording and style >> - make new QMP interface experimental >> >> >> Vladimir Sementsov-Ogievskiy (4): >> qapi/qdev.json: unite DEVICE_* event data into single structure >> qapi: add DEVICE_ON and query-hotplug infrastructure >> shpc: implement DEVICE_ON event and query-hotplug >> pcie: implement DEVICE_ON event and query-hotplug >> >> hw/core/hotplug.c | 12 +++ >> hw/pci-bridge/pci_bridge_dev.c | 14 +++ >> hw/pci-bridge/pcie_pci_bridge.c | 1 + >> hw/pci/pcie.c | 83 +++++++++++++++ >> hw/pci/pcie_port.c | 1 + >> hw/pci/shpc.c | 86 +++++++++++++++ >> include/hw/hotplug.h | 11 ++ >> include/hw/pci/pci_bridge.h | 2 + >> include/hw/pci/pcie.h | 2 + >> include/hw/pci/shpc.h | 2 + >> include/hw/qdev-core.h | 7 ++ >> include/monitor/qdev.h | 6 ++ >> qapi/qdev.json | 178 +++++++++++++++++++++++++++++--- >> softmmu/qdev-monitor.c | 58 +++++++++++ >> 14 files changed, 451 insertions(+), 12 deletions(-) >> > -- Best regards, Vladimir
© 2016 - 2024 Red Hat, Inc.