Re: [PATCH 3/5] KVM: pci device assignment

[Avi Kivity <avi@qumranet.com>]

Ben-Ami Yassour wrote:
> On Tue, 2008-07-29 at 14:49 +0530, Amit Shah wrote:
>   
>> * On Monday 28 Jul 2008 21:56:26 Ben-Ami Yassour wrote:
>>
>>     
>>> +static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
>>> +				      struct kvm_assigned_pci_dev *assigned_dev)
>>> +{
>>>       
>>     
>>> +	if (pci_enable_device(dev)) {
>>> +		printk(KERN_INFO "%s: Could not enable PCI device\n", __func__);
>>> +		r = -EBUSY;
>>> +		goto out_put;
>>> +	}
>>> +	r = pci_request_regions(dev, "kvm_assigned_device");
>>> +	if (r) {
>>> +		printk(KERN_INFO "%s: Could not get access to device regions\n",
>>> +		       __func__);
>>> +		goto out_disable;
>>>       
>> Shouldn't disable here unconditionally (see my comment earlier to the previous 
>> patch).
>>     
> Why? the device should not be used by the host at the same time.
> What is the condition that you were thinking of?
>
>   

pci_enable_device() can succeed even if the device was already enabled, 
so Amit was probably wishing to avoid an assignment failure disabling a 
device under a driver's feet.  But I see that pci_disable_device() will 
pair with pci_enable_device() correctly (doing reference counts), so I 
think the code is correct as is.

-- 
error compiling committee.c: too many arguments to function