s3: libsmbclient: Re-resolving targetcli on every read/write/lseek/ftruncate/close...

Cache targetcli on file open in the SMBCFILE struct.

Bug 11295 - Excessive cli_resolve_path() usage can slow down transmission.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11295

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Michael Adam <obnox@samba.org>
(cherry picked from commit 9f57244bbd1ffa203a1f50bb289789628c4a3f66)

Autobuild-User(v4-1-test): Karolin Seeger <kseeger@samba.org>
Autobuild-Date(v4-1-test): Wed Jun 10 01:20:32 CEST 2015 on sn-devel-104

diff --git a/source3/include/libsmb_internal.h b/source3/include/libsmb_internal.h
index 65fad99892b67f11f5c581a88994a168c40ca58f..38cd5a1257b68e0d96b7103f6f3b505f8ca74f18 100644 (file)
--- a/source3/include/libsmb_internal.h
+++ b/source3/include/libsmb_internal.h
@@ -100,6 +100,11 @@ struct smbc_dir_list {
  */
 struct _SMBCFILE {
        int cli_fd;
+       /*
+        * cache of cli_state we opened cli_fd on.
+        * Due to DFS can be a subsidiary connection to srv->cli
+        */
+       struct cli_state *targetcli;
        char *fname;
        off_t offset;
        struct _SMBCSRV *srv;

diff --git a/source3/libsmb/libsmb_file.c b/source3/libsmb/libsmb_file.c
index 8fb7a2e67eca800763d92fc32a14575d977935df..c8beafce65edb1b4a302b841484df05b0030cad0 100644 (file)
--- a/source3/libsmb/libsmb_file.c
+++ b/source3/libsmb/libsmb_file.c
@@ -144,6 +144,14 @@ SMBC_open_ctx(SMBCCTX *context,
                file->srv     = srv;
                file->offset  = 0;
                file->file    = True;
+               /*
+                * targetcli is either equal to srv->cli or
+                * is a subsidiary DFS connection. Either way
+                * file->cli_fd belongs to it so we must cache
+                * it for read/write/close, not re-resolve each time.
+                * Re-resolving is both slow and incorrect.
+                */
+               file->targetcli = targetcli;
 
                DLIST_ADD(context->internal->files, file);
 
@@ -228,11 +236,6 @@ SMBC_read_ctx(SMBCCTX *context,
               size_t count)
 {
        size_t ret;
-       char *server = NULL, *share = NULL, *user = NULL, *password = NULL;
-       char *path = NULL;
-       char *targetpath = NULL;
-       struct cli_state *targetcli = NULL;
-       uint16_t port = 0;
        TALLOC_CTX *frame = talloc_stackframe();
        NTSTATUS status;
 
@@ -271,39 +274,10 @@ SMBC_read_ctx(SMBCCTX *context,
                return -1;
        }
 
-       /*d_printf(">>>read: parsing %s\n", file->fname);*/
-       if (SMBC_parse_path(frame,
-                            context,
-                            file->fname,
-                            NULL,
-                            &server,
-                            &port,
-                            &share,
-                            &path,
-                            &user,
-                            &password,
-                            NULL)) {
-                errno = EINVAL;
-               TALLOC_FREE(frame);
-                return -1;
-        }
-
-       /*d_printf(">>>read: resolving %s\n", path);*/
-       status = cli_resolve_path(frame, "", context->internal->auth_info,
-                                 file->srv->cli, path,
-                                 &targetcli, &targetpath);
-       if (!NT_STATUS_IS_OK(status)) {
-               d_printf("Could not resolve %s\n", path);
-                errno = ENOENT;
-               TALLOC_FREE(frame);
-               return -1;
-       }
-       /*d_printf(">>>fstat: resolved path as %s\n", targetpath);*/
-
-       status = cli_read(targetcli, file->cli_fd, (char *)buf, offset,
+       status = cli_read(file->targetcli, file->cli_fd, (char *)buf, offset,
                          count, &ret);
        if (!NT_STATUS_IS_OK(status)) {
-               errno = SMBC_errno(context, targetcli);
+               errno = SMBC_errno(context, file->targetcli);
                TALLOC_FREE(frame);
                return -1;
        }
@@ -327,11 +301,6 @@ SMBC_write_ctx(SMBCCTX *context,
                size_t count)
 {
         off_t offset;
-       char *server = NULL, *share = NULL, *user = NULL, *password = NULL;
-       char *path = NULL;
-       char *targetpath = NULL;
-       struct cli_state *targetcli = NULL;
-       uint16_t port = 0;
        TALLOC_CTX *frame = talloc_stackframe();
        NTSTATUS status;
 
@@ -359,36 +328,7 @@ SMBC_write_ctx(SMBCCTX *context,
 
         offset = file->offset; /* See "offset" comment in SMBC_read_ctx() */
 
-       /*d_printf(">>>write: parsing %s\n", file->fname);*/
-       if (SMBC_parse_path(frame,
-                            context,
-                            file->fname,
-                            NULL,
-                            &server,
-                            &port,
-                            &share,
-                            &path,
-                            &user,
-                            &password,
-                            NULL)) {
-                errno = EINVAL;
-               TALLOC_FREE(frame);
-                return -1;
-        }
-
-       /*d_printf(">>>write: resolving %s\n", path);*/
-       status = cli_resolve_path(frame, "", context->internal->auth_info,
-                                 file->srv->cli, path,
-                                 &targetcli, &targetpath);
-       if (!NT_STATUS_IS_OK(status)) {
-               d_printf("Could not resolve %s\n", path);
-                errno = ENOENT;
-               TALLOC_FREE(frame);
-               return -1;
-       }
-       /*d_printf(">>>write: resolved path as %s\n", targetpath);*/
-
-       status = cli_writeall(targetcli, file->cli_fd,
+       status = cli_writeall(file->targetcli, file->cli_fd,
                              0, (const uint8_t *)buf, offset, count, NULL);
        if (!NT_STATUS_IS_OK(status)) {
                errno = map_errno_from_nt_status(status);
@@ -410,14 +350,7 @@ int
 SMBC_close_ctx(SMBCCTX *context,
                SMBCFILE *file)
 {
-        SMBCSRV *srv;
-       char *server = NULL, *share = NULL, *user = NULL, *password = NULL;
-       char *path = NULL;
-       char *targetpath = NULL;
-       uint16_t port = 0;
-       struct cli_state *targetcli = NULL;
        TALLOC_CTX *frame = talloc_stackframe();
-       NTSTATUS status;
 
        if (!context || !context->internal->initialized) {
                errno = EINVAL;
@@ -437,41 +370,13 @@ SMBC_close_ctx(SMBCCTX *context,
                return smbc_getFunctionClosedir(context)(context, file);
        }
 
-       /*d_printf(">>>close: parsing %s\n", file->fname);*/
-       if (SMBC_parse_path(frame,
-                            context,
-                            file->fname,
-                            NULL,
-                            &server,
-                            &port,
-                            &share,
-                            &path,
-                            &user,
-                            &password,
-                            NULL)) {
-                errno = EINVAL;
-               TALLOC_FREE(frame);
-                return -1;
-        }
-
-       /*d_printf(">>>close: resolving %s\n", path);*/
-       status = cli_resolve_path(frame, "", context->internal->auth_info,
-                                 file->srv->cli, path,
-                                 &targetcli, &targetpath);
-       if (!NT_STATUS_IS_OK(status)) {
-               d_printf("Could not resolve %s\n", path);
-                errno = ENOENT;
-               TALLOC_FREE(frame);
-               return -1;
-       }
-       /*d_printf(">>>close: resolved path as %s\n", targetpath);*/
-
-       if (!NT_STATUS_IS_OK(cli_close(targetcli, file->cli_fd))) {
+       if (!NT_STATUS_IS_OK(cli_close(file->targetcli, file->cli_fd))) {
+               SMBCSRV *srv;
                DEBUG(3, ("cli_close failed on %s. purging server.\n",
                          file->fname));
                /* Deallocate slot and remove the server
                 * from the server cache if unused */
-               errno = SMBC_errno(context, targetcli);
+               errno = SMBC_errno(context, file->targetcli);
                srv = file->srv;
                DLIST_REMOVE(context->internal->files, file);
                SAFE_FREE(file->fname);
@@ -707,13 +612,7 @@ SMBC_lseek_ctx(SMBCCTX *context,
                int whence)
 {
        off_t size;
-       char *server = NULL, *share = NULL, *user = NULL, *password = NULL;
-       char *path = NULL;
-       char *targetpath = NULL;
-       struct cli_state *targetcli = NULL;
-       uint16_t port = 0;
        TALLOC_CTX *frame = talloc_stackframe();
-       NTSTATUS status;
 
        if (!context || !context->internal->initialized) {
                errno = EINVAL;
@@ -741,41 +640,12 @@ SMBC_lseek_ctx(SMBCCTX *context,
                file->offset += offset;
                break;
        case SEEK_END:
-               /*d_printf(">>>lseek: parsing %s\n", file->fname);*/
-               if (SMBC_parse_path(frame,
-                                    context,
-                                    file->fname,
-                                    NULL,
-                                    &server,
-                                    &port,
-                                    &share,
-                                    &path,
-                                    &user,
-                                    &password,
-                                    NULL)) {
-                       errno = EINVAL;
-                       TALLOC_FREE(frame);
-                       return -1;
-               }
-
-               /*d_printf(">>>lseek: resolving %s\n", path);*/
-               status = cli_resolve_path(
-                       frame, "", context->internal->auth_info,
-                       file->srv->cli, path, &targetcli, &targetpath);
-               if (!NT_STATUS_IS_OK(status)) {
-                       d_printf("Could not resolve %s\n", path);
-                        errno = ENOENT;
-                       TALLOC_FREE(frame);
-                       return -1;
-               }
-
-               /*d_printf(">>>lseek: resolved path as %s\n", targetpath);*/
                if (!NT_STATUS_IS_OK(cli_qfileinfo_basic(
-                                            targetcli, file->cli_fd, NULL,
+                                            file->targetcli, file->cli_fd, NULL,
                                             &size, NULL, NULL, NULL, NULL,
                                             NULL))) {
                         off_t b_size = size;
-                       if (!NT_STATUS_IS_OK(cli_getattrE(targetcli, file->cli_fd,
+                       if (!NT_STATUS_IS_OK(cli_getattrE(file->targetcli, file->cli_fd,
                                           NULL, &b_size, NULL, NULL, NULL))) {
                                 errno = EINVAL;
                                 TALLOC_FREE(frame);
@@ -805,16 +675,7 @@ SMBC_ftruncate_ctx(SMBCCTX *context,
                    off_t length)
 {
        off_t size = length;
-       char *server = NULL;
-       char *share = NULL;
-       char *user = NULL;
-       char *password = NULL;
-       char *path = NULL;
-        char *targetpath = NULL;
-       uint16_t port = 0;
-       struct cli_state *targetcli = NULL;
        TALLOC_CTX *frame = talloc_stackframe();
-       NTSTATUS status;
 
        if (!context || !context->internal->initialized) {
                errno = EINVAL;
@@ -834,36 +695,7 @@ SMBC_ftruncate_ctx(SMBCCTX *context,
                return -1;
        }
 
-       /*d_printf(">>>fstat: parsing %s\n", file->fname);*/
-       if (SMBC_parse_path(frame,
-                            context,
-                            file->fname,
-                            NULL,
-                            &server,
-                            &port,
-                            &share,
-                            &path,
-                            &user,
-                            &password,
-                            NULL)) {
-                errno = EINVAL;
-               TALLOC_FREE(frame);
-                return -1;
-        }
-
-       /*d_printf(">>>fstat: resolving %s\n", path);*/
-       status = cli_resolve_path(frame, "", context->internal->auth_info,
-                                 file->srv->cli, path,
-                                 &targetcli, &targetpath);
-       if (!NT_STATUS_IS_OK(status)) {
-               d_printf("Could not resolve %s\n", path);
-                errno = ENOENT;
-               TALLOC_FREE(frame);
-               return -1;
-       }
-       /*d_printf(">>>fstat: resolved path as %s\n", targetpath);*/
-
-        if (!NT_STATUS_IS_OK(cli_ftruncate(targetcli, file->cli_fd, (uint64_t)size))) {
+        if (!NT_STATUS_IS_OK(cli_ftruncate(file->targetcli, file->cli_fd, (uint64_t)size))) {
                 errno = EINVAL;
                 TALLOC_FREE(frame);
                 return -1;