[PATCH] cifs: remove "legacy" parm from CIFSSMBQPathInfo

[PATCH] cifs: remove "legacy" parm from CIFSSMBQPathInfo

[Suresh Jayaraman]

as all the callers are setting it to 0 anyway.

I stumbled upon this code when I was looking at the strange hardlink
behavior reported in the mailing list. But it seems this patch was
originally posted by Jeff Layton on Feb:
  http://lists.samba.org/archive/linux-cifs-client/2010-February/005557.html

I didn't see any discussions, ACK/NACK on this patch.


Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
---
 fs/cifs/cifsproto.h |    1 -
 fs/cifs/cifssmb.c   |   22 +++-------------------
 fs/cifs/connect.c   |    2 +-
 fs/cifs/inode.c     |    1 -
 4 files changed, 4 insertions(+), 22 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index edb6d90..fa7e8d1 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -167,7 +167,6 @@ extern int CIFSSMBQFileInfo(const int xid, struct cifsTconInfo *tcon,
 extern int CIFSSMBQPathInfo(const int xid, struct cifsTconInfo *tcon,
 			const unsigned char *searchName,
 			FILE_ALL_INFO *findData,
-			int legacy /* whether to use old info level */,
 			const struct nls_table *nls_codepage, int remap);
 extern int SMBQueryInformation(const int xid, struct cifsTconInfo *tcon,
 			const unsigned char *searchName,
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index e98f1f3..392c01b 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -3356,7 +3356,6 @@ int
 CIFSSMBQPathInfo(const int xid, struct cifsTconInfo *tcon,
 		 const unsigned char *searchName,
 		 FILE_ALL_INFO *pFindData,
-		 int legacy /* old style infolevel */,
 		 const struct nls_table *nls_codepage, int remap)
 {
 /* level 263 SMB_QUERY_FILE_ALL_INFO */
@@ -3406,10 +3405,7 @@ QPathInfoRetry:
 	byte_count = params + 1 /* pad */ ;
 	pSMB->TotalParameterCount = cpu_to_le16(params);
 	pSMB->ParameterCount = pSMB->TotalParameterCount;
-	if (legacy)
-		pSMB->InformationLevel = cpu_to_le16(SMB_INFO_STANDARD);
-	else
-		pSMB->InformationLevel = cpu_to_le16(SMB_QUERY_FILE_ALL_INFO);
+	pSMB->InformationLevel = cpu_to_le16(SMB_QUERY_FILE_ALL_INFO);
 	pSMB->Reserved4 = 0;
 	pSMB->hdr.smb_buf_length += byte_count;
 	pSMB->ByteCount = cpu_to_le16(byte_count);
@@ -3423,26 +3419,14 @@ QPathInfoRetry:
 
 		if (rc) /* BB add auto retry on EOPNOTSUPP? */
 			rc = -EIO;
-		else if (!legacy && (pSMBr->ByteCount < 40))
+		else if (pSMBr->ByteCount < 40)
 			rc = -EIO;	/* bad smb */
-		else if (legacy && (pSMBr->ByteCount < 24))
-			rc = -EIO;  /* 24 or 26 expected but we do not read
-					last field */
 		else if (pFindData) {
-			int size;
 			__u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset);
 
-			/* On legacy responses we do not read the last field,
-			EAsize, fortunately since it varies by subdialect and
-			also note it differs on Set vs. Get, ie two bytes or 4
-			bytes depending but we don't care here */
-			if (legacy)
-				size = sizeof(FILE_INFO_STANDARD);
-			else
-				size = sizeof(FILE_ALL_INFO);
 			memcpy((char *) pFindData,
 			       (char *) &pSMBr->hdr.Protocol +
-			       data_offset, size);
+			       data_offset, sizeof(FILE_ALL_INFO));
 		} else
 		    rc = -ENOMEM;
 	}
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 469c3dd..b44bfd5 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2629,7 +2629,7 @@ is_path_accessible(int xid, struct cifsTconInfo *tcon,
 		return -ENOMEM;
 
 	rc = CIFSSMBQPathInfo(xid, tcon, full_path, pfile_info,
-			      0 /* not legacy */, cifs_sb->local_nls,
+			      cifs_sb->local_nls,
 			      cifs_sb->mnt_cifs_flags &
 				CIFS_MOUNT_MAP_SPECIAL_CHR);
 	kfree(pfile_info);
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 39869c3..2448ad1 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -615,7 +615,6 @@ int cifs_get_inode_info(struct inode **pinode,
 
 		/* could do find first instead but this returns more info */
 		rc = CIFSSMBQPathInfo(xid, pTcon, full_path, pfindData,
-			      0 /* not legacy */,
 			      cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
 				CIFS_MOUNT_MAP_SPECIAL_CHR);
 		/* BB optimize code so we do not make the above call

Re: [PATCH] cifs: remove "legacy" parm from CIFSSMBQPathInfo

[Shirish Pargaonkar]

On Thu, Oct 28, 2010 at 2:10 AM, Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
> as all the callers are setting it to 0 anyway.
>
> I stumbled upon this code when I was looking at the strange hardlink
> behavior reported in the mailing list. But it seems this patch was
> originally posted by Jeff Layton on Feb:
>  http://lists.samba.org/archive/linux-cifs-client/2010-February/005557.html
>
> I didn't see any discussions, ACK/NACK on this patch.
>
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
> ---
>  fs/cifs/cifsproto.h |    1 -
>  fs/cifs/cifssmb.c   |   22 +++-------------------
>  fs/cifs/connect.c   |    2 +-
>  fs/cifs/inode.c     |    1 -
>  4 files changed, 4 insertions(+), 22 deletions(-)
>
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index edb6d90..fa7e8d1 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -167,7 +167,6 @@ extern int CIFSSMBQFileInfo(const int xid, struct cifsTconInfo *tcon,
>  extern int CIFSSMBQPathInfo(const int xid, struct cifsTconInfo *tcon,
>                        const unsigned char *searchName,
>                        FILE_ALL_INFO *findData,
> -                       int legacy /* whether to use old info level */,
>                        const struct nls_table *nls_codepage, int remap);
>  extern int SMBQueryInformation(const int xid, struct cifsTconInfo *tcon,
>                        const unsigned char *searchName,
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index e98f1f3..392c01b 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -3356,7 +3356,6 @@ int
>  CIFSSMBQPathInfo(const int xid, struct cifsTconInfo *tcon,
>                 const unsigned char *searchName,
>                 FILE_ALL_INFO *pFindData,
> -                int legacy /* old style infolevel */,
>                 const struct nls_table *nls_codepage, int remap)
>  {
>  /* level 263 SMB_QUERY_FILE_ALL_INFO */
> @@ -3406,10 +3405,7 @@ QPathInfoRetry:
>        byte_count = params + 1 /* pad */ ;
>        pSMB->TotalParameterCount = cpu_to_le16(params);
>        pSMB->ParameterCount = pSMB->TotalParameterCount;
> -       if (legacy)
> -               pSMB->InformationLevel = cpu_to_le16(SMB_INFO_STANDARD);
> -       else
> -               pSMB->InformationLevel = cpu_to_le16(SMB_QUERY_FILE_ALL_INFO);
> +       pSMB->InformationLevel = cpu_to_le16(SMB_QUERY_FILE_ALL_INFO);
>        pSMB->Reserved4 = 0;
>        pSMB->hdr.smb_buf_length += byte_count;
>        pSMB->ByteCount = cpu_to_le16(byte_count);
> @@ -3423,26 +3419,14 @@ QPathInfoRetry:
>
>                if (rc) /* BB add auto retry on EOPNOTSUPP? */
>                        rc = -EIO;
> -               else if (!legacy && (pSMBr->ByteCount < 40))
> +               else if (pSMBr->ByteCount < 40)
>                        rc = -EIO;      /* bad smb */
> -               else if (legacy && (pSMBr->ByteCount < 24))
> -                       rc = -EIO;  /* 24 or 26 expected but we do not read
> -                                       last field */
>                else if (pFindData) {
> -                       int size;
>                        __u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset);
>
> -                       /* On legacy responses we do not read the last field,
> -                       EAsize, fortunately since it varies by subdialect and
> -                       also note it differs on Set vs. Get, ie two bytes or 4
> -                       bytes depending but we don't care here */
> -                       if (legacy)
> -                               size = sizeof(FILE_INFO_STANDARD);
> -                       else
> -                               size = sizeof(FILE_ALL_INFO);
>                        memcpy((char *) pFindData,
>                               (char *) &pSMBr->hdr.Protocol +
> -                              data_offset, size);
> +                              data_offset, sizeof(FILE_ALL_INFO));
>                } else
>                    rc = -ENOMEM;
>        }
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 469c3dd..b44bfd5 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2629,7 +2629,7 @@ is_path_accessible(int xid, struct cifsTconInfo *tcon,
>                return -ENOMEM;
>
>        rc = CIFSSMBQPathInfo(xid, tcon, full_path, pfile_info,
> -                             0 /* not legacy */, cifs_sb->local_nls,
> +                             cifs_sb->local_nls,
>                              cifs_sb->mnt_cifs_flags &
>                                CIFS_MOUNT_MAP_SPECIAL_CHR);
>        kfree(pfile_info);
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 39869c3..2448ad1 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -615,7 +615,6 @@ int cifs_get_inode_info(struct inode **pinode,
>
>                /* could do find first instead but this returns more info */
>                rc = CIFSSMBQPathInfo(xid, pTcon, full_path, pfindData,
> -                             0 /* not legacy */,
>                              cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
>                                CIFS_MOUNT_MAP_SPECIAL_CHR);
>                /* BB optimize code so we do not make the above call
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Looks correct to me.

Re: [PATCH] cifs: remove "legacy" parm from CIFSSMBQPathInfo

[Steve French]

What is the code flow then on Windows9x, OS/2 or in the EOPNOSUPP
case?  Seems strange that we don't retry with legacy or in the case
where the dialect (or previous call) shows that we don't support the
infolevel.

On Thu, Oct 28, 2010 at 2:10 AM, Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
> as all the callers are setting it to 0 anyway.
>
> I stumbled upon this code when I was looking at the strange hardlink
> behavior reported in the mailing list. But it seems this patch was
> originally posted by Jeff Layton on Feb:
>  http://lists.samba.org/archive/linux-cifs-client/2010-February/005557.html
>
> I didn't see any discussions, ACK/NACK on this patch.
>
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
> ---
>  fs/cifs/cifsproto.h |    1 -
>  fs/cifs/cifssmb.c   |   22 +++-------------------
>  fs/cifs/connect.c   |    2 +-
>  fs/cifs/inode.c     |    1 -
>  4 files changed, 4 insertions(+), 22 deletions(-)
>
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index edb6d90..fa7e8d1 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -167,7 +167,6 @@ extern int CIFSSMBQFileInfo(const int xid, struct cifsTconInfo *tcon,
>  extern int CIFSSMBQPathInfo(const int xid, struct cifsTconInfo *tcon,
>                        const unsigned char *searchName,
>                        FILE_ALL_INFO *findData,
> -                       int legacy /* whether to use old info level */,
>                        const struct nls_table *nls_codepage, int remap);
>  extern int SMBQueryInformation(const int xid, struct cifsTconInfo *tcon,
>                        const unsigned char *searchName,
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index e98f1f3..392c01b 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -3356,7 +3356,6 @@ int
>  CIFSSMBQPathInfo(const int xid, struct cifsTconInfo *tcon,
>                 const unsigned char *searchName,
>                 FILE_ALL_INFO *pFindData,
> -                int legacy /* old style infolevel */,
>                 const struct nls_table *nls_codepage, int remap)
>  {
>  /* level 263 SMB_QUERY_FILE_ALL_INFO */
> @@ -3406,10 +3405,7 @@ QPathInfoRetry:
>        byte_count = params + 1 /* pad */ ;
>        pSMB->TotalParameterCount = cpu_to_le16(params);
>        pSMB->ParameterCount = pSMB->TotalParameterCount;
> -       if (legacy)
> -               pSMB->InformationLevel = cpu_to_le16(SMB_INFO_STANDARD);
> -       else
> -               pSMB->InformationLevel = cpu_to_le16(SMB_QUERY_FILE_ALL_INFO);
> +       pSMB->InformationLevel = cpu_to_le16(SMB_QUERY_FILE_ALL_INFO);
>        pSMB->Reserved4 = 0;
>        pSMB->hdr.smb_buf_length += byte_count;
>        pSMB->ByteCount = cpu_to_le16(byte_count);
> @@ -3423,26 +3419,14 @@ QPathInfoRetry:
>
>                if (rc) /* BB add auto retry on EOPNOTSUPP? */
>                        rc = -EIO;
> -               else if (!legacy && (pSMBr->ByteCount < 40))
> +               else if (pSMBr->ByteCount < 40)
>                        rc = -EIO;      /* bad smb */
> -               else if (legacy && (pSMBr->ByteCount < 24))
> -                       rc = -EIO;  /* 24 or 26 expected but we do not read
> -                                       last field */
>                else if (pFindData) {
> -                       int size;
>                        __u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset);
>
> -                       /* On legacy responses we do not read the last field,
> -                       EAsize, fortunately since it varies by subdialect and
> -                       also note it differs on Set vs. Get, ie two bytes or 4
> -                       bytes depending but we don't care here */
> -                       if (legacy)
> -                               size = sizeof(FILE_INFO_STANDARD);
> -                       else
> -                               size = sizeof(FILE_ALL_INFO);
>                        memcpy((char *) pFindData,
>                               (char *) &pSMBr->hdr.Protocol +
> -                              data_offset, size);
> +                              data_offset, sizeof(FILE_ALL_INFO));
>                } else
>                    rc = -ENOMEM;
>        }
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 469c3dd..b44bfd5 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2629,7 +2629,7 @@ is_path_accessible(int xid, struct cifsTconInfo *tcon,
>                return -ENOMEM;
>
>        rc = CIFSSMBQPathInfo(xid, tcon, full_path, pfile_info,
> -                             0 /* not legacy */, cifs_sb->local_nls,
> +                             cifs_sb->local_nls,
>                              cifs_sb->mnt_cifs_flags &
>                                CIFS_MOUNT_MAP_SPECIAL_CHR);
>        kfree(pfile_info);
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 39869c3..2448ad1 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -615,7 +615,6 @@ int cifs_get_inode_info(struct inode **pinode,
>
>                /* could do find first instead but this returns more info */
>                rc = CIFSSMBQPathInfo(xid, pTcon, full_path, pfindData,
> -                             0 /* not legacy */,
>                              cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
>                                CIFS_MOUNT_MAP_SPECIAL_CHR);
>                /* BB optimize code so we do not make the above call
>



-- 
Thanks,

Steve

Re: [PATCH] cifs: remove "legacy" parm from CIFSSMBQPathInfo

[Suresh Jayaraman]

On 10/28/2010 09:12 PM, Steve French wrote:
> What is the code flow then on Windows9x, OS/2 or in the EOPNOSUPP
> case?  Seems strange that we don't retry with legacy or in the case
> where the dialect (or previous call) shows that we don't support the
> infolevel.
> 

If CIFSSMBOpen fails, the callers seem to be fall back to legacy open using
SMBLegacyOpen().

cifs_open()

       ...
        if (tcon->ses->capabilities & CAP_NT_SMBS)
                rc = CIFSSMBOpen(xid, tcon, full_path, disposition,
                         desiredAccess, CREATE_NOT_DIR, &netfid, &oplock, buf,
                         cifs_sb->local_nls, cifs_sb->mnt_cifs_flags
                                 & CIFS_MOUNT_MAP_SPECIAL_CHR);
        else
                rc = -EIO; /* no NT SMB support fall into legacy open below */
                
        if (rc == -EIO) {
                /* Old server, try legacy style OpenX */
                rc = SMBLegacyOpen(xid, tcon, full_path, disposition,
                        desiredAccess, CREATE_NOT_DIR, &netfid, &oplock, buf,
                        cifs_sb->local_nls, cifs_sb->mnt_cifs_flags
                                & CIFS_MOUNT_MAP_SPECIAL_CHR);
        }



-- 
Suresh Jayaraman

Re: [PATCH] cifs: remove "legacy" parm from CIFSSMBQPathInfo

[Shirish Pargaonkar]

On Thu, Oct 28, 2010 at 11:19 AM, Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
> On 10/28/2010 09:12 PM, Steve French wrote:
>> What is the code flow then on Windows9x, OS/2 or in the EOPNOSUPP
>> case?  Seems strange that we don't retry with legacy or in the case
>> where the dialect (or previous call) shows that we don't support the
>> infolevel.
>>
>
> If CIFSSMBOpen fails, the callers seem to be fall back to legacy open using
> SMBLegacyOpen().
>
> cifs_open()
>
>       ...
>        if (tcon->ses->capabilities & CAP_NT_SMBS)
>                rc = CIFSSMBOpen(xid, tcon, full_path, disposition,
>                         desiredAccess, CREATE_NOT_DIR, &netfid, &oplock, buf,
>                         cifs_sb->local_nls, cifs_sb->mnt_cifs_flags
>                                 & CIFS_MOUNT_MAP_SPECIAL_CHR);
>        else
>                rc = -EIO; /* no NT SMB support fall into legacy open below */
>
>        if (rc == -EIO) {
>                /* Old server, try legacy style OpenX */
>                rc = SMBLegacyOpen(xid, tcon, full_path, disposition,
>                        desiredAccess, CREATE_NOT_DIR, &netfid, &oplock, buf,
>                        cifs_sb->local_nls, cifs_sb->mnt_cifs_flags
>                                & CIFS_MOUNT_MAP_SPECIAL_CHR);
>        }
>
>
>
> --
> Suresh Jayaraman
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

This is a general comment but should cifs client be using (server)
dialect specific set of smb commands
and those smb commands specific info levels instead of sending a
command and trying downlevel / legacy
if that command failed with "operation not supported"?

Re: [PATCH] cifs: remove "legacy" parm from CIFSSMBQPathInfo

[Steve French]

On Thu, Oct 28, 2010 at 11:41 AM, Shirish Pargaonkar
<shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, Oct 28, 2010 at 11:19 AM, Suresh Jayaraman <sjayaraman@suse.de> wrote:
>> On 10/28/2010 09:12 PM, Steve French wrote:
>>> What is the code flow then on Windows9x, OS/2 or in the EOPNOSUPP
>>> case?  Seems strange that we don't retry with legacy or in the case
>>> where the dialect (or previous call) shows that we don't support the
>>> infolevel.
>>>
>>
>> If CIFSSMBOpen fails, the callers seem to be fall back to legacy open using
>> SMBLegacyOpen().
>>
>> cifs_open()
>>
>>       ...
>>        if (tcon->ses->capabilities & CAP_NT_SMBS)
>>                rc = CIFSSMBOpen(xid, tcon, full_path, disposition,
>>                         desiredAccess, CREATE_NOT_DIR, &netfid, &oplock, buf,
>>                         cifs_sb->local_nls, cifs_sb->mnt_cifs_flags
>>                                 & CIFS_MOUNT_MAP_SPECIAL_CHR);
>>        else
>>                rc = -EIO; /* no NT SMB support fall into legacy open below */
>>
>>        if (rc == -EIO) {
>>                /* Old server, try legacy style OpenX */
>>                rc = SMBLegacyOpen(xid, tcon, full_path, disposition,
>>                        desiredAccess, CREATE_NOT_DIR, &netfid, &oplock, buf,
>>                        cifs_sb->local_nls, cifs_sb->mnt_cifs_flags
>>                                & CIFS_MOUNT_MAP_SPECIAL_CHR);
>>        }
>>
>>
>>
>> --
>> Suresh Jayaraman
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
> This is a general comment but should cifs client be using (server)
> dialect specific set of smb commands
> and those smb commands specific info levels instead of sending a
> command and trying downlevel / legacy
> if that command failed with "operation not supported"?

We can do the former for smb2, but not for cifs unfortunately.

For cifs there are lots of cases where a dialect doesn't support an
infolevel it should (or even viceversa).   Part of this difficulty
mapping levels to dialects is presumably due to lack of authoritative
documentation for cifs when some of the server developers were doing
initial cifs development years ago - even so there are cases where we
can make assumptions (based on feedback from 7 years of test events)
but it depends on the operation.

-- 
Thanks,

Steve

Re: [PATCH] cifs: remove "legacy" parm from CIFSSMBQPathInfo

[Suresh Jayaraman]

On 10/28/2010 11:17 PM, Steve French wrote:
> On Thu, Oct 28, 2010 at 11:41 AM, Shirish Pargaonkar
> <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Thu, Oct 28, 2010 at 11:19 AM, Suresh Jayaraman <sjayaraman@suse.de> wrote:
>>> On 10/28/2010 09:12 PM, Steve French wrote:
>>>> What is the code flow then on Windows9x, OS/2 or in the EOPNOSUPP
>>>> case? �Seems strange that we don't retry with legacy or in the case
>>>> where the dialect (or previous call) shows that we don't support the
>>>> infolevel.
>>>
>>> If CIFSSMBOpen fails, the callers seem to be fall back to legacy open using
>>> SMBLegacyOpen().
>>>
>>> cifs_open()
>>>
>>> � � � ...
>>> � � � �if (tcon->ses->capabilities & CAP_NT_SMBS)
>>> � � � � � � � �rc = CIFSSMBOpen(xid, tcon, full_path, disposition,
>>> � � � � � � � � � � � � desiredAccess, CREATE_NOT_DIR, &netfid, &oplock, buf,
>>> � � � � � � � � � � � � cifs_sb->local_nls, cifs_sb->mnt_cifs_flags
>>> � � � � � � � � � � � � � � � � & CIFS_MOUNT_MAP_SPECIAL_CHR);
>>> � � � �else
>>> � � � � � � � �rc = -EIO; /* no NT SMB support fall into legacy open below */
>>>
>>> � � � �if (rc == -EIO) {
>>> � � � � � � � �/* Old server, try legacy style OpenX */
>>> � � � � � � � �rc = SMBLegacyOpen(xid, tcon, full_path, disposition,
>>> � � � � � � � � � � � �desiredAccess, CREATE_NOT_DIR, &netfid, &oplock, buf,
>>> � � � � � � � � � � � �cifs_sb->local_nls, cifs_sb->mnt_cifs_flags
>>> � � � � � � � � � � � � � � � �& CIFS_MOUNT_MAP_SPECIAL_CHR);
>>> � � � �}
>>>
>> This is a general comment but should cifs client be using (server)
>> dialect specific set of smb commands
>> and those smb commands specific info levels instead of sending a
>> command and trying downlevel / legacy
>> if that command failed with "operation not supported"?
> 
> We can do the former for smb2, but not for cifs unfortunately.
> 
> For cifs there are lots of cases where a dialect doesn't support an
> infolevel it should (or even viceversa).

So that means this patch is applicable to CIFS as is, right?


-- 
Suresh Jayaraman