Patchwork [v6,11/11] Add 'query-dump-guest-memory-capability' command

login
register
mail settings
Submitter Qiao Nuohan
Date Jan. 5, 2014, 7:27 a.m.
Message ID <1388906864-1083-12-git-send-email-qiaonuohan@cn.fujitsu.com>
Download mbox | patch
Permalink /patch/306924/
State New
Headers show

Comments

Qiao Nuohan - Jan. 5, 2014, 7:27 a.m.
'query-dump-guest-memory-capability' is used to query whether option 'format'
is available for 'dump-guest-memory' and the available format. The output
of the command will be like:

-> { "execute": "query-dump-guest-memory-capability" }
<- { "return": {
        "format-option": "optional",
        "capabilities": [
            {"available": true, "format": "elf"},
            {"available": true, "format": "kdump-zlib"},
            {"available": true, "format": "kdump-lzo"},
            {"available": true, "format": "kdump-snappy"}
        ]
    }

Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
---
 dump.c           |   38 ++++++++++++++++++++++++++++++++++++++
 qapi-schema.json |   13 +++++++++++++
 qmp-commands.hx  |    7 +++++++
 3 files changed, 58 insertions(+), 0 deletions(-)
Laszlo Ersek - Jan. 9, 2014, 4:34 p.m.
comments below

On 01/05/14 08:27, Qiao Nuohan wrote:
> 'query-dump-guest-memory-capability' is used to query whether option 'format'
> is available for 'dump-guest-memory' and the available format. The output
> of the command will be like:
> 
> -> { "execute": "query-dump-guest-memory-capability" }
> <- { "return": {
>         "format-option": "optional",
>         "capabilities": [
>             {"available": true, "format": "elf"},
>             {"available": true, "format": "kdump-zlib"},
>             {"available": true, "format": "kdump-lzo"},
>             {"available": true, "format": "kdump-snappy"}
>         ]
>     }

The order of "available" and "format" in the commit message is reverse
in relation to the json. Not too important.

> 
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> ---
>  dump.c           |   38 ++++++++++++++++++++++++++++++++++++++
>  qapi-schema.json |   13 +++++++++++++
>  qmp-commands.hx  |    7 +++++++
>  3 files changed, 58 insertions(+), 0 deletions(-)
> 
> diff --git a/dump.c b/dump.c
> index b4e79ff..51fe23a 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -1757,3 +1757,41 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
>  
>      g_free(s);
>  }
> +
> +DumpGuestMemoryCapability *qmp_query_dump_guest_memory_capability(Error **errp)
> +{
> +    int i;
> +    DumpGuestMemoryCapabilityStatusList *item;
> +    DumpGuestMemoryCapability *cap =
> +                                  g_malloc0(sizeof(DumpGuestMemoryCapability));
> +
> +    cap->format_option = g_strdup_printf("optional");
> +
> +    for (i = 0; i < DUMP_GUEST_MEMORY_FORMAT_MAX; i++) {
> +        if (cap->capabilities == NULL) {
> +            item = g_malloc0(sizeof(DumpGuestMemoryCapabilityStatusList));
> +            cap->capabilities = item;
> +        } else {
> +            item->next = g_malloc0(sizeof(DumpGuestMemoryCapabilityStatusList));
> +            item = item->next;
> +        }
> +
> +        item->value = g_malloc0(sizeof(struct DumpGuestMemoryCapabilityStatus));
> +        item->value->format = i;
> +        item->value->available = true;
> +
> +        if (i == DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO) {
> +#ifndef CONFIG_LZO
> +            item->value->available = false;
> +#endif
> +        }
> +
> +        if (i == DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY) {
> +#ifndef CONFIG_SNAPPY
> +            item->value->available = false;
> +#endif
> +        }
> +    }

You could move the preprocessor directives outside.

Also, since the "available" field is false by nature of g_malloc0(),
maybe the code could be reorganized
- from "false to true to maybe false"
- to   "false to maybe true".

Not overly important.

> +
> +    return cap;
> +}
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 19b2b23..e545e0f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2740,6 +2740,19 @@
>              '*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
>  
>  ##
> +# Since: 1.8

2.0

> +##
> +{ 'type':  'DumpGuestMemoryCapabilityStatus',
> +  'data': { 'format': 'DumpGuestMemoryFormat', 'available': 'bool' } }
> +
> +{ 'type': 'DumpGuestMemoryCapability',
> +  'data': {
> +      'format-option': 'str',
> +      'capabilities': ['DumpGuestMemoryCapabilityStatus'] } }
> +
> +{ 'command': 'query-dump-guest-memory-capability', 'returns': 'DumpGuestMemoryCapability' }
> +
> +##
>  # @netdev_add:
>  #
>  # Add a network backend.
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 3de9e7d..3379c75 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -828,6 +828,13 @@ Notes:
>  EQMP
>  
>      {
> +        .name       = "query-dump-guest-memory-capability",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_query_dump_guest_memory_capability,
> +    },
> +
> +
> +    {
>          .name       = "netdev_add",
>          .args_type  = "netdev:O",
>          .mhandler.cmd_new = qmp_netdev_add,
> 

I think some more documentation is usual in the qmp-commands.hx file for
new commands; an SQMP/EQMP section should be likely added. However I'm
not giving my R-b only because of the 1.8 vs. 2.0 comment.

I'll let Eric decide if this query command serves libvirt sufficiently.

Thanks!
Laszlo

Patch

diff --git a/dump.c b/dump.c
index b4e79ff..51fe23a 100644
--- a/dump.c
+++ b/dump.c
@@ -1757,3 +1757,41 @@  void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
 
     g_free(s);
 }
+
+DumpGuestMemoryCapability *qmp_query_dump_guest_memory_capability(Error **errp)
+{
+    int i;
+    DumpGuestMemoryCapabilityStatusList *item;
+    DumpGuestMemoryCapability *cap =
+                                  g_malloc0(sizeof(DumpGuestMemoryCapability));
+
+    cap->format_option = g_strdup_printf("optional");
+
+    for (i = 0; i < DUMP_GUEST_MEMORY_FORMAT_MAX; i++) {
+        if (cap->capabilities == NULL) {
+            item = g_malloc0(sizeof(DumpGuestMemoryCapabilityStatusList));
+            cap->capabilities = item;
+        } else {
+            item->next = g_malloc0(sizeof(DumpGuestMemoryCapabilityStatusList));
+            item = item->next;
+        }
+
+        item->value = g_malloc0(sizeof(struct DumpGuestMemoryCapabilityStatus));
+        item->value->format = i;
+        item->value->available = true;
+
+        if (i == DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO) {
+#ifndef CONFIG_LZO
+            item->value->available = false;
+#endif
+        }
+
+        if (i == DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY) {
+#ifndef CONFIG_SNAPPY
+            item->value->available = false;
+#endif
+        }
+    }
+
+    return cap;
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index 19b2b23..e545e0f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2740,6 +2740,19 @@ 
             '*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
 
 ##
+# Since: 1.8
+##
+{ 'type':  'DumpGuestMemoryCapabilityStatus',
+  'data': { 'format': 'DumpGuestMemoryFormat', 'available': 'bool' } }
+
+{ 'type': 'DumpGuestMemoryCapability',
+  'data': {
+      'format-option': 'str',
+      'capabilities': ['DumpGuestMemoryCapabilityStatus'] } }
+
+{ 'command': 'query-dump-guest-memory-capability', 'returns': 'DumpGuestMemoryCapability' }
+
+##
 # @netdev_add:
 #
 # Add a network backend.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 3de9e7d..3379c75 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -828,6 +828,13 @@  Notes:
 EQMP
 
     {
+        .name       = "query-dump-guest-memory-capability",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_dump_guest_memory_capability,
+    },
+
+
+    {
         .name       = "netdev_add",
         .args_type  = "netdev:O",
         .mhandler.cmd_new = qmp_netdev_add,