Re: [Qemu-devel] [PATCH v5 7/7] qmp-event: add event notification for memory hot unplug error

[Zhu Guihua]

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 <address@hidden>
> > ---
> >   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' } }