| Message ID | 22addebd05899a031834f63dbfe4358c5d38ac9e.1427954659.git.zhugh.fnst@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On 04/02/2015 03:50 AM, Zhu Guihua wrote: > When memory hot unplug fails, this patch adds support to send > QMP event to notify mgmt about this failure. > > Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> > --- > docs/qmp/qmp-events.txt | 17 +++++++++++++++++ > hw/acpi/memory_hotplug.c | 10 +++++++++- > monitor.c | 1 + > qapi/event.json | 14 ++++++++++++++ > trace-events | 1 + > 5 files changed, 42 insertions(+), 1 deletion(-) > > +++ b/monitor.c > @@ -587,6 +587,7 @@ static void monitor_qapi_event_init(void) > monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_REPORT_BAD, 1000); > monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_FAILURE, 1000); > monitor_qapi_event_throttle(QAPI_EVENT_VSERPORT_CHANGE, 1000); > + monitor_qapi_event_throttle(QAPI_EVENT_MEM_UNPLUG_ERROR, 1000); Is this event triggerable under guest control, or only under management control? We MUST throttle events that the guest can trigger, but it makes no sense to trigger something that can only happen under host control. > +++ b/qapi/event.json > @@ -330,3 +330,17 @@ > ## > { 'event': 'VSERPORT_CHANGE', > 'data': { 'id': 'str', 'open': 'bool' } } > + > +## > +# @MEM_UNPLUG_ERROR > +# > +# Emitted when memory hot unplug error occurs. > +# > +# @device: device name > +# > +# @msg: Informative message Any reason you abbreviated instead of spelling it out as 'message'?
On 04/10/2015 11:37 PM, Eric Blake wrote: > On 04/02/2015 03:50 AM, Zhu Guihua wrote: >> When memory hot unplug fails, this patch adds support to send >> QMP event to notify mgmt about this failure. >> >> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> >> --- >> docs/qmp/qmp-events.txt | 17 +++++++++++++++++ >> hw/acpi/memory_hotplug.c | 10 +++++++++- >> monitor.c | 1 + >> qapi/event.json | 14 ++++++++++++++ >> trace-events | 1 + >> 5 files changed, 42 insertions(+), 1 deletion(-) >> >> +++ b/monitor.c >> @@ -587,6 +587,7 @@ static void monitor_qapi_event_init(void) >> monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_REPORT_BAD, 1000); >> monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_FAILURE, 1000); >> monitor_qapi_event_throttle(QAPI_EVENT_VSERPORT_CHANGE, 1000); >> + monitor_qapi_event_throttle(QAPI_EVENT_MEM_UNPLUG_ERROR, 1000); > Is this event triggerable under guest control, or only under management > control? We MUST throttle events that the guest can trigger, but it > makes no sense to trigger something that can only happen under host control. It's under management control, this event should not be throttled, it's my mistake and I will delete this. Thanks. > >> +++ b/qapi/event.json >> @@ -330,3 +330,17 @@ >> ## >> { 'event': 'VSERPORT_CHANGE', >> 'data': { 'id': 'str', 'open': 'bool' } } >> + >> +## >> +# @MEM_UNPLUG_ERROR >> +# >> +# Emitted when memory hot unplug error occurs. >> +# >> +# @device: device name >> +# >> +# @msg: Informative message > Any reason you abbreviated instead of spelling it out as 'message'? This only refer to the spelling of event BLOCK_IMAGE_CORRUPTED in docs/qmp/qmp-events.txt. Thanks, Zhu
On 04/12/2015 07:56 PM, Zhu Guihua wrote: > > On 04/10/2015 11:37 PM, Eric Blake wrote: >> On 04/02/2015 03:50 AM, Zhu Guihua wrote: >>> When memory hot unplug fails, this patch adds support to send >>> QMP event to notify mgmt about this failure. >>> >>> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> >>> --- >>> docs/qmp/qmp-events.txt | 17 +++++++++++++++++ >>> hw/acpi/memory_hotplug.c | 10 +++++++++- >>> monitor.c | 1 + >>> qapi/event.json | 14 ++++++++++++++ >>> trace-events | 1 + >>> 5 files changed, 42 insertions(+), 1 deletion(-) >>> +## >>> +# @MEM_UNPLUG_ERROR >>> +# >>> +# Emitted when memory hot unplug error occurs. >>> +# >>> +# @device: device name >>> +# >>> +# @msg: Informative message >> Any reason you abbreviated instead of spelling it out as 'message'? > > This only refer to the spelling of event BLOCK_IMAGE_CORRUPTED > in docs/qmp/qmp-events.txt. Just because existing code abbreviated does not mean that new code needs to copy the bad example.
On 13/04/2015 16:40, Eric Blake wrote: >>>> +## >>>> >>> +# @MEM_UNPLUG_ERROR >>>> >>> +# >>>> >>> +# Emitted when memory hot unplug error occurs. >>>> >>> +# >>>> >>> +# @device: device name >>>> >>> +# >>>> >>> +# @msg: Informative message >>> >> Any reason you abbreviated instead of spelling it out as 'message'? >> > >> > This only refer to the spelling of event BLOCK_IMAGE_CORRUPTED >> > in docs/qmp/qmp-events.txt. > Just because existing code abbreviated does not mean that new code needs > to copy the bad example. To some extent it does; consistency is a good thing to have. So I think "msg" is okay here. Paolo
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt index d759d19..3be468f 100644 --- a/docs/qmp/qmp-events.txt +++ b/docs/qmp/qmp-events.txt @@ -226,6 +226,23 @@ Example: { "event": "GUEST_PANICKED", "data": { "action": "pause" } } +MEM_HOT_UNPLUG_ERROR +-------------------- +Emitted when memory hot unplug error occurs. + +Data: + +- "device": device name (json-string) +- "msg": Informative message (e.g., reason for the error) (json-string) + +Example: + +{ "event": "MEM_HOT_UNPLUG_ERROR" + "data": { "device": "dimm1", + "msg": "acpi: device unplug for unsupported device" + }, + "timestamp": { "seconds": 1265044230, "microseconds": 450486 } } + NIC_RX_FILTER_CHANGED --------------------- diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c index 35bbfeb..34cef1e 100644 --- a/hw/acpi/memory_hotplug.c +++ b/hw/acpi/memory_hotplug.c @@ -94,6 +94,7 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data, ACPIOSTInfo *info; DeviceState *dev = NULL; HotplugHandler *hotplug_ctrl = NULL; + Error *local_err = NULL; if (!mem_st->dev_count) { return; @@ -148,7 +149,14 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data, dev = DEVICE(mdev->dimm); hotplug_ctrl = qdev_get_hotplug_handler(dev); /* call pc-dimm unplug cb */ - hotplug_handler_unplug(hotplug_ctrl, dev, NULL); + hotplug_handler_unplug(hotplug_ctrl, dev, &local_err); + if (local_err) { + trace_mhp_acpi_pc_dimm_delete_failed(mem_st->selector); + qapi_event_send_mem_unplug_error(dev->id, + error_get_pretty(local_err), + &error_abort); + break; + } trace_mhp_acpi_pc_dimm_deleted(mem_st->selector); } break; diff --git a/monitor.c b/monitor.c index 68873ec..d52ab87 100644 --- a/monitor.c +++ b/monitor.c @@ -587,6 +587,7 @@ static void monitor_qapi_event_init(void) monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_REPORT_BAD, 1000); monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_FAILURE, 1000); monitor_qapi_event_throttle(QAPI_EVENT_VSERPORT_CHANGE, 1000); + monitor_qapi_event_throttle(QAPI_EVENT_MEM_UNPLUG_ERROR, 1000); qmp_event_set_func_emit(monitor_qapi_event_queue); } diff --git a/qapi/event.json b/qapi/event.json index c51dc49..378dda5 100644 --- a/qapi/event.json +++ b/qapi/event.json @@ -330,3 +330,17 @@ ## { 'event': 'VSERPORT_CHANGE', 'data': { 'id': 'str', 'open': 'bool' } } + +## +# @MEM_UNPLUG_ERROR +# +# Emitted when memory hot unplug error occurs. +# +# @device: device name +# +# @msg: Informative message +# +# Since: 2.4 +## +{ 'event': 'MEM_UNPLUG_ERROR', + 'data': { 'device': 'str', 'msg': 'str' } } diff --git a/trace-events b/trace-events index 46f6ef0..11387c3 100644 --- a/trace-events +++ b/trace-events @@ -1575,6 +1575,7 @@ mhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "slot[0x%"PRIx32"] OST STA mhp_acpi_clear_insert_evt(uint32_t slot) "slot[0x%"PRIx32"] clear insert event" mhp_acpi_clear_remove_evt(uint32_t slot) "slot[0x%"PRIx32"] clear remove event" mhp_acpi_pc_dimm_deleted(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm deleted" +mhp_acpi_pc_dimm_delete_failed(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm delete failed" # hw/i386/pc.c mhp_pc_dimm_assigned_slot(int slot) "0x%d"
When memory hot unplug fails, this patch adds support to send QMP event to notify mgmt about this failure. Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- docs/qmp/qmp-events.txt | 17 +++++++++++++++++ hw/acpi/memory_hotplug.c | 10 +++++++++- monitor.c | 1 + qapi/event.json | 14 ++++++++++++++ trace-events | 1 + 5 files changed, 42 insertions(+), 1 deletion(-)