From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34181) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VMFnd-0005XE-9q for qemu-devel@nongnu.org; Wed, 18 Sep 2013 07:20:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VMFnY-0002Uw-8L for qemu-devel@nongnu.org; Wed, 18 Sep 2013 07:20:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:64165) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VMFnY-0002UV-0d for qemu-devel@nongnu.org; Wed, 18 Sep 2013 07:19:56 -0400 Message-ID: <5239612F.5040500@redhat.com> Date: Wed, 18 Sep 2013 10:15:43 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1379478118-20448-1-git-send-email-aik@ozlabs.ru> <52395A5E.2010006@ozlabs.ru> <52395F88.7000203@ozlabs.ru> In-Reply-To: <52395F88.7000203@ozlabs.ru> Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] kvm: add set_one_reg/get_one_reg List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: Peter Maydell , QEMU Developers , Alexander Graf Il 18/09/2013 10:08, Alexey Kardashevskiy ha scritto: > On 09/18/2013 05:59 PM, Peter Maydell wrote: >> On 18 September 2013 16:46, Alexey Kardashevskiy wrote: >>> On 09/18/2013 05:14 PM, Peter Maydell wrote: >>>> On 18 September 2013 05:21, Alexey Kardashevskiy wrote: >>>>> This adds QEMU wrappers for KVM_SET_ONE_REG/KVM_GET_ONE_REG ioctls. >>>>> >>>>> Signed-off-by: Alexey Kardashevskiy >>>>> --- >>>>> 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. include/exec/memory.h, include/qom/object.h, include/block/blockjob.h, include/qemu/main-loop.h, include/block/aio.h have many, thankfully. Paolo