cpus.c | 81 ++++++++++++++++++++++++++++++++++++++++--------- hmp-commands.hx | 18 +++++++++-- hmp.c | 16 ++++++++-- hmp.h | 1 + qapi/misc.json | 20 ++++++++++++ 5 files changed, 118 insertions(+), 18 deletions(-)
Hello,
Revised patch series rebased on current master which should
incorporate all comments mentioned in the review. I've decided to
make size and offset optional for QMP, but keep it required for
HMP to make pmemload consistent with memsave/pmemsave.
Regards
Simon
Simon Ruderich (7):
cpus: correct coding style in qmp_memsave/qmp_pmemsave
cpus: convert qmp_memsave/qmp_pmemsave to use qemu_open
cpus: use size_t in qmp_memsave/qmp_pmemsave
hmp: use l for size argument in memsave/pmemsave
hmp: use F for filename arguments in memsave/pmemsave
qmp: add pmemload command
hmp: add pmemload command
cpus.c | 81 ++++++++++++++++++++++++++++++++++++++++---------
hmp-commands.hx | 18 +++++++++--
hmp.c | 16 ++++++++--
hmp.h | 1 +
qapi/misc.json | 20 ++++++++++++
5 files changed, 118 insertions(+), 18 deletions(-)
Interdiff:
diff --git a/cpus.c b/cpus.c
index 4141ef766c..d79bf8b485 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2369,8 +2369,10 @@ exit:
qemu_close(fd);
}
-void qmp_pmemload(int64_t addr, int64_t size, int64_t offset,
- const char *filename, Error **errp)
+void qmp_pmemload(int64_t addr, const char *filename,
+ bool has_size, int64_t size,
+ bool has_offset, int64_t offset,
+ Error **errp)
{
int fd;
size_t l;
@@ -2382,13 +2384,21 @@ void qmp_pmemload(int64_t addr, int64_t size, int64_t offset,
error_setg_file_open(errp, errno, filename);
return;
}
- if (offset > 0) {
+ if (has_offset && offset > 0) {
if (lseek(fd, offset, SEEK_SET) != offset) {
error_setg_errno(errp, errno,
"could not seek to offset %" PRIx64, offset);
goto exit;
}
}
+ if (!has_size) {
+ struct stat s;
+ if (fstat(fd, &s)) {
+ error_setg_errno(errp, errno, "could not fstat fd to get size");
+ goto exit;
+ }
+ size = s.st_size;
+ }
while (size != 0) {
l = sizeof(buf);
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 5a43dae133..c39d745a22 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -818,7 +818,7 @@ ETEXI
{
.name = "memsave",
- .args_type = "val:l,size:i,filename:s",
+ .args_type = "val:l,size:l,filename:F",
.params = "addr size file",
.help = "save to disk virtual memory dump starting at 'addr' of size 'size'",
.cmd = hmp_memsave,
@@ -832,7 +832,7 @@ ETEXI
{
.name = "pmemsave",
- .args_type = "val:l,size:i,filename:s",
+ .args_type = "val:l,size:l,filename:F",
.params = "addr size file",
.help = "save to disk physical memory dump starting at 'addr' of size 'size'",
.cmd = hmp_pmemsave,
@@ -846,7 +846,7 @@ ETEXI
{
.name = "pmemload",
- .args_type = "val:l,size:i,offset:i,filename:s",
+ .args_type = "val:l,size:l,offset:l,filename:F",
.params = "addr size offset file",
.help = "load from disk physical memory dump starting at 'addr' of size 'size' at file offset 'offset'",
.cmd = hmp_pmemload,
diff --git a/hmp.c b/hmp.c
index 73de92e629..293e067ed5 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1128,7 +1128,7 @@ void hmp_pmemload(Monitor *mon, const QDict *qdict)
uint64_t addr = qdict_get_int(qdict, "val");
Error *err = NULL;
- qmp_pmemload(addr, size, offset, filename, &err);
+ qmp_pmemload(addr, filename, true, size, true, offset, &err);
hmp_handle_error(mon, &err);
}
diff --git a/qapi/misc.json b/qapi/misc.json
index 6c34b2ff8b..06cf36f3d4 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -1188,18 +1188,18 @@
#
# @val: the physical address of the guest to start from
#
-# @size: the size of memory region to load
-#
-# @offset: the offset in the file to start from
-#
# @filename: the file to load the memory from as binary data
#
+# @size: the size of memory region to load (defaults to whole file)
+#
+# @offset: the offset in the file to start from (defaults to 0)
+#
# Returns: Nothing on success
#
-# Since: 2.13
+# Since: 3.1
##
{ 'command': 'pmemload',
- 'data': {'val': 'int', 'size': 'int', 'offset': 'int', 'filename': 'str'} }
+ 'data': {'val': 'int', 'filename': 'str', '*size': 'int', '*offset': 'int'} }
##
# @cont:
--
2.17.1
Signed-off-by: Simon Ruderich <simon@ruderich.org>
---
cpus.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/cpus.c b/cpus.c
index b5844b7103..125adc4da4 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2318,8 +2318,9 @@ void qmp_memsave(int64_t addr, int64_t size, const char *filename,
while (size != 0) {
l = sizeof(buf);
- if (l > size)
+ if (l > size) {
l = size;
+ }
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);
@@ -2352,8 +2353,9 @@ void qmp_pmemsave(int64_t addr, int64_t size, const char *filename,
while (size != 0) {
l = sizeof(buf);
- if (l > size)
+ if (l > size) {
l = size;
+ }
cpu_physical_memory_read(addr, buf, l);
if (fwrite(buf, 1, l, f) != l) {
error_setg(errp, QERR_IO_ERROR);
--
2.17.1
On 08/16/2018 04:01 AM, Simon Ruderich wrote: > Signed-off-by: Simon Ruderich <simon@ruderich.org> > --- > cpus.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
qemu_open() allow passing file descriptors to qemu which is used in
restricted environments like libvirt where open() is prohibited.
Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Simon Ruderich <simon@ruderich.org>
---
cpus.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/cpus.c b/cpus.c
index 125adc4da4..dd8ba36d2f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2293,7 +2293,7 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
void qmp_memsave(int64_t addr, int64_t size, const char *filename,
bool has_cpu, int64_t cpu_index, Error **errp)
{
- FILE *f;
+ int fd;
uint32_t l;
CPUState *cpu;
uint8_t buf[1024];
@@ -2310,8 +2310,8 @@ void qmp_memsave(int64_t addr, int64_t size, const char *filename,
return;
}
- f = fopen(filename, "wb");
- if (!f) {
+ fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0600);
+ if (fd < 0) {
error_setg_file_open(errp, errno, filename);
return;
}
@@ -2326,7 +2326,7 @@ void qmp_memsave(int64_t addr, int64_t size, const char *filename,
" specified", orig_addr, orig_size);
goto exit;
}
- if (fwrite(buf, 1, l, f) != l) {
+ if (qemu_write_full(fd, buf, l) != l) {
error_setg(errp, QERR_IO_ERROR);
goto exit;
}
@@ -2335,18 +2335,18 @@ void qmp_memsave(int64_t addr, int64_t size, const char *filename,
}
exit:
- fclose(f);
+ qemu_close(fd);
}
void qmp_pmemsave(int64_t addr, int64_t size, const char *filename,
Error **errp)
{
- FILE *f;
+ int fd;
uint32_t l;
uint8_t buf[1024];
- f = fopen(filename, "wb");
- if (!f) {
+ fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0600);
+ if (fd < 0) {
error_setg_file_open(errp, errno, filename);
return;
}
@@ -2357,7 +2357,7 @@ void qmp_pmemsave(int64_t addr, int64_t size, const char *filename,
l = size;
}
cpu_physical_memory_read(addr, buf, l);
- if (fwrite(buf, 1, l, f) != l) {
+ if (qemu_write_full(fd, buf, l) != l) {
error_setg(errp, QERR_IO_ERROR);
goto exit;
}
@@ -2366,7 +2366,7 @@ void qmp_pmemsave(int64_t addr, int64_t size, const char *filename,
}
exit:
- fclose(f);
+ qemu_close(fd);
}
void qmp_inject_nmi(Error **errp)
--
2.17.1
On 08/16/2018 04:01 AM, Simon Ruderich wrote: > qemu_open() allow passing file descriptors to qemu which is used in > restricted environments like libvirt where open() is prohibited. > > Suggested-by: Eric Blake <eblake@redhat.com> > Signed-off-by: Simon Ruderich <simon@ruderich.org> > --- > cpus.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
It's the natural type for object sizes and matches the return value of
sizeof(buf).
Signed-off-by: Simon Ruderich <simon@ruderich.org>
---
cpus.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cpus.c b/cpus.c
index dd8ba36d2f..243f2c0d2e 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2294,7 +2294,7 @@ void qmp_memsave(int64_t addr, int64_t size, const char *filename,
bool has_cpu, int64_t cpu_index, Error **errp)
{
int fd;
- uint32_t l;
+ size_t l;
CPUState *cpu;
uint8_t buf[1024];
int64_t orig_addr = addr, orig_size = size;
@@ -2342,7 +2342,7 @@ void qmp_pmemsave(int64_t addr, int64_t size, const char *filename,
Error **errp)
{
int fd;
- uint32_t l;
+ size_t l;
uint8_t buf[1024];
fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0600);
--
2.17.1
On 08/16/2018 04:01 AM, Simon Ruderich wrote: > It's the natural type for object sizes and matches the return value of > sizeof(buf). > > Signed-off-by: Simon Ruderich <simon@ruderich.org> > --- > cpus.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
i is only 32-bit. To prevent possible truncation when dumping large
memory regions use l which is target long.
Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Simon Ruderich <simon@ruderich.org>
---
hmp-commands.hx | 4 ++--
hmp.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 91dfe51c37..79c158a140 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -818,7 +818,7 @@ ETEXI
{
.name = "memsave",
- .args_type = "val:l,size:i,filename:s",
+ .args_type = "val:l,size:l,filename:s",
.params = "addr size file",
.help = "save to disk virtual memory dump starting at 'addr' of size 'size'",
.cmd = hmp_memsave,
@@ -832,7 +832,7 @@ ETEXI
{
.name = "pmemsave",
- .args_type = "val:l,size:i,filename:s",
+ .args_type = "val:l,size:l,filename:s",
.params = "addr size file",
.help = "save to disk physical memory dump starting at 'addr' of size 'size'",
.cmd = hmp_pmemsave,
diff --git a/hmp.c b/hmp.c
index 2aafb50e8e..1fb0724107 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1094,7 +1094,7 @@ void hmp_cpu(Monitor *mon, const QDict *qdict)
void hmp_memsave(Monitor *mon, const QDict *qdict)
{
- uint32_t size = qdict_get_int(qdict, "size");
+ uint64_t size = qdict_get_int(qdict, "size");
const char *filename = qdict_get_str(qdict, "filename");
uint64_t addr = qdict_get_int(qdict, "val");
Error *err = NULL;
@@ -1111,7 +1111,7 @@ void hmp_memsave(Monitor *mon, const QDict *qdict)
void hmp_pmemsave(Monitor *mon, const QDict *qdict)
{
- uint32_t size = qdict_get_int(qdict, "size");
+ uint64_t size = qdict_get_int(qdict, "size");
const char *filename = qdict_get_str(qdict, "filename");
uint64_t addr = qdict_get_int(qdict, "val");
Error *err = NULL;
--
2.17.1
* Simon Ruderich (simon@ruderich.org) wrote:
> i is only 32-bit. To prevent possible truncation when dumping large
> memory regions use l which is target long.
>
> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Simon Ruderich <simon@ruderich.org>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> hmp-commands.hx | 4 ++--
> hmp.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 91dfe51c37..79c158a140 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -818,7 +818,7 @@ ETEXI
>
> {
> .name = "memsave",
> - .args_type = "val:l,size:i,filename:s",
> + .args_type = "val:l,size:l,filename:s",
> .params = "addr size file",
> .help = "save to disk virtual memory dump starting at 'addr' of size 'size'",
> .cmd = hmp_memsave,
> @@ -832,7 +832,7 @@ ETEXI
>
> {
> .name = "pmemsave",
> - .args_type = "val:l,size:i,filename:s",
> + .args_type = "val:l,size:l,filename:s",
> .params = "addr size file",
> .help = "save to disk physical memory dump starting at 'addr' of size 'size'",
> .cmd = hmp_pmemsave,
> diff --git a/hmp.c b/hmp.c
> index 2aafb50e8e..1fb0724107 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1094,7 +1094,7 @@ void hmp_cpu(Monitor *mon, const QDict *qdict)
>
> void hmp_memsave(Monitor *mon, const QDict *qdict)
> {
> - uint32_t size = qdict_get_int(qdict, "size");
> + uint64_t size = qdict_get_int(qdict, "size");
> const char *filename = qdict_get_str(qdict, "filename");
> uint64_t addr = qdict_get_int(qdict, "val");
> Error *err = NULL;
> @@ -1111,7 +1111,7 @@ void hmp_memsave(Monitor *mon, const QDict *qdict)
>
> void hmp_pmemsave(Monitor *mon, const QDict *qdict)
> {
> - uint32_t size = qdict_get_int(qdict, "size");
> + uint64_t size = qdict_get_int(qdict, "size");
> const char *filename = qdict_get_str(qdict, "filename");
> uint64_t addr = qdict_get_int(qdict, "val");
> Error *err = NULL;
> --
> 2.17.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
This enables completion for the filename arguments.
Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Simon Ruderich <simon@ruderich.org>
---
hmp-commands.hx | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 79c158a140..5715bd7b51 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -818,7 +818,7 @@ ETEXI
{
.name = "memsave",
- .args_type = "val:l,size:l,filename:s",
+ .args_type = "val:l,size:l,filename:F",
.params = "addr size file",
.help = "save to disk virtual memory dump starting at 'addr' of size 'size'",
.cmd = hmp_memsave,
@@ -832,7 +832,7 @@ ETEXI
{
.name = "pmemsave",
- .args_type = "val:l,size:l,filename:s",
+ .args_type = "val:l,size:l,filename:F",
.params = "addr size file",
.help = "save to disk physical memory dump starting at 'addr' of size 'size'",
.cmd = hmp_pmemsave,
--
2.17.1
* Simon Ruderich (simon@ruderich.org) wrote:
> This enables completion for the filename arguments.
>
> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Simon Ruderich <simon@ruderich.org>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> hmp-commands.hx | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 79c158a140..5715bd7b51 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -818,7 +818,7 @@ ETEXI
>
> {
> .name = "memsave",
> - .args_type = "val:l,size:l,filename:s",
> + .args_type = "val:l,size:l,filename:F",
> .params = "addr size file",
> .help = "save to disk virtual memory dump starting at 'addr' of size 'size'",
> .cmd = hmp_memsave,
> @@ -832,7 +832,7 @@ ETEXI
>
> {
> .name = "pmemsave",
> - .args_type = "val:l,size:l,filename:s",
> + .args_type = "val:l,size:l,filename:F",
> .params = "addr size file",
> .help = "save to disk physical memory dump starting at 'addr' of size 'size'",
> .cmd = hmp_pmemsave,
> --
> 2.17.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Adapted patch from Baojun Wang [1] with the following commit message:
I found this could be useful to have qemu-softmmu as a cross
debugger (launch with -s -S command line option), then if we can
have a command to load guest physical memory, we can use cross gdb
to do some target debug which gdb cannot do directly.
This patch contains only the qmp changes of the original patch.
pmemload is necessary to directly write physical memory which is not
possible with gdb alone as it uses only logical addresses.
The QAPI for pmemload uses "val" as parameter name for the physical
address. This name is not very descriptive but is consistent with the
existing pmemsave. Changing the parameter name of pmemsave is not
possible without breaking the existing API.
[1]: https://lists.gnu.org/archive/html/qemu-trivial/2014-04/msg00074.html
Based-on-patch-by: Baojun Wang <wangbj@gmail.com>
Signed-off-by: Simon Ruderich <simon@ruderich.org>
---
cpus.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
qapi/misc.json | 20 ++++++++++++++++++++
2 files changed, 71 insertions(+)
diff --git a/cpus.c b/cpus.c
index 243f2c0d2e..d79bf8b485 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2369,6 +2369,57 @@ exit:
qemu_close(fd);
}
+void qmp_pmemload(int64_t addr, const char *filename,
+ bool has_size, int64_t size,
+ bool has_offset, int64_t offset,
+ Error **errp)
+{
+ int fd;
+ size_t l;
+ ssize_t r;
+ uint8_t buf[1024];
+
+ fd = qemu_open(filename, O_RDONLY | O_BINARY);
+ if (fd < 0) {
+ error_setg_file_open(errp, errno, filename);
+ return;
+ }
+ if (has_offset && offset > 0) {
+ if (lseek(fd, offset, SEEK_SET) != offset) {
+ error_setg_errno(errp, errno,
+ "could not seek to offset %" PRIx64, offset);
+ goto exit;
+ }
+ }
+ if (!has_size) {
+ struct stat s;
+ if (fstat(fd, &s)) {
+ error_setg_errno(errp, errno, "could not fstat fd to get size");
+ goto exit;
+ }
+ size = s.st_size;
+ }
+
+ while (size != 0) {
+ l = sizeof(buf);
+ if (l > size) {
+ l = size;
+ }
+ r = read(fd, buf, l);
+ if (r <= 0) {
+ error_setg(errp, QERR_IO_ERROR);
+ goto exit;
+ }
+ l = r; /* in case of short read */
+ cpu_physical_memory_write(addr, buf, l);
+ addr += l;
+ size -= l;
+ }
+
+exit:
+ qemu_close(fd);
+}
+
void qmp_inject_nmi(Error **errp)
{
nmi_monitor_handle(monitor_get_cpu_index(), errp);
diff --git a/qapi/misc.json b/qapi/misc.json
index d450cfef21..06cf36f3d4 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -1181,6 +1181,26 @@
{ 'command': 'pmemsave',
'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
+##
+# @pmemload:
+#
+# Load a portion of guest physical memory from a file.
+#
+# @val: the physical address of the guest to start from
+#
+# @filename: the file to load the memory from as binary data
+#
+# @size: the size of memory region to load (defaults to whole file)
+#
+# @offset: the offset in the file to start from (defaults to 0)
+#
+# Returns: Nothing on success
+#
+# Since: 3.1
+##
+{ 'command': 'pmemload',
+ 'data': {'val': 'int', 'filename': 'str', '*size': 'int', '*offset': 'int'} }
+
##
# @cont:
#
--
2.17.1
On 08/16/2018 04:01 AM, Simon Ruderich wrote:
> Adapted patch from Baojun Wang [1] with the following commit message:
>
> I found this could be useful to have qemu-softmmu as a cross
> debugger (launch with -s -S command line option), then if we can
> have a command to load guest physical memory, we can use cross gdb
> to do some target debug which gdb cannot do directly.
>
> This patch contains only the qmp changes of the original patch.
>
> pmemload is necessary to directly write physical memory which is not
> possible with gdb alone as it uses only logical addresses.
>
> The QAPI for pmemload uses "val" as parameter name for the physical
> address. This name is not very descriptive but is consistent with the
> existing pmemsave. Changing the parameter name of pmemsave is not
> possible without breaking the existing API.
>
> [1]: https://lists.gnu.org/archive/html/qemu-trivial/2014-04/msg00074.html
>
> Based-on-patch-by: Baojun Wang <wangbj@gmail.com>
> Signed-off-by: Simon Ruderich <simon@ruderich.org>
> ---
> + }
> + if (!has_size) {
> + struct stat s;
> + if (fstat(fd, &s)) {
> + error_setg_errno(errp, errno, "could not fstat fd to get size");
> + goto exit;
> + }
> + size = s.st_size;
> + }
This works for regular files, but not for block devices.
Do we have a helper function for easily determining the size of an
arbitrary fd (whether file or block device)? If not, should we? As
there are multiple spots in the code that grab this sort of information.
Otherwise looks okay to me.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
On Thu, Aug 16, 2018 at 03:01:31PM -0500, Eric Blake wrote:
>> + }
>> + if (!has_size) {
>> + struct stat s;
>> + if (fstat(fd, &s)) {
>> + error_setg_errno(errp, errno, "could not fstat fd to get size");
>> + goto exit;
>> + }
>> + size = s.st_size;
>> + }
>
> This works for regular files, but not for block devices.
>
> Do we have a helper function for easily determining the size of an arbitrary
> fd (whether file or block device)? If not, should we? As there are
> multiple spots in the code that grab this sort of information.
I found raw_getlength() in block/file-posix.c, but its static and
it takes a BlockDriverState* as argument. For most systems it
could be extracted without too much trouble, but some (e.g
__sun__ and BSD) are quite entangled with BlockDriverState code.
> Otherwise looks okay to me.
Should I check for S_ISCHR() and S_ISBLK() and abort with an
error? I don't think it's a common use case for pmemload anyway.
Regards
Simon
--
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9
Adapted patch from Baojun Wang [1] with the following commit message:
I found this could be useful to have qemu-softmmu as a cross
debugger (launch with -s -S command line option), then if we can
have a command to load guest physical memory, we can use cross gdb
to do some target debug which gdb cannot do directly.
This patch contains only the qmp changes of the original patch.
pmemload is necessary to directly write physical memory which is not
possible with gdb alone as it uses only logical addresses.
The QAPI for pmemload uses "val" as parameter name for the physical
address. This name is not very descriptive but is consistent with the
existing pmemsave. Changing the parameter name of pmemsave is not
possible without breaking the existing API.
[1]: https://lists.gnu.org/archive/html/qemu-trivial/2014-04/msg00074.html
Based-on-patch-by: Baojun Wang <wangbj@gmail.com>
Signed-off-by: Simon Ruderich <simon@ruderich.org>
---
Hello,
I've adapted the patch to error out if a char/block device is
used. I think that's the simplest fix for the issue mentioned by
Eric Blake.
Are the any other issues remaining? All other patches are
unchanged, should I resend the whole series?
Regards
Simon
Diff of this patch since v4:
diff --git a/cpus.c b/cpus.c
index d79bf8b485..1622f00846 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2397,6 +2397,10 @@ void qmp_pmemload(int64_t addr, const char *filename,
error_setg_errno(errp, errno, "could not fstat fd to get size");
goto exit;
}
+ if (S_ISCHR(s.st_mode) || S_ISBLK(s.st_mode)) {
+ error_setg(errp, "pmemload doesn't support char/block devices");
+ goto exit;
+ }
size = s.st_size;
}
cpus.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++
qapi/misc.json | 20 ++++++++++++++++++
2 files changed, 75 insertions(+)
diff --git a/cpus.c b/cpus.c
index 243f2c0d2e..1622f00846 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2369,6 +2369,61 @@ exit:
qemu_close(fd);
}
+void qmp_pmemload(int64_t addr, const char *filename,
+ bool has_size, int64_t size,
+ bool has_offset, int64_t offset,
+ Error **errp)
+{
+ int fd;
+ size_t l;
+ ssize_t r;
+ uint8_t buf[1024];
+
+ fd = qemu_open(filename, O_RDONLY | O_BINARY);
+ if (fd < 0) {
+ error_setg_file_open(errp, errno, filename);
+ return;
+ }
+ if (has_offset && offset > 0) {
+ if (lseek(fd, offset, SEEK_SET) != offset) {
+ error_setg_errno(errp, errno,
+ "could not seek to offset %" PRIx64, offset);
+ goto exit;
+ }
+ }
+ if (!has_size) {
+ struct stat s;
+ if (fstat(fd, &s)) {
+ error_setg_errno(errp, errno, "could not fstat fd to get size");
+ goto exit;
+ }
+ if (S_ISCHR(s.st_mode) || S_ISBLK(s.st_mode)) {
+ error_setg(errp, "pmemload doesn't support char/block devices");
+ goto exit;
+ }
+ size = s.st_size;
+ }
+
+ while (size != 0) {
+ l = sizeof(buf);
+ if (l > size) {
+ l = size;
+ }
+ r = read(fd, buf, l);
+ if (r <= 0) {
+ error_setg(errp, QERR_IO_ERROR);
+ goto exit;
+ }
+ l = r; /* in case of short read */
+ cpu_physical_memory_write(addr, buf, l);
+ addr += l;
+ size -= l;
+ }
+
+exit:
+ qemu_close(fd);
+}
+
void qmp_inject_nmi(Error **errp)
{
nmi_monitor_handle(monitor_get_cpu_index(), errp);
diff --git a/qapi/misc.json b/qapi/misc.json
index d450cfef21..06cf36f3d4 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -1181,6 +1181,26 @@
{ 'command': 'pmemsave',
'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
+##
+# @pmemload:
+#
+# Load a portion of guest physical memory from a file.
+#
+# @val: the physical address of the guest to start from
+#
+# @filename: the file to load the memory from as binary data
+#
+# @size: the size of memory region to load (defaults to whole file)
+#
+# @offset: the offset in the file to start from (defaults to 0)
+#
+# Returns: Nothing on success
+#
+# Since: 3.1
+##
+{ 'command': 'pmemload',
+ 'data': {'val': 'int', 'filename': 'str', '*size': 'int', '*offset': 'int'} }
+
##
# @cont:
#
--
2.17.1
--
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9
On Tue, Aug 21, 2018 at 02:38:02PM +0200, Simon Ruderich wrote: > Hello, > > I've adapted the patch to error out if a char/block device is > used. I think that's the simplest fix for the issue mentioned by > Eric Blake. > > Are the any other issues remaining? All other patches are > unchanged, should I resend the whole series? Hello, Any news? Anything I can do to get the patches merged? Regards Simon -- + privacy is necessary + using gnupg http://gnupg.org + public key id: 0x92FEFDB7E44C32F9
On Fri, Sep 21, 2018 at 01:02:05PM +0200, Simon Ruderich wrote: > On Tue, Aug 21, 2018 at 02:38:02PM +0200, Simon Ruderich wrote: >> Hello, >> >> I've adapted the patch to error out if a char/block device is >> used. I think that's the simplest fix for the issue mentioned by >> Eric Blake. >> >> Are the any other issues remaining? All other patches are >> unchanged, should I resend the whole series? > > Hello, > > Any news? Anything I can do to get the patches merged? Hello again, Any news? I'd really like to get this patchset merged. Regards Simon -- + privacy is necessary + using gnupg http://gnupg.org + public key id: 0x92FEFDB7E44C32F9
Adapted patch from Baojun Wang [1] with the following commit message:
I found this could be useful to have qemu-softmmu as a cross
debugger (launch with -s -S command line option), then if we can
have a command to load guest physical memory, we can use cross gdb
to do some target debug which gdb cannot do directly.
This patch contains only the hmp changes of the original patch.
[1]: https://lists.gnu.org/archive/html/qemu-trivial/2014-04/msg00074.html
Based-on-patch-by: Baojun Wang <wangbj@gmail.com>
Signed-off-by: Simon Ruderich <simon@ruderich.org>
---
hmp-commands.hx | 14 ++++++++++++++
hmp.c | 12 ++++++++++++
hmp.h | 1 +
3 files changed, 27 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 5715bd7b51..c39d745a22 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -842,6 +842,20 @@ STEXI
@item pmemsave @var{addr} @var{size} @var{file}
@findex pmemsave
save to disk physical memory dump starting at @var{addr} of size @var{size}.
+ETEXI
+
+ {
+ .name = "pmemload",
+ .args_type = "val:l,size:l,offset:l,filename:F",
+ .params = "addr size offset file",
+ .help = "load from disk physical memory dump starting at 'addr' of size 'size' at file offset 'offset'",
+ .cmd = hmp_pmemload,
+ },
+
+STEXI
+@item pmemload @var{addr} @var{size} @var{offset} @var{file}
+@findex pmemload
+load from disk physical memory dump starting at @var{addr} of size @var{size} at file offset @var{offset}.
ETEXI
{
diff --git a/hmp.c b/hmp.c
index 1fb0724107..293e067ed5 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1120,6 +1120,18 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &err);
}
+void hmp_pmemload(Monitor *mon, const QDict *qdict)
+{
+ uint64_t size = qdict_get_int(qdict, "size");
+ uint64_t offset = qdict_get_int(qdict, "offset");
+ const char *filename = qdict_get_str(qdict, "filename");
+ uint64_t addr = qdict_get_int(qdict, "val");
+ Error *err = NULL;
+
+ qmp_pmemload(addr, filename, true, size, true, offset, &err);
+ hmp_handle_error(mon, &err);
+}
+
void hmp_ringbuf_write(Monitor *mon, const QDict *qdict)
{
const char *chardev = qdict_get_str(qdict, "device");
diff --git a/hmp.h b/hmp.h
index 33354f1bdd..f2b2a72a29 100644
--- a/hmp.h
+++ b/hmp.h
@@ -48,6 +48,7 @@ void hmp_exit_preconfig(Monitor *mon, const QDict *qdict);
void hmp_cpu(Monitor *mon, const QDict *qdict);
void hmp_memsave(Monitor *mon, const QDict *qdict);
void hmp_pmemsave(Monitor *mon, const QDict *qdict);
+void hmp_pmemload(Monitor *mon, const QDict *qdict);
void hmp_ringbuf_write(Monitor *mon, const QDict *qdict);
void hmp_ringbuf_read(Monitor *mon, const QDict *qdict);
void hmp_cont(Monitor *mon, const QDict *qdict);
--
2.17.1
* Simon Ruderich (simon@ruderich.org) wrote:
> Adapted patch from Baojun Wang [1] with the following commit message:
>
> I found this could be useful to have qemu-softmmu as a cross
> debugger (launch with -s -S command line option), then if we can
> have a command to load guest physical memory, we can use cross gdb
> to do some target debug which gdb cannot do directly.
>
> This patch contains only the hmp changes of the original patch.
>
> [1]: https://lists.gnu.org/archive/html/qemu-trivial/2014-04/msg00074.html
>
> Based-on-patch-by: Baojun Wang <wangbj@gmail.com>
> Signed-off-by: Simon Ruderich <simon@ruderich.org>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> hmp-commands.hx | 14 ++++++++++++++
> hmp.c | 12 ++++++++++++
> hmp.h | 1 +
> 3 files changed, 27 insertions(+)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 5715bd7b51..c39d745a22 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -842,6 +842,20 @@ STEXI
> @item pmemsave @var{addr} @var{size} @var{file}
> @findex pmemsave
> save to disk physical memory dump starting at @var{addr} of size @var{size}.
> +ETEXI
> +
> + {
> + .name = "pmemload",
> + .args_type = "val:l,size:l,offset:l,filename:F",
> + .params = "addr size offset file",
> + .help = "load from disk physical memory dump starting at 'addr' of size 'size' at file offset 'offset'",
> + .cmd = hmp_pmemload,
> + },
> +
> +STEXI
> +@item pmemload @var{addr} @var{size} @var{offset} @var{file}
> +@findex pmemload
> +load from disk physical memory dump starting at @var{addr} of size @var{size} at file offset @var{offset}.
> ETEXI
>
> {
> diff --git a/hmp.c b/hmp.c
> index 1fb0724107..293e067ed5 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1120,6 +1120,18 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
> hmp_handle_error(mon, &err);
> }
>
> +void hmp_pmemload(Monitor *mon, const QDict *qdict)
> +{
> + uint64_t size = qdict_get_int(qdict, "size");
> + uint64_t offset = qdict_get_int(qdict, "offset");
> + const char *filename = qdict_get_str(qdict, "filename");
> + uint64_t addr = qdict_get_int(qdict, "val");
> + Error *err = NULL;
> +
> + qmp_pmemload(addr, filename, true, size, true, offset, &err);
> + hmp_handle_error(mon, &err);
> +}
> +
> void hmp_ringbuf_write(Monitor *mon, const QDict *qdict)
> {
> const char *chardev = qdict_get_str(qdict, "device");
> diff --git a/hmp.h b/hmp.h
> index 33354f1bdd..f2b2a72a29 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -48,6 +48,7 @@ void hmp_exit_preconfig(Monitor *mon, const QDict *qdict);
> void hmp_cpu(Monitor *mon, const QDict *qdict);
> void hmp_memsave(Monitor *mon, const QDict *qdict);
> void hmp_pmemsave(Monitor *mon, const QDict *qdict);
> +void hmp_pmemload(Monitor *mon, const QDict *qdict);
> void hmp_ringbuf_write(Monitor *mon, const QDict *qdict);
> void hmp_ringbuf_read(Monitor *mon, const QDict *qdict);
> void hmp_cont(Monitor *mon, const QDict *qdict);
> --
> 2.17.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
© 2016 - 2025 Red Hat, Inc.