From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ingo Molnar Subject: Re: [PATCH 3/3] kvm tools: Convert virtio devices to use IRQ registry Date: Fri, 6 May 2011 21:16:03 +0200 Message-ID: <20110506191603.GD7151@elte.hu> References: <1304681052-30992-1-git-send-email-levinsasha928@gmail.com> <1304681052-30992-3-git-send-email-levinsasha928@gmail.com> <20110506115635.GA17112@elte.hu> <1304700608.10534.11.camel@lappy> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: penberg@kernel.org, asias.hejun@gmail.com, gorcunov@gmail.com, prasadjoshi124@gmail.com, kvm@vger.kernel.org To: Sasha Levin Return-path: Received: from mx3.mail.elte.hu ([157.181.1.138]:39154 "EHLO mx3.mail.elte.hu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932202Ab1EFTQN (ORCPT ); Fri, 6 May 2011 15:16:13 -0400 Content-Disposition: inline In-Reply-To: <1304700608.10534.11.camel@lappy> Sender: kvm-owner@vger.kernel.org List-ID: * Sasha Levin wrote: > On Fri, 2011-05-06 at 13:56 +0200, Ingo Molnar wrote: > > * Sasha Levin wrote: > > > > > + bdev->pci_device.irq_pin = pin; > > > + bdev->pci_device.irq_line = line; > > > > One small remaining naming inconsistency caught my eyes. The generic convention > > should be something like: > > > > - structure names should be along the 'struct xyz_device' scheme > > > > - structure field names should be 'xyz_dev' > > > > - variable names within xyz driver's .c file should be 'xdev', > > but 'xyz_dev' is OK too, especially if used in some other file) > > > > In that sense, the above should be: > > > > bdev->pci_dev.irq_pin = pin; > > bdev->pci_dev.irq_line = line; > > > > This could be fixed in a followup patch - and there's more of the same > > inconsistency in other driver files as well. > > > > If such details are sorted out early on in a project's lifetime it will be > > applied in a very natural way as the code grows. > > The struct name there actually refers to a PCI header of a device, and > not an actual device. > > I'll rename it to pci_hdr instead of making it pci_dev, since it looks > more confusing than desirable. Yeah, indeed that would be good, it certainly confused me! :-) Thanks, Ingo