From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49618) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YcQ4t-0004ND-P2 for qemu-devel@nongnu.org; Sun, 29 Mar 2015 23:09:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YcQ4q-0001sD-JM for qemu-devel@nongnu.org; Sun, 29 Mar 2015 23:09:27 -0400 Received: from [59.151.112.132] (port=12141 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YcQ4p-0001rY-F9 for qemu-devel@nongnu.org; Sun, 29 Mar 2015 23:09:24 -0400 Message-ID: <5518BDFE.3010801@cn.fujitsu.com> Date: Mon, 30 Mar 2015 11:07:42 +0800 From: Zhu Guihua MIME-Version: 1.0 References: <55156F03.8090700@redhat.com> In-Reply-To: <55156F03.8090700@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 7/7] qmp-event: add event notification for memory hot unplug error List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org, imammedo@redhat.com, pbonzini@redhat.com Cc: guz.fnst@cn.fujitsu.com, izumi.taku@jp.fujitsu.com, tangchen@cn.fujitsu.com On 03/27/2015 10:53 PM, Eric Blake wrote: > On 03/27/2015 03:20 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 >> --- >> docs/qmp/qmp-events.txt | 17 +++++++++++++++++ >> hw/acpi/memory_hotplug.c | 8 +++++++- >> monitor.c | 1 + >> qapi/event.json | 14 ++++++++++++++ >> 4 files changed, 39 insertions(+), 1 deletion(-) >> >> diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt >> index d759d19..7a05705 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 occurs error. > s/occurs error/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 not supported device" > s/not supported/unsupported/ > >> + }, >> + "timestamp": { "seconds": 1265044230, "microseconds": 450486 } } >> + >> NIC_RX_FILTER_CHANGED >> --------------------- >> >> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c >> index 2a1b866..f1cef71 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,12 @@ 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) { >> + qapi_event_send_mem_unplug_error(dev->id, >> + error_get_pretty(local_err), >> + &error_abort); >> + } >> } > Does this leak local_err? Here should handle this error not to forget it. It is a good way to send qmp event to notify mgmt. > >> +++ b/qapi/event.json >> @@ -330,3 +330,17 @@ >> ## >> { 'event': 'VSERPORT_CHANGE', >> 'data': { 'id': 'str', 'open': 'bool' } } >> + >> +## >> +# @MEM_UNPLUG_ERROR >> +# >> +# Emitted when memory hot unplug occurs error. > s/occurs error/error occurs/ > >> +# >> +# @device: device name >> +# >> +# @msg: Informative message >> +# >> +# Since: 2.3 > You're awfully late for 2.3; this may need to be 2.4. Got it, thanks. Regards, Zhu > >> +## >> +{ 'event': 'MEM_UNPLUG_ERROR', >> + 'data': { 'device': 'str', 'msg': 'str' } } >>