Re: [PATCH V3 1/7] mdev: class id support

[Jason Wang <jasowang@redhat.com>]

On 2019/10/16 上午12:38, Alex Williamson wrote:
> On Fri, 11 Oct 2019 16:15:51 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>    
>> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
>> index b558d4cfd082..724e9b9841d8 100644
>> --- a/drivers/vfio/mdev/mdev_core.c
>> +++ b/drivers/vfio/mdev/mdev_core.c
>> @@ -45,6 +45,12 @@ void mdev_set_drvdata(struct mdev_device *mdev, void *data)
>>   }
>>   EXPORT_SYMBOL(mdev_set_drvdata);
>>   
>> +void mdev_set_class(struct mdev_device *mdev, u16 id)
>> +{
>> +	mdev->class_id = id;
>> +}
>> +EXPORT_SYMBOL(mdev_set_class);
>> +
>>   struct device *mdev_dev(struct mdev_device *mdev)
>>   {
>>   	return &mdev->dev;
>> @@ -135,6 +141,7 @@ static int mdev_device_remove_cb(struct device *dev, void *data)
>>    * mdev_register_device : Register a device
>>    * @dev: device structure representing parent device.
>>    * @ops: Parent device operation structure to be registered.
>> + * @id: class id.
>>    *
>>    * Add device to list of registered parent devices.
>>    * Returns a negative value on error, otherwise 0.
>> @@ -324,6 +331,9 @@ int mdev_device_create(struct kobject *kobj,
>>   	if (ret)
>>   		goto ops_create_fail;
>>   
>> +	if (!mdev->class_id)
> This is a sanity test failure of the parent driver on a privileged
> path, I think it's fair to print a warning when this occurs rather than
> only return an errno to the user.  In fact, ret is not set to an error
> value here, so it looks like this fails to create the device but
> returns success.  Thanks,
>
> Alex


Will fix.

Thanks


>
>> +		goto class_id_fail;
>> +
>>   	ret = device_add(&mdev->dev);
>>   	if (ret)
>>   		goto add_fail;
>> @@ -340,6 +350,7 @@ int mdev_device_create(struct kobject *kobj,
>>   
>>   sysfs_fail:
>>   	device_del(&mdev->dev);
>> +class_id_fail:
>>   add_fail:
>>   	parent->ops->remove(mdev);
>>   ops_create_fail: