[linux-cifs-client] SMB_COM_CLOSE and LastWriteTime

[Guenter Kukkukk]

Am Donnerstag, 24. April 2008 schrieb Jeff Layton:
> On Thu, 24 Apr 2008 17:26:58 +0200
> Guenter Kukkukk <linux at kukkukk.com> wrote:
> 
> > Am Donnerstag, 24. April 2008 schrieb Jeff Layton:
> > > On Thu, 24 Apr 2008 16:13:46 +0200
> > > Guenter Kukkukk <linux at kukkukk.com> wrote:
> > > 
> > > > Am Donnerstag, 24. April 2008 schrieb Guenter Kukkukk:
> > > > 
> > > > > An additional note:
> > > > > i get the following, when i use
> > > > > cp -p foo /mnt/linux/cifs
> > > > > cp: preserving permissions for `/mnt/linux/cifs/settime.c': Input/output error
> > > > > I must specify the "noacl" mount option to avoid this.
> > > > > Happens when mounting recent 3.0.x or 3.2 samba server.
> > > > 
> > > > Ouch - should read  
> > > > 
> > > > cp -p settime.c /mnt/linux/cifs
> > > > cp: preserving permissions for `/mnt/linux/cifs/settime.c': Input/output error
> > > > 
> > > > We can leave this one alone atm - could also be a wrong server setup.
> > > > Cheers, Günter
> > > 
> > > 
> > > Agreed. This doesn't appear to be directly related to the problem with
> > > setting times.
> > > 
> > > I tested out the patch that you sent on March 13 and it seems to fix
> > > the problem of setting times against win2k3. My only concern is that it
> > > looks like it rips out some of the code that did an implicit
> > > open/setattr/close of a file when it's not already open. Some comments
> > > seem to indicate that older servers needed the file open in order to
> > > properly set the time. Do we know that this won't break against older
> > > servers?
> > > 
> > > Thanks,
> > 
> > I already worked on this area of code about 2 years ago, when Steve and
> > me tried to add some legacy functionality for os/2 (and win9x/me).
> > Most of those additions regarding setpathinfo, setfileinfo and also the
> > corresponding query functions did not really make it into the code that days...
> > The biggest problem was, that the legacy servers need different info levels
> > and so also the passed structures like FILE_BASIC_INFO do not match.
> > 
> > Not counting legacy stuff, I remember that the special handling for 
> > CIFS_SES_NT4 made some sense those days, but in my recent more comprehensive
> > tests i did not "hit any reason for that" anymore.
> > My recent patch also works with winnt.
> > Probably Steve can comment on this.
> > 
> > I already did some recent work to add more legacy functionality. 
> > Talked to Steve at sambaxp whether we should place all/most of that
> > new functions into a separate "legacy" source file. He agreed on this.
> > One additional needed time conversion function could still be placed into
> > netmisc.c. But more on that later ...
> > 
> 
> I'm afraid I don't have a working NT4 image, but I did test this patch
> against Win98se. cp -p always failed to set the mtime for me on that
> host before. If I understand what wireshark is telling me,
> Win98se doesn't support SET_PATH_INFO calls. It does support
> SET_FILE_INFO calls, however, so this patch fixes mtime setting when
> doing a cp -p to a win98se share (since the file is open we end up
> using SET_FILE_INFO here).
> 
> I suspect though, that we'll probably still need the implicit
> open/set_file_info/close to make win98se and other early vintage
> servers work correctly (if it's deemed worthwhile to fix them).
> 
> If we care about fixing this universally, we'll some fairly
> complex logic:
> 
> 1) if file is open use SET_FILE_INFO

Due to the (strange) troubles with windows here, i agree that this
one should be checked _first_ - unfortunately with some costs!
Is "find_writable_file(cifsInode);" the _only_ way to check for
the file open case?

> 2) if file is closed and server supports it, use SET_PATH_INFO
> 3) if file is close and server doesn't support SET_PATH_INFO, then open
> the file, use SET_FILE_INFO, and close it again.

I need to setup a win9x/me system to check this a bit more.
AFAIR - nearly all (legacy) servers support SET_PATH_INFO, it's
just a question of the requested info level (which isn't done
right atm).
I must admit that i never tested against MSDOS/IBMDOS servers....

Btw - i had a short look at the smbfs sources. I haven't found 
that "special" sequence
  open
  SET_FILE_INFO
  close
So i think, we don't need that at all - it should be enough legacy
support in cifsfs, when it "closely matches smbfs features" ... 
> 
> I think that even this is probably too simple, and we might need to
> special case some stuff, but I suspect this will get most cases...
> 
We need to check "the special cases" inside the current code - but
should put those additional functions into separate (legacy) source
file(s).
Most of the legacy functionality is already implemented - and the
currently known missing ones will NOT blow up the cifs memory footstep
heavily.

So i think, we should do it step by step:
  - add my patch to cifs_setattr()
  - add a similar change to cifs_unlink()
to fix winnt, winxp, w2k3 ... 
Lateron we can add more legacy support in another step.

Cheers, Günter

Btw - there are "other known places" where legacy support is
not working atm.