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

Jason Wang
2019-10-16T04:41:10Z

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);