Re: [PATCH]sysfs: Don't emit a warning when sysfs_rename_link() fails.

[ebiederm@xmission.com (Eric W. Biederman)]

Cornelia Huck <cornelia.huck@de.ibm.com> writes:

> On Wed, 17 Sep 2008 11:55:14 -0700,
> ebiederm@xmission.com (Eric W. Biederman) wrote:
>> Cornelia Huck <cornelia.huck@de.ibm.com> writes:


>> Or we are actually using a conflicting name.  In which case it is a
>> real and valid problem. 
>
> Which may be in userspace as well. In the past the networking folks
> were unhappy about the sysfs warning, claiming that failures should
> rather be handled in their layer.

Because the failures have always been handled in the networking
stack.

The only case that was ever warned about by sysfs were noop
renames.

So no we will not catch any userspace bugs with this code.

>> The netdev layer especially since the
>> networking layer already has validated that the rename is valid
>> before calling device_rename.
>
> Does it? Or do I just don't get it because of lack of coffee?

In the dev_change_name we either do dev_alloc_name
which generates a unique device name or __dev_get_by_name
which looks to see if we are already using that name.

The code in cfg80211_dev_rename is a trivial probe so I
don't think that is subtle.

>> > The following patch switches sysfs_rename_link() to non-warning symlink
>> > creation again. It is on top of the current driver core series.
>> 
>> We don't need this.  Using the non-warning symlink creation is unnecessary.
>> Using non-warning symlink creation hides real errors.
>
> For most cases, yes.

In these cases especially.

>> In practice any errors that show up will be errors in sysfs, because
>> the network subsystem validates everything before calling us.
>
> I was under the impression that no checks are done if the rename is
> triggered via ioctl. And it makes more sense to have the caller of the
> ioctl get an error than to spit a sysfs warning.

Currently there are exactly 2 callers of device_rename.
net/core/dev.c:dev_change_name 
net/wireless/core.c:cfg80211_dev_rename

Both of which verify that the rename is valid before
they call device_rename.  In fact that they have to because
the kobject layer does not have enough information to verify
that the rename is valid, and sysfs is not necessarily compiled in.

Eric