[PATCH v2] qmp: Use unsigned integers for address parameters

[PATCH v2] qmp: Use unsigned integers for address parameters

[Josh Junon]

Fixes higher-half address parsing for QMP commands
`[p]memsave` and `dump-guest-memory`.

Signed-off-by: Josh Junon <junon@oro.sh>
---
 dump/dump.c       |  4 ++--
 qapi/dump.json    |  2 +-
 qapi/machine.json | 11 +++++++++--
 system/cpus.c     |  8 ++++----
 4 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/dump/dump.c b/dump/dump.c
index 45e84428ae..00a1323735 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -2063,8 +2063,8 @@ DumpQueryResult *qmp_query_dump(Error **errp)
 
 void qmp_dump_guest_memory(bool paging, const char *protocol,
                            bool has_detach, bool detach,
-                           bool has_begin, int64_t begin,
-                           bool has_length, int64_t length,
+                           bool has_begin, uint64_t begin,
+                           bool has_length, uint64_t length,
                            bool has_format, DumpGuestMemoryFormat format,
                            Error **errp)
 {
diff --git a/qapi/dump.json b/qapi/dump.json
index d8145dad97..3b751c0356 100644
--- a/qapi/dump.json
+++ b/qapi/dump.json
@@ -102,7 +102,7 @@
 ##
 { 'command': 'dump-guest-memory',
   'data': { 'paging': 'bool', 'protocol': 'str', '*detach': 'bool',
-            '*begin': 'int', '*length': 'int',
+            '*begin': 'uint64', '*length': 'size',
             '*format': 'DumpGuestMemoryFormat'} }
 
 ##
diff --git a/qapi/machine.json b/qapi/machine.json
index fcfd249e2d..fb618dc99f 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -852,7 +852,11 @@
 #     <- { "return": {} }
 ##
 { 'command': 'memsave',
-  'data': {'val': 'int', 'size': 'int', 'filename': 'str', '*cpu-index': 'int'} }
+  'data': {
+     'val': 'uint64',
+     'size': 'size',
+     'filename': 'str',
+     '*cpu-index': 'int' } }
 
 ##
 # @pmemsave:
@@ -878,7 +882,10 @@
 #     <- { "return": {} }
 ##
 { 'command': 'pmemsave',
-  'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
+  'data': {
+    'val': 'uint64',
+    'size': 'size',
+    'filename': 'str' } }
 
 ##
 # @Memdev:
diff --git a/system/cpus.c b/system/cpus.c
index 5e3a988a0a..128face42b 100644
--- a/system/cpus.c
+++ b/system/cpus.c
@@ -792,14 +792,14 @@ int vm_stop_force_state(RunState state)
     }
 }
 
-void qmp_memsave(int64_t addr, int64_t size, const char *filename,
+void qmp_memsave(uint64_t addr, uint64_t size, const char *filename,
                  bool has_cpu, int64_t cpu_index, Error **errp)
 {
     FILE *f;
     uint32_t l;
     CPUState *cpu;
     uint8_t buf[1024];
-    int64_t orig_addr = addr, orig_size = size;
+    uint64_t orig_addr = addr, orig_size = size;
 
     if (!has_cpu) {
         cpu_index = 0;
@@ -840,11 +840,11 @@ exit:
     fclose(f);
 }
 
-void qmp_pmemsave(int64_t addr, int64_t size, const char *filename,
+void qmp_pmemsave(uint64_t addr, uint64_t size, const char *filename,
                   Error **errp)
 {
     FILE *f;
-    uint32_t l;
+    uint64_t l;
     uint8_t buf[1024];
 
     f = fopen(filename, "wb");
-- 
2.34.1

Re: [PATCH v2] qmp: Use unsigned integers for address parameters

[Markus Armbruster]

Josh Junon <junon@oro.sh> writes:

> Fixes higher-half address parsing for QMP commands
> `[p]memsave` and `dump-guest-memory`.
>
> Signed-off-by: Josh Junon <junon@oro.sh>
> ---
>  dump/dump.c       |  4 ++--
>  qapi/dump.json    |  2 +-
>  qapi/machine.json | 11 +++++++++--
>  system/cpus.c     |  8 ++++----
>  4 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/dump/dump.c b/dump/dump.c
> index 45e84428ae..00a1323735 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -2063,8 +2063,8 @@ DumpQueryResult *qmp_query_dump(Error **errp)
>  
>  void qmp_dump_guest_memory(bool paging, const char *protocol,
>                             bool has_detach, bool detach,
> -                           bool has_begin, int64_t begin,
> -                           bool has_length, int64_t length,
> +                           bool has_begin, uint64_t begin,
> +                           bool has_length, uint64_t length,
>                             bool has_format, DumpGuestMemoryFormat format,
>                             Error **errp)
>  {
[...]
       dump_init(s, fd, has_format, format, paging, has_begin,
-->              begin, length, kdump_raw, errp);
       if (*errp) {
           qatomic_set(&s->status, DUMP_STATUS_FAILED);
           return;
       }

Passing @begin and @length to dump_init() converts them to int64_t.
Whether values exceeding 2^63 work is anybody's guess.

A complete fix for dump-guest-memory will be a bit of a yak shave, I'm
afraid.  Feel free fix just memsave and pmemsave.

> diff --git a/qapi/dump.json b/qapi/dump.json
> index d8145dad97..3b751c0356 100644
> --- a/qapi/dump.json
> +++ b/qapi/dump.json
> @@ -102,7 +102,7 @@
>  ##
>  { 'command': 'dump-guest-memory',
>    'data': { 'paging': 'bool', 'protocol': 'str', '*detach': 'bool',
> -            '*begin': 'int', '*length': 'int',
> +            '*begin': 'uint64', '*length': 'size',
>              '*format': 'DumpGuestMemoryFormat'} }
>  
>  ##
> diff --git a/qapi/machine.json b/qapi/machine.json
> index fcfd249e2d..fb618dc99f 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -852,7 +852,11 @@
>  #     <- { "return": {} }
>  ##
>  { 'command': 'memsave',
> -  'data': {'val': 'int', 'size': 'int', 'filename': 'str', '*cpu-index': 'int'} }
> +  'data': {
> +     'val': 'uint64',
> +     'size': 'size',
> +     'filename': 'str',
> +     '*cpu-index': 'int' } }
>  
>  ##
>  # @pmemsave:
> @@ -878,7 +882,10 @@
>  #     <- { "return": {} }
>  ##
>  { 'command': 'pmemsave',
> -  'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
> +  'data': {
> +    'val': 'uint64',
> +    'size': 'size',
> +    'filename': 'str' } }
>  
>  ##
>  # @Memdev:
> diff --git a/system/cpus.c b/system/cpus.c
> index 5e3a988a0a..128face42b 100644
> --- a/system/cpus.c
> +++ b/system/cpus.c
> @@ -792,14 +792,14 @@ int vm_stop_force_state(RunState state)
>      }
>  }
>  
> -void qmp_memsave(int64_t addr, int64_t size, const char *filename,
> +void qmp_memsave(uint64_t addr, uint64_t size, const char *filename,
>                   bool has_cpu, int64_t cpu_index, Error **errp)
>  {
>      FILE *f;
>      uint32_t l;
>      CPUState *cpu;
>      uint8_t buf[1024];
> -    int64_t orig_addr = addr, orig_size = size;
> +    uint64_t orig_addr = addr, orig_size = size;
>  
>      if (!has_cpu) {
>          cpu_index = 0;
       }
       [...]
           if (cpu_memory_rw_debug(cpu, addr, buf, l, 0) != 0) {
               error_setg(errp, "Invalid addr 0x%016" PRIx64 "/size %" PRId64

PRIu64

                                " specified", orig_addr, orig_size);
               goto exit;
           }
> @@ -840,11 +840,11 @@ exit:
>      fclose(f);
>  }
>  
> -void qmp_pmemsave(int64_t addr, int64_t size, const char *filename,
> +void qmp_pmemsave(uint64_t addr, uint64_t size, const char *filename,
>                    Error **errp)
>  {
>      FILE *f;
> -    uint32_t l;
> +    uint64_t l;

Either leave it alone here, or change it in qmp_memsave(), too.

>      uint8_t buf[1024];
>  
>      f = fopen(filename, "wb");