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>

diff --git a/source3/include/libsmb_internal.h b/source3/include/libsmb_internal.h
index fcdb51301540ee274efefb39de58f17f4d12a63b..cf51f34fcca773b4ae19b4daddb3ea14d9b67895 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 fe5703521eb7b64a399b2aabb3655ea63d43b176..29a6a0fbbec580ddf06e32428bfdbfa0571106a4 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;
        }
@@ -436,11 +410,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;
 
@@ -468,36 +437,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);
@@ -519,14 +459,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;
@@ -546,41 +479,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);
@@ -816,13 +721,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;
@@ -850,41 +749,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);
@@ -914,16 +784,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;
@@ -943,36 +804,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;