Patchwork [RFC,v2,13/21] Implement memory hotplug notification lists

login
register
mail settings
Submitter Vasilis Liaskovitis
Date July 11, 2012, 10:31 a.m.
Message ID <1342002726-18258-14-git-send-email-vasilis.liaskovitis@profitbricks.com>
Download mbox | patch
Permalink /patch/170431/
State New
Headers show

Comments

Vasilis Liaskovitis - July 11, 2012, 10:31 a.m.
Guest can respond to ACPI hotplug events e.g. with _EJ or _OST method.
This patch implements a tail queue to store guest notifications for memory
hot-add and hot-remove requests.

Guest responses for memory hotplug command on a per-dimm basis can be detected
with the new hmp command "info memhp" or the new qmp command "query-memhp"
Examples:
    
(qemu) dimm_add dimm0
(qemu) info memhp
Dimm: dimm0 hot-add success
or
Dimm: dimm0 hot-add failure
    
(qemu) dimm_del dimm0
(qemu) info memhp
Dimm: dimm0 hot-remove success
or
Dimm: dimm0 hot-remove failure

Results are removed from the queue once read.

This patch only queues _EJ events that signal hot-remove success.
For  _OST event queuing, which cover the hot-remove failure and
hot-add success/failure cases, the next 2 patches are also needed.

These notification items should probably be part of migration state (not yet
implemented)

Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
---
 hmp-commands.hx  |    2 +
 hmp.c            |   17 ++++++++++++++++
 hmp.h            |    1 +
 hw/dimm.c        |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/dimm.h        |    6 +++++
 monitor.c        |    7 ++++++
 qapi-schema.json |   26 +++++++++++++++++++++++++
 qmp-commands.hx  |   38 +++++++++++++++++++++++++++++++++++++
 8 files changed, 152 insertions(+), 0 deletions(-)
Eric Blake - July 11, 2012, 2:59 p.m.
On 07/11/2012 04:31 AM, Vasilis Liaskovitis wrote:
> Guest can respond to ACPI hotplug events e.g. with _EJ or _OST method.
> This patch implements a tail queue to store guest notifications for memory
> hot-add and hot-remove requests.
> 
> Guest responses for memory hotplug command on a per-dimm basis can be detected
> with the new hmp command "info memhp" or the new qmp command "query-memhp"
> Examples:
>     

> +++ b/qapi-schema.json
> @@ -1862,3 +1862,29 @@
>  # Since: 0.14.0
>  ##
>  { 'command': 'netdev_del', 'data': {'id': 'str'} }
> +
> +##
> +# @MemHpInfo:
> +#
> +# Information about status of a memory hotplug command
> +#
> +# @Dimm: the Dimm associated with the result
> +#
> +# @result: the result of the hotplug command
> +#
> +# Since: 1.1.3

Should probably be 1.2, not 1.1.3.

> +#
> +##
> +{ 'type': 'MemHpInfo',
> +  'data': {'Dimm': 'str', 'request': 'str', 'result': 'str'} }

Why the upper case?  Wouldn't 'dimm' be more consistent?

> +
> +##
> +# @query-memhp:

Why are we abbreviating?  It might be better to name the QMP command
query-memory-hotplug

> +#
> +# Returns a list of information about pending hotplug commands
> +#
> +# Returns: a list of @MemhpInfo
> +#
> +# Since: 1.1.3

Likewise for 1.2.

> +
> +- "Dimm": Dimm name (json-str)
> +- "request": type of hot request: hot-add or hot-remove  (json-str)
> +- "result": result of the hotplug request for this Dimm success or failure (json-str)

This may need tweaks (such as s/Dimm/dimm/) based on resolution of above
comments.
Vasilis Liaskovitis - July 11, 2012, 4:47 p.m.
Hi,

On Wed, Jul 11, 2012 at 08:59:03AM -0600, Eric Blake wrote:
> On 07/11/2012 04:31 AM, Vasilis Liaskovitis wrote:
> > Guest can respond to ACPI hotplug events e.g. with _EJ or _OST method.
> > This patch implements a tail queue to store guest notifications for memory
> > hot-add and hot-remove requests.
> > 
> > Guest responses for memory hotplug command on a per-dimm basis can be detected
> > with the new hmp command "info memhp" or the new qmp command "query-memhp"
> > Examples:
> >     
> 
> > +++ b/qapi-schema.json
> > @@ -1862,3 +1862,29 @@
> >  # Since: 0.14.0
> >  ##
> >  { 'command': 'netdev_del', 'data': {'id': 'str'} }
> > +
> > +##
> > +# @MemHpInfo:
> > +#
> > +# Information about status of a memory hotplug command
> > +#
> > +# @Dimm: the Dimm associated with the result
> > +#
> > +# @result: the result of the hotplug command
> > +#
> > +# Since: 1.1.3
> 
> Should probably be 1.2, not 1.1.3.
>

right

> > +#
> > +##
> > +{ 'type': 'MemHpInfo',
> > +  'data': {'Dimm': 'str', 'request': 'str', 'result': 'str'} }
> 
> Why the upper case?  Wouldn't 'dimm' be more consistent?
>

I will change to "dimm"

> > +
> > +##
> > +# @query-memhp:
> 
> Why are we abbreviating?  It might be better to name the QMP command
> query-memory-hotplug
> 

agreed, memhp is a bit cryptic. I will change to your suggestion 

> > +#
> > +# Returns a list of information about pending hotplug commands
> > +#
> > +# Returns: a list of @MemhpInfo
> > +#
> > +# Since: 1.1.3
> 
> Likewise for 1.2.

right

> 
> > +
> > +- "Dimm": Dimm name (json-str)
> > +- "request": type of hot request: hot-add or hot-remove  (json-str)
> > +- "result": result of the hotplug request for this Dimm success or failure (json-str)
> 
> This may need tweaks (such as s/Dimm/dimm/) based on resolution of above
> comments.
ok, it will be "dimm"

thanks,

- Vasilis

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 012c150..3172cde 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1459,6 +1459,8 @@  show device tree
 show qdev device model list
 @item info roms
 show roms
+@item info memhp
+show memhp
 @end table
 ETEXI
 
diff --git a/hmp.c b/hmp.c
index b9cec1d..ec25d9a 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1000,3 +1000,20 @@  void hmp_netdev_del(Monitor *mon, const QDict *qdict)
     qmp_netdev_del(id, &err);
     hmp_handle_error(mon, &err);
 }
+
+void hmp_info_memhp(Monitor *mon)
+{
+    MemHpInfoList *info;
+    MemHpInfoList *item;
+    MemHpInfo *dimm;
+
+    info = qmp_query_memhp(NULL);
+    for (item = info; item; item = item->next) {
+        dimm = item->value;
+        monitor_printf(mon, "Dimm: %s %s %s\n", dimm->Dimm,
+                dimm->request, dimm->result);
+        dimm->Dimm = NULL;
+    }
+
+    qapi_free_MemHpInfoList(info);
+}
diff --git a/hmp.h b/hmp.h
index 79d138d..971e7c4 100644
--- a/hmp.h
+++ b/hmp.h
@@ -64,5 +64,6 @@  void hmp_device_del(Monitor *mon, const QDict *qdict);
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
 void hmp_netdev_add(Monitor *mon, const QDict *qdict);
 void hmp_netdev_del(Monitor *mon, const QDict *qdict);
+void hmp_info_memhp(Monitor *mon);
 
 #endif
diff --git a/hw/dimm.c b/hw/dimm.c
index 00c4623..9b32386 100644
--- a/hw/dimm.c
+++ b/hw/dimm.c
@@ -26,6 +26,7 @@ 
 static DeviceState *dimm_hotplug_qdev;
 static dimm_hotplug_fn dimm_hotplug;
 static QTAILQ_HEAD(Dimmlist, DimmState)  dimmlist;
+static QTAILQ_HEAD(dimm_hp_result_head, dimm_hp_result)  dimm_hp_result_queue;
 
 static Property dimm_properties[] = {
     DEFINE_PROP_END_OF_LIST()
@@ -189,16 +190,69 @@  void dimm_notify(uint32_t idx, uint32_t event)
     DimmState *s;
     s = dimm_find_from_idx(idx);
     assert(s != NULL);
+    struct dimm_hp_result *result = g_malloc0(sizeof(*result));
 
+    result->s = s;
+    result->ret = event;
     switch(event) {
         case DIMM_REMOVE_SUCCESS:
             dimm_depopulate(s);
+            QTAILQ_INSERT_TAIL(&dimm_hp_result_queue, result, next);
             break;
         default:
+            g_free(result);
             break;
     }
 }
 
+MemHpInfoList *qmp_query_memhp(Error **errp)
+{
+    MemHpInfoList *head = NULL, *cur_item = NULL, *info;
+    struct dimm_hp_result *item, *nextitem;
+
+    QTAILQ_FOREACH_SAFE(item, &dimm_hp_result_queue, next, nextitem) {
+
+        info = g_malloc0(sizeof(*info));
+        info->value = g_malloc0(sizeof(*info->value));
+        info->value->Dimm = g_malloc0(sizeof(char) * 32);
+        info->value->request = g_malloc0(sizeof(char) * 16);
+        info->value->result = g_malloc0(sizeof(char) * 16);
+        switch (item->ret) {
+            case DIMM_REMOVE_SUCCESS:
+                strcpy(info->value->request, "hot-remove");
+                strcpy(info->value->result, "success");
+                break;
+            case DIMM_REMOVE_FAIL:
+                strcpy(info->value->request, "hot-remove");
+                strcpy(info->value->result, "failure");
+                break;
+            case DIMM_ADD_SUCCESS:
+                strcpy(info->value->request, "hot-add");
+                strcpy(info->value->result, "success");
+                break;
+            case DIMM_ADD_FAIL:
+                strcpy(info->value->request, "hot-add");
+                strcpy(info->value->result, "failure");
+                break;
+            default:
+                break;    
+        }
+        strcpy(info->value->Dimm, item->s->busdev.qdev.id);
+        /* XXX: waiting for the qapi to support GSList */
+        if (!cur_item) {
+            head = cur_item = info;
+        } else {
+            cur_item->next = info;
+            cur_item = info;
+        }
+
+        /* hotplug notification copied to qmp list, delete original item */
+        QTAILQ_REMOVE(&dimm_hp_result_queue, item, next);
+        g_free(item);
+    }
+
+    return head;
+}
 static int dimm_init(SysBusDevice *s)
 {
     DimmState *slot;
@@ -217,6 +271,7 @@  static void dimm_class_init(ObjectClass *klass, void *data)
     sc->init = dimm_init;
     dimm_hotplug = NULL;
     QTAILQ_INIT(&dimmlist);
+    QTAILQ_INIT(&dimm_hp_result_queue);
 }
 
 static TypeInfo dimm_info = {
diff --git a/hw/dimm.h b/hw/dimm.h
index 643f319..3e55ed3 100644
--- a/hw/dimm.h
+++ b/hw/dimm.h
@@ -37,6 +37,12 @@  typedef struct DimmState {
     QTAILQ_ENTRY (DimmState) nextdimm;
 } DimmState;
 
+struct dimm_hp_result {
+    DimmState *s;
+    dimm_hp_result_code ret;
+    QTAILQ_ENTRY (dimm_hp_result) next;
+};
+
 typedef int (*dimm_hotplug_fn)(DeviceState *qdev, SysBusDevice *dev, int add);
 typedef target_phys_addr_t (*dimm_calcoffset_fn)(uint64_t size);
 
diff --git a/monitor.c b/monitor.c
index d3d95a6..4a14e26 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2732,6 +2732,13 @@  static mon_cmd_t info_cmds[] = {
         .mhandler.info = do_trace_print_events,
     },
     {
+        .name       = "memhp",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show memory hotplug status",
+        .mhandler.info = hmp_info_memhp,
+    },
+    {
         .name       = NULL,
     },
 };
diff --git a/qapi-schema.json b/qapi-schema.json
index 3b6e346..049f6f9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1862,3 +1862,29 @@ 
 # Since: 0.14.0
 ##
 { 'command': 'netdev_del', 'data': {'id': 'str'} }
+
+##
+# @MemHpInfo:
+#
+# Information about status of a memory hotplug command
+#
+# @Dimm: the Dimm associated with the result
+#
+# @result: the result of the hotplug command
+#
+# Since: 1.1.3
+#
+##
+{ 'type': 'MemHpInfo',
+  'data': {'Dimm': 'str', 'request': 'str', 'result': 'str'} }
+
+##
+# @query-memhp:
+#
+# Returns a list of information about pending hotplug commands
+#
+# Returns: a list of @MemhpInfo
+#
+# Since: 1.1.3
+##
+{ 'command': 'query-memhp', 'returns': ['MemHpInfo'] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 7efd628..cd1d5f0 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2248,3 +2248,41 @@  Example:
 <- { "return": {} }
 
 EQMP
+
+    {
+        .name       = "query-memhp",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_memhp
+    },
+SQMP
+query-memhp
+----------
+
+Show memory hotplug command notifications.
+
+Return a json-array. Each DIMM that has a pending notification is represented
+by a json-object, which contains:
+
+- "Dimm": Dimm name (json-str)
+- "request": type of hot request: hot-add or hot-remove  (json-str)
+- "result": result of the hotplug request for this Dimm success or failure (json-str)
+
+Example:
+
+-> { "execute": "query-memhp" }
+<- {
+      "return":[
+         {
+            "result": "failure",
+            "request": "hot-remove",
+            "Dimm": "dimm10"
+         },
+         {
+            "result": "success",
+            "request": "hot-add",
+            "Dimm": "dimm3"
+         }
+      ]
+   }
+
+EQMP