qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Simon Ruderich
Subject: Re: [Qemu-devel] [PATCH v2 5/5] qmp: add pmemload command
Date: Tue, 24 Apr 2018 16:50:53 +0200
User-agent: Mutt/1.9.5 (2018-04-13)

On Mon, Apr 23, 2018 at 02:46:57PM -0500, Eric Blake wrote:
> Back-compat in the QMP interface matters.  The HMP interface, however,
> exists to serve humans not machines, and we can break it at will to
> something that makes more sense to humans.  So don't let HMP concerns
> hold you back from a sane interface.

I see. However I don't like breaking existing interfaces unless I
have to. In this case I think not having the optional parameters
is fine and the consistency between the existing memsave/pmemsave
functions is more important.

> Optional parameters are listed as '*name':'type' in the .json file,
> which tells the generator to create a 'has_name' bool parameter
> alongside the 'name' parameter in the C code for the QMP interface.  The
> HMP interface should then call into the QMP interface.
>
> Recent HMP patches that I've authored may offer some inspiration: commit
> 08fb10a added a new command, and commit dba4932 added an optional
> parameter to an existing command.

Thank you for the explanation, this looks straight-forward.

Do you have strong opinions regarding the optional parameters or
would you accept the patch as is (minus possible implementation
issues)? I like the symmetry to the existing functions (I noticed
that size can only be optional for pmemload because saving the
complete memory doesn't sound useful) and having to specify
size/offset doesn't hurt the caller too much.

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9



reply via email to

[Prev in Thread] Current Thread [Next in Thread]