| Message ID | 524f69650903041102v415a807m8a93f0462458a51@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2009-03-04 at 13:02 -0600, Steve French wrote: > Attached is patch to workaround the problem found in posix open to > Samba versions 3.3.1 and earlier > (create works, but open would fail with invalid parameter) - it > disables requests to try posix open after > a first failure. Why do you completely disable posix opens ? Wouldn't it make sense to reopen only when it fails ? After all when you create new files it works correctly. Simo.
On Wed, Mar 04, 2009 at 02:24:30PM -0500, simo wrote: > On Wed, 2009-03-04 at 13:02 -0600, Steve French wrote: > > Attached is patch to workaround the problem found in posix open to > > Samba versions 3.3.1 and earlier > > (create works, but open would fail with invalid parameter) - it > > disables requests to try posix open after > > a first failure. > > Why do you completely disable posix opens ? > Wouldn't it make sense to reopen only when it fails ? > After all when you create new files it works correctly. No, you don't want to do this complexity. Much more simple to just disable that call if you're running against a buggy server. Jeremy.
On Wed, Mar 4, 2009 at 1:02 PM, Steve French <smfrench@gmail.com> wrote: > Attached is patch to workaround the problem found in posix open to > Samba versions 3.3.1 and earlier > (create works, but open would fail with invalid parameter) - it > disables requests to try posix open after > a first failure. > > > > -- > Thanks, > > Steve > > _______________________________________________ > linux-cifs-client mailing list > linux-cifs-client@lists.samba.org > https://lists.samba.org/mailman/listinfo/linux-cifs-client > > Steve, I think I will have to apply similar logic in cifs_lookup for lookup intents i.e. when cifs_posix_open fails with the same return code, follow the usual lookup path, I do not have to disable posix opens though as Simo said. Would be wasting a transaction in the process though in case of samba servers that have this problem. Regards, Shirish
We still do posix open in the cifs_create (file creation) path since
that does not appear to be broken:
if (tcon->unix_ext && (tcon->ses->capabilities & CAP_UNIX) &&
(CIFS_UNIX_POSIX_PATH_OPS_CAP &
le64_to_cpu(tcon->fsUnixInfo.Capability))) {
rc = cifs_posix_open(full_path, &newinode, inode->i_sb,
mode, oflags, &oplock, &fileHandle, xid);
and if we try to reopen file, we still try reopening posix first (since reopen
is rarer, and not performance sensitive, it is ok to try twice)
We just disable it in the cifs_open (not create) path:
if (!tcon->broken_posix_open && tcon->unix_ext &&
(tcon->ses->capabilities & CAP_UNIX) &&
(CIFS_UNIX_POSIX_PATH_OPS_CAP &
le64_to_cpu(tcon->fsUnixInfo.Capability))) {
int oflags = (int) cifs_posix_convert_flags(file->f_flags);
/* can not refresh inode info since size could be stale */
rc = cifs_posix_open(full_path, &inode, inode->i_sb,
cifs_sb->mnt_file_mode /* ignored */,
oflags, &oplock, &netfid, xid);
if (rc == 0) {
cFYI(1, ("posix open succeeded"));
/* no need for special case handling of setting mode
on read only files needed here */
cifs_posix_open_inode_helper(inode, file, pCifsInode,
pCifsFile, oplock, netfid);
goto out;
} else if ((rc == -EINVAL) || (rc == -EOPNOTSUPP)) {
if (tcon->ses->serverNOS)
cERROR(1, ("server %s of type %s returned"
" unexpected error on SMB posix open"
", disabling posix open support."
" Check if server update available.",
tcon->ses->serverName,
tcon->ses->serverNOS));
On Wed, Mar 4, 2009 at 1:24 PM, simo <idra@samba.org> wrote:
> On Wed, 2009-03-04 at 13:02 -0600, Steve French wrote:
>> Attached is patch to workaround the problem found in posix open to
>> Samba versions 3.3.1 and earlier
>> (create works, but open would fail with invalid parameter) - it
>> disables requests to try posix open after
>> a first failure.
>
> Why do you completely disable posix opens ?
> Wouldn't it make sense to reopen only when it fails ?
> After all when you create new files it works correctly.
>
> Simo.
>
> --
> Simo Sorce
> Samba Team GPL Compliance Officer <simo@samba.org>
> Principal Software Engineer at Red Hat, Inc. <simo@redhat.com>
>
>
On Wed, Mar 4, 2009 at 1:29 PM, Jeremy Allison <jra@samba.org> wrote: > On Wed, Mar 04, 2009 at 02:24:30PM -0500, simo wrote: > No, you don't want to do this complexity. Much more > simple to just disable that call if you're running > against a buggy server. Current patch is pretty simple ... don't want to make it more complicated if possible
On Wed, 2009-03-04 at 13:30 -0600, Steve French wrote: > and if we try to reopen file, we still try reopening posix first > (since reopen > is rarer, and not performance sensitive, it is ok to try twice) > > We just disable it in the cifs_open (not create) path: Oh ok, I misread the patch, thanks! Simo.
diff --git a/fs/cifs/CHANGES b/fs/cifs/CHANGES index b33c841..fc977df 100644 --- a/fs/cifs/CHANGES +++ b/fs/cifs/CHANGES @@ -11,6 +11,8 @@ to better ensure that we wait for server to write all of the data to server disk (not just write it over the network). Add new mount parameter to allow user to disable sending the (slow) SMB flush on fsync if desired (fsync still flushes all cached write data to the server). +Posix file open support added (turned off after one attempt if server +fails to support it properly, as with Samba server versions prior to 3.3.2) Version 1.56 ------------ diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index 44ff94d..9fbf4df 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -299,6 +299,7 @@ struct cifsTconInfo { bool unix_ext:1; /* if false disable Linux extensions to CIFS protocol for this mount even if server would support */ bool local_lease:1; /* check leases (only) on local system not remote */ + bool broken_posix_open; /* e.g. Samba server versions < 3.3.2, 3.2.9 */ bool need_reconnect:1; /* connection reset, tid now invalid */ /* BB add field for back pointer to sb struct(s)? */ }; diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 7bef4cc..b877f9a 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -328,7 +328,8 @@ int cifs_open(struct inode *inode, struct file *file) else oplock = 0; - if (tcon->unix_ext && (tcon->ses->capabilities & CAP_UNIX) && + if (!tcon->broken_posix_open && tcon->unix_ext && + (tcon->ses->capabilities & CAP_UNIX) && (CIFS_UNIX_POSIX_PATH_OPS_CAP & le64_to_cpu(tcon->fsUnixInfo.Capability))) { int oflags = (int) cifs_posix_convert_flags(file->f_flags); @@ -344,11 +345,19 @@ int cifs_open(struct inode *inode, struct file *file) cifs_posix_open_inode_helper(inode, file, pCifsInode, pCifsFile, oplock, netfid); goto out; + } else if ((rc == -EINVAL) || (rc == -EOPNOTSUPP)) { + if (tcon->ses->serverNOS) + cERROR(1, ("server of type %s returned" + " unexpected error on SMB posix open" + ", disabling posix open support." + " Check if server update available.", + tcon->ses->serverNOS)); + tcon->broken_posix_open = true; } else if ((rc != -EIO) && (rc != -EREMOTE) && (rc != -EOPNOTSUPP)) /* path not found or net err */ goto out; - /* fallthrough to retry open the old way on operation - not supported or DFS errors */ + /* else fallthrough to retry open the old way on network i/o + or DFS errors */ } desiredAccess = cifs_convert_flags(file->f_flags);