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

[Cornelia Huck]

On Wed, 17 Sep 2008 11:55:14 -0700,
ebiederm@xxxxxxxxxxxx (Eric W. Biederman) wrote:

> Cornelia Huck <cornelia.huck@xxxxxxxxxx> writes:
> 
> > Hi Greg, hi Eric,
> >
> > the recent sysfs tagged directory changes switched device_rename() to
> > sysfs_rename_link() - which is a good thing but AFAICS re-introduces
> > the scary warnings when a netdevice is renamed to something that
> > already exists (which I tried to fix with
> > 36ce6dad6e3cb3f050ed41e0beac0070d2062b25).
> 
> A netdevice can not be renamed to something that already exists, correctly
> and still emit warnings.  Either it is a noop rename in which case
> the fact that we delete the link before creating it will avoid warnings.

Sure, that case is fine.

> 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.

> 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?

> 
> > 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 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/