On 04/09/2014 10:54 AM, Baojun Wang wrote: > Signed-off-by: Baojun Wang <wan...@gmail.com>
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