Re: [Qemu-devel] [PATCH v3 5/5] qmp: add pmemload command

[Markus Armbruster]

Simon Ruderich <address@hidden> writes:

> On Thu, Aug 16, 2018 at 07:43:41AM +0200, Markus Armbruster wrote:
>>> On Fri, Aug 10, 2018 at 11:36:51AM +0100, Dr. David Alan Gilbert wrote:
>>>> Also, had you considered rearranging and making them optional,
>>>> for example if you do:
>>>>
>>>> val:l,filename:F,offset:i?,size:i?
>>>>
>>>> I think that would mean you can do the fairly obvious:
>>>>   pmemload addr "myfile"
>>>>
>>>> with the assumption that loads the whole file.
>>>
>>> This would deviate from pmemsave/memsave, but feels more natural.
>>
>> The different order or arguments in HMP is somewhat ugly.  Okay if it
>> makes the command more pleasant to use.  Up to you and Dave to decide.
>>
>> If you decide to deviate, consider
>>
>>     filename:F,address:l,size:i?,offset:i?
>
> From what I understand we can't have optional arguments in the
> middle. Would you prefer mandatory size/offset parameters in HMP
> or optional parameters but inconsistent with pmemsave/memsave?

If you guys decide you want consistency with memsave, that's fine with
me.  If you decide you want to rearrange arguments for better usability,
that's also fine.  I just wanted to throw in another rearrangement for
you to consider.