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

[Markus Armbruster]

Let's go back to v1's title

    qmp: Fix higher half vaddrs for [p]memsave

Josh Junon <junon@oro.sh> writes:

> Fixes higher-half address parsing for QMP commands
> `[p]memsave`.
>
> Signed-off-by: Josh Junon <junon@oro.sh>
> ---
>  qapi/machine.json | 11 +++++++++--
>  system/cpus.c     | 10 +++++-----
>  2 files changed, 14 insertions(+), 7 deletions(-)
>
> 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..f33e4fbff0 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;
> +    uint64_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
                                " specified", orig_addr, orig_size);

@size is now unsigned and should be formatted with PRIu64.

               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;
>      uint8_t buf[1024];
>  
>      f = fopen(filename, "wb");

I can address my nitpicks in my tree, no respin necessary.

Reviewed-by: Markus Armbruster <armbru@redhat.com>