[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [PATCH V4 1/1] qmp: add pmemload command
From: |
Eric Blake |
Subject: |
Re: [Qemu-trivial] [PATCH V4 1/1] qmp: add pmemload command |
Date: |
Wed, 09 Apr 2014 11:02:18 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 |
On 04/09/2014 10:54 AM, Baojun Wang wrote:
> Signed-off-by: Baojun Wang <address@hidden>
You lost this part of your commit message, which gives more details
about the 'why' (the subject line covers the 'what', but it is often the
'why' that is most needed when reviewing a commit later). Your earlier
mail had:
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.
>
> +void qmp_pmemload(int64_t addr, int64_t size, const char *filename,
> + Error **errp)
> +{
> + FILE *f;
> + uint32_t l;
> + uint8_t buf[1024];
> +
> + f = fopen(filename, "rb");
I still think that fopen()/fread() is wrong, and that you should be
using qemu_open()/read() - that way, libvirt could pass in a file
descriptor and use qemu's already-existing /dev/fdset/nnn support rather
than forcing qemu to open something from the filesystem.
> +++ b/hmp-commands.hx
> @@ -809,6 +809,19 @@ save to disk physical memory dump starting at @var{addr}
> of size @var{size}.
> ETEXI
>
> {
> + .name = "pmemload",
> + .args_type = "val:l,size:i,filename:s",
> + .params = "addr size file",
> + .help = "load from disk physical memory dump starting at
> 'addr' of size 'size'",
> + .mhandler.cmd = hmp_pmemload,
And I still think that you should list filename first, with val and size
optional at the end.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-trivial] [PATCH V3 1/1] Add pmemsave command for both monitor and qmp, which could be useful to have qemu-softmmu as a cross debugger to load guest physical memory., Baojun Wang, 2014/04/08
- Re: [Qemu-trivial] [PATCH V3 1/1] Add pmemsave command for both monitor and qmp, which could be useful to have qemu-softmmu as a cross debugger to load guest physical memory., Eric Blake, 2014/04/08
- Re: [Qemu-trivial] [PATCH V3 1/1] Add pmemsave command for both monitor and qmp, which could be useful to have qemu-softmmu as a cross debugger to load guest physical memory., Baojun Wang, 2014/04/08
- [Qemu-trivial] [PATCH V4 1/1] qmp: add pmemload command, Baojun Wang, 2014/04/09
- Re: [Qemu-trivial] [PATCH V4 1/1] qmp: add pmemload command,
Eric Blake <=
- [Qemu-trivial] [PATCH V5 1/1] qmp: add pmemload command, Baojun Wang, 2014/04/09
- Re: [Qemu-trivial] [PATCH V5 1/1] qmp: add pmemload command, Michael Tokarev, 2014/04/27
- Re: [Qemu-trivial] [Qemu-devel] [PATCH V5 1/1] qmp: add pmemload command, Markus Armbruster, 2014/04/28
- Re: [Qemu-trivial] [PATCH V5 1/1] qmp: add pmemload command, Baojun Wang, 2014/04/30