On Wed, Sep 10, 2014 at 11:05:49PM -0600, Alex Williamson wrote: > On Thu, 2014-09-11 at 05:10 +0200, Christoffer Dall wrote: > > On Mon, Sep 01, 2014 at 02:52:47PM +0200, Eric Auger wrote: [...] > > > > > > +#ifdef __KVM_HAVE_ARCH_KVM_VFIO_FORWARD > > > +int kvm_arch_set_fwd_state(struct kvm_fwd_irq *pfwd, > > > > what's the 'p' in pfwd? > > p is for pointer? > shouldn't the type declation spell out quite clearly to me that I'm dealing with a pointer? [...] > > > > need some spaceing here, also, I would turn this around, first check if > > the strcmp fails, and then error out, then do you next check etc., to > > avoid so many nested statements. > > > > > + /* is a ref to this device already owned by the KVM-VFIO device? */ > > > > this comment is not particularly helpful in its current form, it would > > be helpful if you specified that we're checking whether that particular > > device/irq combo is already registered. > > > > > + *kvm_vdev = kvm_vfio_find_device(kv, vdev); > > > + if (*kvm_vdev) { > > > + if (kvm_vfio_find_irq(*kvm_vdev, fwd_irq->index)) { > > > + kvm_err("%s irq %d already forwarded\n", > > > + __func__, *hwirq); > > Why didn't we do this first? > huh? -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html