This allows udev to trigger rules when a parent device is registered
or unregistered from mdev.
Signed-off-by: Alex Williamson <[email protected]>
---
drivers/vfio/mdev/mdev_core.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
index ae23151442cb..ecec2a3b13cb 100644
--- a/drivers/vfio/mdev/mdev_core.c
+++ b/drivers/vfio/mdev/mdev_core.c
@@ -146,6 +146,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
{
int ret;
struct mdev_parent *parent;
+ char *env_string = "MDEV_STATE=registered";
+ char *envp[] = { env_string, NULL };
/* check for mandatory ops */
if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
@@ -196,7 +198,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
list_add(&parent->next, &parent_list);
mutex_unlock(&parent_list_lock);
- dev_info(dev, "MDEV: Registered\n");
+ kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
+
return 0;
add_dev_err:
@@ -220,6 +223,8 @@ EXPORT_SYMBOL(mdev_register_device);
void mdev_unregister_device(struct device *dev)
{
struct mdev_parent *parent;
+ char *env_string = "MDEV_STATE=unregistered";
+ char *envp[] = { env_string, NULL };
mutex_lock(&parent_list_lock);
parent = __find_parent_device(dev);
@@ -228,7 +233,6 @@ void mdev_unregister_device(struct device *dev)
mutex_unlock(&parent_list_lock);
return;
}
- dev_info(dev, "MDEV: Unregistering\n");
list_del(&parent->next);
mutex_unlock(&parent_list_lock);
@@ -243,6 +247,8 @@ void mdev_unregister_device(struct device *dev)
up_write(&parent->unreg_sem);
mdev_put_parent(parent);
+
+ kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
}
EXPORT_SYMBOL(mdev_unregister_device);
On 6/26/2019 7:57 PM, Alex Williamson wrote:
> This allows udev to trigger rules when a parent device is registered
> or unregistered from mdev.
>
> Signed-off-by: Alex Williamson <[email protected]>
> ---
> drivers/vfio/mdev/mdev_core.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index ae23151442cb..ecec2a3b13cb 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -146,6 +146,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
> {
> int ret;
> struct mdev_parent *parent;
> + char *env_string = "MDEV_STATE=registered";
> + char *envp[] = { env_string, NULL };
>
> /* check for mandatory ops */
> if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
> @@ -196,7 +198,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
> list_add(&parent->next, &parent_list);
> mutex_unlock(&parent_list_lock);
>
> - dev_info(dev, "MDEV: Registered\n");
> + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
> +
Its good to have udev event, but don't remove debug print from dmesg.
Same for unregister.
Thanks,
Kirti
> return 0;
>
> add_dev_err:
> @@ -220,6 +223,8 @@ EXPORT_SYMBOL(mdev_register_device);
> void mdev_unregister_device(struct device *dev)
> {
> struct mdev_parent *parent;
> + char *env_string = "MDEV_STATE=unregistered";
> + char *envp[] = { env_string, NULL };
>
> mutex_lock(&parent_list_lock);
> parent = __find_parent_device(dev);
> @@ -228,7 +233,6 @@ void mdev_unregister_device(struct device *dev)
> mutex_unlock(&parent_list_lock);
> return;
> }
> - dev_info(dev, "MDEV: Unregistering\n");
>
> list_del(&parent->next);
> mutex_unlock(&parent_list_lock);
> @@ -243,6 +247,8 @@ void mdev_unregister_device(struct device *dev)
> up_write(&parent->unreg_sem);
>
> mdev_put_parent(parent);
> +
> + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
> }
> EXPORT_SYMBOL(mdev_unregister_device);
>
>
On Wed, 26 Jun 2019 23:23:00 +0530
Kirti Wankhede <[email protected]> wrote:
> On 6/26/2019 7:57 PM, Alex Williamson wrote:
> > This allows udev to trigger rules when a parent device is registered
> > or unregistered from mdev.
> >
> > Signed-off-by: Alex Williamson <[email protected]>
> > ---
> > drivers/vfio/mdev/mdev_core.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> > index ae23151442cb..ecec2a3b13cb 100644
> > --- a/drivers/vfio/mdev/mdev_core.c
> > +++ b/drivers/vfio/mdev/mdev_core.c
> > @@ -146,6 +146,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
> > {
> > int ret;
> > struct mdev_parent *parent;
> > + char *env_string = "MDEV_STATE=registered";
> > + char *envp[] = { env_string, NULL };
> >
> > /* check for mandatory ops */
> > if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
> > @@ -196,7 +198,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
> > list_add(&parent->next, &parent_list);
> > mutex_unlock(&parent_list_lock);
> >
> > - dev_info(dev, "MDEV: Registered\n");
> > + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
> > +
>
> Its good to have udev event, but don't remove debug print from dmesg.
> Same for unregister.
Who consumes these? They seem noisy. Thanks,
Alex
> > return 0;
> >
> > add_dev_err:
> > @@ -220,6 +223,8 @@ EXPORT_SYMBOL(mdev_register_device);
> > void mdev_unregister_device(struct device *dev)
> > {
> > struct mdev_parent *parent;
> > + char *env_string = "MDEV_STATE=unregistered";
> > + char *envp[] = { env_string, NULL };
> >
> > mutex_lock(&parent_list_lock);
> > parent = __find_parent_device(dev);
> > @@ -228,7 +233,6 @@ void mdev_unregister_device(struct device *dev)
> > mutex_unlock(&parent_list_lock);
> > return;
> > }
> > - dev_info(dev, "MDEV: Unregistering\n");
> >
> > list_del(&parent->next);
> > mutex_unlock(&parent_list_lock);
> > @@ -243,6 +247,8 @@ void mdev_unregister_device(struct device *dev)
> > up_write(&parent->unreg_sem);
> >
> > mdev_put_parent(parent);
> > +
> > + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
> > }
> > EXPORT_SYMBOL(mdev_unregister_device);
> >
> >
On 6/26/2019 11:35 PM, Alex Williamson wrote:
> On Wed, 26 Jun 2019 23:23:00 +0530
> Kirti Wankhede <[email protected]> wrote:
>
>> On 6/26/2019 7:57 PM, Alex Williamson wrote:
>>> This allows udev to trigger rules when a parent device is registered
>>> or unregistered from mdev.
>>>
>>> Signed-off-by: Alex Williamson <[email protected]>
>>> ---
>>> drivers/vfio/mdev/mdev_core.c | 10 ++++++++--
>>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
>>> index ae23151442cb..ecec2a3b13cb 100644
>>> --- a/drivers/vfio/mdev/mdev_core.c
>>> +++ b/drivers/vfio/mdev/mdev_core.c
>>> @@ -146,6 +146,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
>>> {
>>> int ret;
>>> struct mdev_parent *parent;
>>> + char *env_string = "MDEV_STATE=registered";
>>> + char *envp[] = { env_string, NULL };
>>>
>>> /* check for mandatory ops */
>>> if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
>>> @@ -196,7 +198,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
>>> list_add(&parent->next, &parent_list);
>>> mutex_unlock(&parent_list_lock);
>>>
>>> - dev_info(dev, "MDEV: Registered\n");
>>> + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
>>> +
>>
>> Its good to have udev event, but don't remove debug print from dmesg.
>> Same for unregister.
>
> Who consumes these? They seem noisy. Thanks,
>
I don't think its noisy, its more of logging purpose. This is seen in
kernel log only when physical device is registered to mdev.
Thanks,
Kirti
> Alex
>
>>> return 0;
>>>
>>> add_dev_err:
>>> @@ -220,6 +223,8 @@ EXPORT_SYMBOL(mdev_register_device);
>>> void mdev_unregister_device(struct device *dev)
>>> {
>>> struct mdev_parent *parent;
>>> + char *env_string = "MDEV_STATE=unregistered";
>>> + char *envp[] = { env_string, NULL };
>>>
>>> mutex_lock(&parent_list_lock);
>>> parent = __find_parent_device(dev);
>>> @@ -228,7 +233,6 @@ void mdev_unregister_device(struct device *dev)
>>> mutex_unlock(&parent_list_lock);
>>> return;
>>> }
>>> - dev_info(dev, "MDEV: Unregistering\n");
>>>
>>> list_del(&parent->next);
>>> mutex_unlock(&parent_list_lock);
>>> @@ -243,6 +247,8 @@ void mdev_unregister_device(struct device *dev)
>>> up_write(&parent->unreg_sem);
>>>
>>> mdev_put_parent(parent);
>>> +
>>> + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
>>> }
>>> EXPORT_SYMBOL(mdev_unregister_device);
>>>
>>>
>
On Wed, 26 Jun 2019 08:27:58 -0600
Alex Williamson <[email protected]> wrote:
> This allows udev to trigger rules when a parent device is registered
> or unregistered from mdev.
>
> Signed-off-by: Alex Williamson <[email protected]>
> ---
> drivers/vfio/mdev/mdev_core.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index ae23151442cb..ecec2a3b13cb 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -146,6 +146,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
> {
> int ret;
> struct mdev_parent *parent;
> + char *env_string = "MDEV_STATE=registered";
This string is probably reasonable enough.
> + char *envp[] = { env_string, NULL };
>
> /* check for mandatory ops */
> if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
> @@ -196,7 +198,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
> list_add(&parent->next, &parent_list);
> mutex_unlock(&parent_list_lock);
>
> - dev_info(dev, "MDEV: Registered\n");
> + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
I also agree with the positioning here.
> +
> return 0;
>
> add_dev_err:
> @@ -220,6 +223,8 @@ EXPORT_SYMBOL(mdev_register_device);
> void mdev_unregister_device(struct device *dev)
> {
> struct mdev_parent *parent;
> + char *env_string = "MDEV_STATE=unregistered";
Ok.
> + char *envp[] = { env_string, NULL };
>
> mutex_lock(&parent_list_lock);
> parent = __find_parent_device(dev);
> @@ -228,7 +233,6 @@ void mdev_unregister_device(struct device *dev)
> mutex_unlock(&parent_list_lock);
> return;
> }
> - dev_info(dev, "MDEV: Unregistering\n");
>
> list_del(&parent->next);
> mutex_unlock(&parent_list_lock);
> @@ -243,6 +247,8 @@ void mdev_unregister_device(struct device *dev)
> up_write(&parent->unreg_sem);
>
> mdev_put_parent(parent);
> +
> + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
I'm wondering whether we should indicate this uevent earlier: Once we
have detached from the parent list, we're basically done for all
practical purposes. So maybe move this right before we grab the
unreg_sem?
> }
> EXPORT_SYMBOL(mdev_unregister_device);
>
>
On Thu, 27 Jun 2019 00:33:59 +0530
Kirti Wankhede <[email protected]> wrote:
> On 6/26/2019 11:35 PM, Alex Williamson wrote:
> > On Wed, 26 Jun 2019 23:23:00 +0530
> > Kirti Wankhede <[email protected]> wrote:
> >
> >> On 6/26/2019 7:57 PM, Alex Williamson wrote:
> >>> This allows udev to trigger rules when a parent device is registered
> >>> or unregistered from mdev.
> >>>
> >>> Signed-off-by: Alex Williamson <[email protected]>
> >>> ---
> >>> drivers/vfio/mdev/mdev_core.c | 10 ++++++++--
> >>> 1 file changed, 8 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> >>> index ae23151442cb..ecec2a3b13cb 100644
> >>> --- a/drivers/vfio/mdev/mdev_core.c
> >>> +++ b/drivers/vfio/mdev/mdev_core.c
> >>> @@ -146,6 +146,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
> >>> {
> >>> int ret;
> >>> struct mdev_parent *parent;
> >>> + char *env_string = "MDEV_STATE=registered";
> >>> + char *envp[] = { env_string, NULL };
> >>>
> >>> /* check for mandatory ops */
> >>> if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
> >>> @@ -196,7 +198,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
> >>> list_add(&parent->next, &parent_list);
> >>> mutex_unlock(&parent_list_lock);
> >>>
> >>> - dev_info(dev, "MDEV: Registered\n");
> >>> + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
> >>> +
> >>
> >> Its good to have udev event, but don't remove debug print from dmesg.
> >> Same for unregister.
> >
> > Who consumes these? They seem noisy. Thanks,
> >
>
> I don't think its noisy, its more of logging purpose. This is seen in
> kernel log only when physical device is registered to mdev.
Yes; but why do you want to log success? If you need to log it
somewhere, wouldn't a trace event be a much better choice?
On 6/27/2019 1:51 PM, Cornelia Huck wrote:
> On Thu, 27 Jun 2019 00:33:59 +0530
> Kirti Wankhede <[email protected]> wrote:
>
>> On 6/26/2019 11:35 PM, Alex Williamson wrote:
>>> On Wed, 26 Jun 2019 23:23:00 +0530
>>> Kirti Wankhede <[email protected]> wrote:
>>>
>>>> On 6/26/2019 7:57 PM, Alex Williamson wrote:
>>>>> This allows udev to trigger rules when a parent device is registered
>>>>> or unregistered from mdev.
>>>>>
>>>>> Signed-off-by: Alex Williamson <[email protected]>
>>>>> ---
>>>>> drivers/vfio/mdev/mdev_core.c | 10 ++++++++--
>>>>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
>>>>> index ae23151442cb..ecec2a3b13cb 100644
>>>>> --- a/drivers/vfio/mdev/mdev_core.c
>>>>> +++ b/drivers/vfio/mdev/mdev_core.c
>>>>> @@ -146,6 +146,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
>>>>> {
>>>>> int ret;
>>>>> struct mdev_parent *parent;
>>>>> + char *env_string = "MDEV_STATE=registered";
>>>>> + char *envp[] = { env_string, NULL };
>>>>>
>>>>> /* check for mandatory ops */
>>>>> if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
>>>>> @@ -196,7 +198,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
>>>>> list_add(&parent->next, &parent_list);
>>>>> mutex_unlock(&parent_list_lock);
>>>>>
>>>>> - dev_info(dev, "MDEV: Registered\n");
>>>>> + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
>>>>> +
>>>>
>>>> Its good to have udev event, but don't remove debug print from dmesg.
>>>> Same for unregister.
>>>
>>> Who consumes these? They seem noisy. Thanks,
>>>
>>
>> I don't think its noisy, its more of logging purpose. This is seen in
>> kernel log only when physical device is registered to mdev.
>
> Yes; but why do you want to log success? If you need to log it
> somewhere, wouldn't a trace event be a much better choice?
>
Trace events are not always collected in production environment, there
kernel log helps.
Thanks,
Kirti
On Thu, 27 Jun 2019 10:19:14 +0200
Cornelia Huck <[email protected]> wrote:
> On Wed, 26 Jun 2019 08:27:58 -0600
> Alex Williamson <[email protected]> wrote:
>
> > This allows udev to trigger rules when a parent device is registered
> > or unregistered from mdev.
> >
> > Signed-off-by: Alex Williamson <[email protected]>
> > ---
> > drivers/vfio/mdev/mdev_core.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> > index ae23151442cb..ecec2a3b13cb 100644
> > --- a/drivers/vfio/mdev/mdev_core.c
> > +++ b/drivers/vfio/mdev/mdev_core.c
> > @@ -146,6 +146,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
> > {
> > int ret;
> > struct mdev_parent *parent;
> > + char *env_string = "MDEV_STATE=registered";
>
> This string is probably reasonable enough.
>
> > + char *envp[] = { env_string, NULL };
> >
> > /* check for mandatory ops */
> > if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
> > @@ -196,7 +198,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
> > list_add(&parent->next, &parent_list);
> > mutex_unlock(&parent_list_lock);
> >
> > - dev_info(dev, "MDEV: Registered\n");
> > + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
>
> I also agree with the positioning here.
>
> > +
> > return 0;
> >
> > add_dev_err:
> > @@ -220,6 +223,8 @@ EXPORT_SYMBOL(mdev_register_device);
> > void mdev_unregister_device(struct device *dev)
> > {
> > struct mdev_parent *parent;
> > + char *env_string = "MDEV_STATE=unregistered";
>
> Ok.
>
> > + char *envp[] = { env_string, NULL };
> >
> > mutex_lock(&parent_list_lock);
> > parent = __find_parent_device(dev);
> > @@ -228,7 +233,6 @@ void mdev_unregister_device(struct device *dev)
> > mutex_unlock(&parent_list_lock);
> > return;
> > }
> > - dev_info(dev, "MDEV: Unregistering\n");
> >
> > list_del(&parent->next);
> > mutex_unlock(&parent_list_lock);
> > @@ -243,6 +247,8 @@ void mdev_unregister_device(struct device *dev)
> > up_write(&parent->unreg_sem);
> >
> > mdev_put_parent(parent);
> > +
> > + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
>
> I'm wondering whether we should indicate this uevent earlier: Once we
> have detached from the parent list, we're basically done for all
> practical purposes. So maybe move this right before we grab the
> unreg_sem?
That would make it a "this thing is about to go away" (ie.
"unregistering") rather than "this thing is gone" ("unregistered"). I
was aiming for the latter as the former just seems like it might make
userspace race to remove devices. Note that I don't actually make use
of this event in mdevctl currently, so we could maybe save it for
later, but the symmetry seemed preferable. Thanks,
Alex
On Fri, 28 Jun 2019 09:56:08 -0600
Alex Williamson <[email protected]> wrote:
> On Thu, 27 Jun 2019 10:19:14 +0200
> Cornelia Huck <[email protected]> wrote:
>
> > On Wed, 26 Jun 2019 08:27:58 -0600
> > Alex Williamson <[email protected]> wrote:
> > > @@ -243,6 +247,8 @@ void mdev_unregister_device(struct device *dev)
> > > up_write(&parent->unreg_sem);
> > >
> > > mdev_put_parent(parent);
> > > +
> > > + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
> >
> > I'm wondering whether we should indicate this uevent earlier: Once we
> > have detached from the parent list, we're basically done for all
> > practical purposes. So maybe move this right before we grab the
> > unreg_sem?
>
> That would make it a "this thing is about to go away" (ie.
> "unregistering") rather than "this thing is gone" ("unregistered"). I
> was aiming for the latter as the former just seems like it might make
> userspace race to remove devices. Note that I don't actually make use
> of this event in mdevctl currently, so we could maybe save it for
> later, but the symmetry seemed preferable. Thanks,
>
> Alex
Fair enough. I was thinking about signaling that it does not make much
sense to register new devices after that point, but if that might
trigger userspace to actually try and remove devices, not much is
gained.
On Thu, 27 Jun 2019 19:42:32 +0530
Kirti Wankhede <[email protected]> wrote:
> On 6/27/2019 1:51 PM, Cornelia Huck wrote:
> > On Thu, 27 Jun 2019 00:33:59 +0530
> > Kirti Wankhede <[email protected]> wrote:
> >
> >> On 6/26/2019 11:35 PM, Alex Williamson wrote:
> >>> On Wed, 26 Jun 2019 23:23:00 +0530
> >>> Kirti Wankhede <[email protected]> wrote:
> >>>
> >>>> On 6/26/2019 7:57 PM, Alex Williamson wrote:
> >>>>> This allows udev to trigger rules when a parent device is registered
> >>>>> or unregistered from mdev.
> >>>>>
> >>>>> Signed-off-by: Alex Williamson <[email protected]>
> >>>>> ---
> >>>>> drivers/vfio/mdev/mdev_core.c | 10 ++++++++--
> >>>>> 1 file changed, 8 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> >>>>> index ae23151442cb..ecec2a3b13cb 100644
> >>>>> --- a/drivers/vfio/mdev/mdev_core.c
> >>>>> +++ b/drivers/vfio/mdev/mdev_core.c
> >>>>> @@ -146,6 +146,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
> >>>>> {
> >>>>> int ret;
> >>>>> struct mdev_parent *parent;
> >>>>> + char *env_string = "MDEV_STATE=registered";
> >>>>> + char *envp[] = { env_string, NULL };
> >>>>>
> >>>>> /* check for mandatory ops */
> >>>>> if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
> >>>>> @@ -196,7 +198,8 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
> >>>>> list_add(&parent->next, &parent_list);
> >>>>> mutex_unlock(&parent_list_lock);
> >>>>>
> >>>>> - dev_info(dev, "MDEV: Registered\n");
> >>>>> + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
> >>>>> +
> >>>>
> >>>> Its good to have udev event, but don't remove debug print from dmesg.
> >>>> Same for unregister.
> >>>
> >>> Who consumes these? They seem noisy. Thanks,
> >>>
> >>
> >> I don't think its noisy, its more of logging purpose. This is seen in
> >> kernel log only when physical device is registered to mdev.
> >
> > Yes; but why do you want to log success? If you need to log it
> > somewhere, wouldn't a trace event be a much better choice?
> >
>
> Trace events are not always collected in production environment, there
> kernel log helps.
I'm with you for *errors*, but I'm not sure you should rely on
*success* messages, though. If you want to be able to figure out the
sequence of registering etc. in all cases, I think it makes more sense
to invest in an infrastructure like tracing and make sure that is it
turned on for any system that matters.
On Wed, 26 Jun 2019 08:27:58 -0600
Alex Williamson <[email protected]> wrote:
> This allows udev to trigger rules when a parent device is registered
> or unregistered from mdev.
>
> Signed-off-by: Alex Williamson <[email protected]>
> ---
> drivers/vfio/mdev/mdev_core.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
Reviewed-by: Cornelia Huck <[email protected]>