On Tue, Apr 17, 2018 at 04:18:43PM -0500, Eric Blake wrote:
> Focusing on just the interface:
>
>> +++ b/qapi/misc.json
>> @@ -1185,6 +1185,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
>> +#
>> +# @size: the size of memory region to load
>
> Should size be an optional parameter (default read to the end of the file)?
>
>> +#
>> +# @offset: the offset in the file to start from
>
> Should offset be an optional parameter (default start reading from
> offset 0 of the file)?
>From the QAPI point-of-view making both optional seems like a
good idea, however then pmemsave should also mark its "size"
parameter optional for consistency (this is backwards compatible
as existing callers are not affected).
On the monitor API side this is more problematic as the order of
the pmemsave/pmemload parameters make optional parameters
impossible.
Personally I'd keep it as is because it's simple and consistent
with the existing interface (and can be changed in the future
without breaking compatibility).
In case you prefer those parameters to be optional I'll require
some help to implement that: I don't understand how
qapi-schema.json, hmp-commands.hx and hmp.h interact (I've to
admit I just copied that from the original patch and didn't give
it much thought as it worked fine) and would require some
pointers on how to implement optional arguments for
qapi-schema.json.
Regards
Simon
--
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9
