[linux-cifs-client] [PATCH 0/7] [CIFS] clean up cifs_setattr and fix setting of mtimes

[Günter Kukkukk]

Am Freitag, 23. Mai 2008 schrieb Jeff Layton:
> This patch breaks up the large and unwieldy cifs_setattr function into
> multiple pieces. In addition to making the function easier to follow and
> reducing indentation, this should also make it easier to implement a
> scheme to reduce the amount of times that we retry functions that we
> know will fail to a particular server. It also fixes the setting of
> mtimes (LastWriteTime) on open files with Windows servers.
> 
> This patch is based on top of the "dynperm" patchset that I've been
> posting, but which is not yet committed.
> 
> Comments and/or suggestions appreciated...
> 
> ---
> 
> Jeff Layton (7):
>       [CIFS] turn cifs_setattr into a multiplexor that calls the correct function
>       [CIFS] move file time and dos attribute setting logic into new function
>       [CIFS] Rename CIFSSMBSetFileTimes to CIFSSMBSetFileInfo and add PID arg
>       [CIFS] change CIFSSMBSetTimes to CIFSSMBSetPathInfo
>       [CIFS] spin off cifs_setattr with unix extensions to its own function
>       [CIFS] bundle up Unix SET_PATH_INFO args into a struct and change name
>       [CIFS] break ATTR_SIZE changes out into their own function
> 
> 
>  fs/cifs/cifspdu.h   |    2 
>  fs/cifs/cifsproto.h |   24 ++
>  fs/cifs/cifssmb.c   |   43 ++--
>  fs/cifs/dir.c       |   58 +++--
>  fs/cifs/file.c      |   19 +-
>  fs/cifs/inode.c     |  551 ++++++++++++++++++++++++++++++++-------------------
>  6 files changed, 422 insertions(+), 275 deletions(-)
> 

Hi Jeff,

i fully agree here to "unbundle" the current completely
overloaded function cifs_setattr() - which has to handle
too many _very_ different requests.

As you propose, the very first distinction should be whether
unix extensions are enabled - or not.
Then separate for "attr/mode" and "file times" changes.
Anyway - your approach will help a lot to "implement
missing legacy functionality"!

Ouch - your patch set is really large - and i really
can't tell whether your new stuff is still working...

Jeff, Steve,

what testcases are you using to check for functionality
and compatibilty in your own environments?
Could really help me to do "my own automated tests".

Btw - there are similar problems in

cifs_get_inode_info() ---> CIFSSMBQPathInfo()

... completely broken legacy support (and also
overloaded functions)

Cheers, Günter