* Re: [PATCH v2] qmp: Use unsigned integers for address parameters
2024-08-02 12:51 [PATCH v2] qmp: Use unsigned integers for address parameters Josh Junon
@ 2024-08-02 13:33 ` Markus Armbruster
0 siblings, 0 replies; 2+ messages in thread
From: Markus Armbruster @ 2024-08-02 13:33 UTC (permalink / raw)
To: Josh Junon
Cc: qemu-devel, Marc-André Lureau, Eric Blake, Eduardo Habkost,
Marcel Apfelbaum, Philippe Mathieu-Daudé, Yanan Wang,
Zhao Liu, Paolo Bonzini, Richard Henderson
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");
^ permalink raw reply [flat|nested] 2+ messages in thread