[linux-cifs-client] SMB_COM_CLOSE and LastWriteTime
Guenter Kukkukk
linux at kukkukk.com
Fri May 2 03:54:49 GMT 2008
Am Freitag, 25. April 2008 schrieb Jeff Layton:
> On Fri, 25 Apr 2008 05:27:23 +0200
> Guenter Kukkukk <linux at kukkukk.com> wrote:
>
> > 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?
> >
>
> There is is_inode_writable(), which seems to be lighter weight and
> doesn't do the implicit reopen. That might be more suitable here.
>
We can't use is_inode_writable(), cause we need "openfile->netfid"
from find_writable_file(); for CIFSSMBSetFileTimes().
> > > 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" ...
>
>
> See smb_proc_settime...
>
> smbfs uses the LastWriteTime in the close call to set it to the current
> mtime instead of setting it to 0xffffffff. smb_proc_settime just opens
> and closes the file. The close sets the LastWriteTime to the inode
> mtime.
>
> Perhaps we could use the same trick in CIFS. After all, 2s time
> resolution is probably good enough for legacy servers anyway. I'm not
> sure about smbfs here though -- looks like the file will be closed
> after return from that piece of the code. That may be OK for smbfs, but
> I'm not sure about CIFS.
>
> > >
> > > 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.
> >
>
> That seems reasonable to me. It would be nice to not clutter up the
> code with special cases for legacy servers.
I totally agree here - but when adding smbfs like legacy features -
some additional cases have to be checked (some will be ugly enough).
Now that smbfs will be dropped completely, SOMEONE has to decide,
whether more legacy support should be added to cifs vfs - or not!
I already spent lots of time in the past regarding more legacy
functionality in cifs vfs - and only a few went in.
I really would appreciate "a definite word" how to go on.
Steve ?
----
I've also setup a windows 98 box (2nd edition) - can now
test here against the following servers:
- most recent samba3.0.x
- most recent samba3.2.x
- windowsXP
- windowsNT
- windows98 (2nd edition)
- OS/2 Warpserver and Client
- (MSDOS/IBMDOS - never tried that in the past)
I did lots of testing and network sniffing during the last days.
Using my settime applet I fortunately also hit the winNT special
case - which I remembered vaguely. The following case really
makes sense:
if (!(pTcon->ses->flags & CIFS_SES_NT4)) !!!!!!!
rc = CIFSSMBSetTimes(xid, pTcon, full_path, &time_buf,
cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags &
CIFS_MOUNT_MAP_SPECIAL_CHR);
else
rc = -EOPNOTSUPP;
WindowsNT does _not_ support setPATHinfo (infolevel 257) and
returns 0x007c0001 == NT_STATUS_INVALID_LEVEL.
So a simple utimes() - without an open file handle - would _not_
work on winNT. In that special case open(), setFILEinfo(), close() really
makes sense. Is already in the current code, but not in my former
patch - but that sequence is working and must be added!
Same _seems_ (!) to also apply to windows98, but here in the same
case of
open()
setFILEinfo !!!!
close()
really strange stuff is happening - nonsense timestamps (year 1940, 1970)
.... but no error is shown (on the cmdline). I'll work on this, to explain
it further...
For OS/2, the used setFILEinfo/setPATHinfo infolevel makes no
sense at all - a different story.
More specific info later.
I know from mailing lists and lots of irc talks, that missing
or wrong time stamps "lead to lots of troubles in real life
network environments" - so I vote, we should handle that
carefully.
Cheers, Günter
Btw - i posted my settime applet already, I think, it can
really help here ...
More information about the linux-cifs-client
mailing list