On Tue, Apr 10, 2018 at 04:33:03PM -0500, Eric Blake wrote: >> +void qmp_pmemload(int64_t addr, int64_t size, const char *filename, >> + Error **errp) >> +{ >> + FILE *f; >> + size_t l; >> + uint8_t buf[1024]; >> + >> + f = fopen(filename, "rb"); > > Use qemu_fopen() here, so that you can automagically support /dev/fdset/ > magic filenames that work on file descriptors passed via SCM_RIGHTS.
Hello, I can't find qemu_fopen() in the source. Did you mean qemu_open()? From reading qemu_close() I guess that I can't use fdopen() (as there's no qemu_fclose()) but must work with the raw fd. Or is there an easy way to fdopen? (I prefer the FILE * interface which is easier to work with.) But I just copied the code from qmp_pmemsave. Should I change it as well to use qemu_open()? >> +++ b/qapi-schema.json >> @@ -1136,6 +1136,24 @@ >> { '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 > > Is 'val' really the best name for this, or would 'phys-addr' or similar > be a more descriptive name? I copied it from pmemsave to keep the argument names consistent. Should I change it only for pmemload? Changing it for pmemsave is problematic as it will break the existing API. >> +# >> +# @size: the size of memory region to load >> +# >> +# @filename: the file to load the memory from as binary data >> +# >> +# Returns: Nothing on success >> +# >> +# Since: 2.10 > > You've missed 2.10 by a long shot. The earliest this new feature could > appear is 2.13. Will change. > Do you additionally need an offset where to start reading from within > the file (that is, since you already have the 'size' parameter to avoid > reading the entire file, and the 'val' parameter to target anywhere in > physical memory, how do I start reading anywhere from the file)? I didn't need it for my usage and wanted to keep the patch as simple as possible. But I see how it can be useful and will add it to my patch in the next iteration. Thank you for the review! Regards Simon -- + privacy is necessary + using gnupg http://gnupg.org + public key id: 0x92FEFDB7E44C32F9