Re: [Qemu-devel] [PATCH v3 1/8] target-ppc: Add helper for KVM_PPC_RTAS_DEFINE_TOKEN

[Alexander Graf]

On 29.08.2013, at 10:18, Alexey Kardashevskiy wrote:

> On 08/27/2013 06:36 PM, Alexander Graf wrote:
>> 
>> On 27.08.2013, at 06:10, Benjamin Herrenschmidt wrote:
>> 
>>> On Tue, 2013-08-27 at 03:48 +0200, Andreas Färber wrote:
>>>> Also, QEMU is definitely not the only project that has higher
>>>> acceptance
>>>> criteria than patch-works-for-the-patch-author. :)
>>> 
>>> There's a difference between high acceptance criteria and systematic
>>> bike shed painting including in some case request to turn reasonably
>>> meaningful identifiers into something that nobody would get :-)
>> 
>> The name doesn't tell at all what the function is doing. This is even true 
>> for the kernel ioctl, but that one is in now, so it's there to stay. Heck, I 
>> even had to look up the API documentation to know whether this sets an RTAS 
>> to be handled in-kernel or in-user-space.
>> 
>> Just come up with a good name and all is well. Or are you enjoying to 
>> complain on every patch review I do now?
> 
> 
> So - kvmppc_define_rtas_in_kernel() or kvmppc_define_rtas_kernel_token()?

The former probably.

> I would actually though that the very first "k" is for "Kernel" already but...

I a symmetric API you could also remove rtas handling from the kernel, so you 
could tell the kernel "handle this in kernel space" and "don't handle this in 
kernel space". Our naming needs to be precise enough that you can guess what is 
going on from it.


Alex