Re: [Qemu-devel] [PATCH] kvm: add set_one_reg/get_one_reg

[Alexey Kardashevskiy]

On 09/18/2013 05:59 PM, Peter Maydell wrote:
> On 18 September 2013 16:46, Alexey Kardashevskiy <address@hidden> wrote:
>> On 09/18/2013 05:14 PM, Peter Maydell wrote:
>>> On 18 September 2013 05:21, Alexey Kardashevskiy <address@hidden> wrote:
>>>> This adds QEMU wrappers for KVM_SET_ONE_REG/KVM_GET_ONE_REG ioctls.
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>>>> ---
>>>>  include/sysemu/kvm.h |  4 ++++
>>>>  kvm-all.c            | 31 +++++++++++++++++++++++++++++++
>>>>  2 files changed, 35 insertions(+)
>>>>
>>>> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
>>>> index c7bc07b..b2d61e9 100644
>>>> --- a/include/sysemu/kvm.h
>>>> +++ b/include/sysemu/kvm.h
>>>> @@ -319,4 +319,8 @@ int kvm_irqchip_remove_irqfd_notifier(KVMState *s, 
>>>> EventNotifier *n, int virq);
>>>>  void kvm_pc_gsi_handler(void *opaque, int n, int level);
>>>>  void kvm_pc_setup_irq_routing(bool pci_enabled);
>>>>  void kvm_init_irq_routing(KVMState *s);
>>>> +
>>>> +int kvm_set_one_reg(CPUState *cs, uint64_t id, void *addr);
>>>> +int kvm_get_one_reg(CPUState *cs, uint64_t id, void *addr);
>>>
>>> Doc comments, please.
>>
>> What comments? Do not the function names speak for themselves already?
> 
> It's a new public function, it should have a comment in the standard
> format saying what it does and what the input parameters and output are.
> Just to pick some examples at random, the fact that the return value
> is 0-on-success-or-negative-errno should be documented, and we should
> refer the user to the ioctl docs for valid id values. It doesn't need to be
> a long essay, but we should be documenting new functions as we add them.


It would be awesome if you just gave me any really good example of what you
expect from such a comment as kvm-all.c does not have any whatsoever. And
pci.c does not. And exec.c does not. But I am sure there is some as
it cannot possibly be me who starts making such comments in qemu. Thanks.


-- 
Alexey