Bug 10982 - fallocate() returned values on failure
fallocate() returned values on failure
Status: RESOLVED FIXED
Product: Samba 4.0
Classification: Unclassified
Component: File services
4.0.5
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-12-04 06:28 UTC by Jones Syue
Modified: 2014-12-20 15:53 UTC (History)
4 users (show)

See Also:


Attachments
1st round patch (1.47 KB, application/mbox)
2014-12-04 06:28 UTC, Jones Syue
no flags Details
Proposed patchset for master. (7.96 KB, patch)
2014-12-04 19:53 UTC, Jeremy Allison
no flags Details
git-am patch for master. (8.33 KB, patch)
2014-12-05 23:47 UTC, Jeremy Allison
no flags Details
Cherry-pick of patches that went into master for 4.2.0 (9.56 KB, patch)
2014-12-08 02:57 UTC, Jeremy Allison
ddiss: review+
Details
Back-port of master fix for 4.1.next. (8.79 KB, patch)
2014-12-08 03:31 UTC, Jeremy Allison
ddiss: review+
Details
Back-port of master fix for 4.0.next. (7.46 KB, patch)
2014-12-08 04:14 UTC, Jeremy Allison
ddiss: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jones Syue 2014-12-04 06:28:01 UTC
Created attachment 10480 [details]
1st round patch

Hello list,

There is samba-4.0.5 in the linux box,
and set 'strict allocate = yes' to go through fallocate api.

Found if fallocate() returns -1 with errno = ENOSPC,
cannot catch by error handling as followings:

    ret = SMB_VFS_FALLOCATE(fsp, VFS_FALLOCATE_EXTEND_SIZE,
                            offset, num_to_write);
    if (ret == ENOSPC) {
        errno = ENOSPC;
        ret = -1;
        goto out;
    }

Per man pages said,
posix_fallocate() returns an error number on failure,
fallocate() returns -1 on failure.

Patch as attached could handle -1 returned from fallocate() on failure,
please help review and any suggestions are appreciated,
thanks.
Comment 1 Jeremy Allison 2014-12-04 19:53:54 UTC
Created attachment 10485 [details]
Proposed patchset for master.

Here's what I think should work - please confirm !
Comment 2 Jones Syue 2014-12-05 06:21:37 UTC
Hello Jeremy,

Yep this patch works for me!

And one more thing,
returns -1 between contend_level2_oplock_{begin,end} pairs 
looks a bit concern to me,
perhaps move them after contend_level2_oplocks_end() ?

     contend_level2_oplocks_begin(fsp, LEVEL2_CONTEND_ALLOC_GROW);

     if (lp_strict_allocate(SNUM(fsp->conn))) {
         /* See if we have a syscall that will allocate beyond
            end-of-file without changing EOF. */
         ret = SMB_VFS_FALLOCATE(fsp, VFS_FALLOCATE_KEEP_SIZE, 0, len);
     } else {
         ret = 0;
     }

     contend_level2_oplocks_end(fsp, LEVEL2_CONTEND_ALLOC_GROW);

+    if (ret == -1 && errno == ENOSPC) {
+        return -1;
+    }
     if (ret == 0) {
         /* We changed the allocation size on disk, but not
            EOF - exactly as required. We're done ! */
         return 0;
     }
Comment 3 Jeremy Allison 2014-12-05 23:47:41 UTC
Created attachment 10496 [details]
git-am patch for master.
Comment 4 David Disseldorp 2014-12-07 19:36:28 UTC
(In reply to Jeremy Allison from comment #3)

As mentioned via mail:

- sys_posix_fallocate() may still return an errno value
- smb_time_audit_fallocate() should save errno before calling
  clock_gettime()

Otherwise looks good.
Comment 5 Jeremy Allison 2014-12-08 02:57:19 UTC
Created attachment 10499 [details]
Cherry-pick of patches that went into master for 4.2.0
Comment 6 Jeremy Allison 2014-12-08 03:31:46 UTC
Created attachment 10500 [details]
Back-port of master fix for 4.1.next.
Comment 7 Jeremy Allison 2014-12-08 04:14:58 UTC
Created attachment 10501 [details]
Back-port of master fix for 4.0.next.
Comment 8 David Disseldorp 2014-12-08 07:46:59 UTC
Ready for maintenance branches. @Karo, please merge.
Comment 9 Karolin Seeger 2014-12-08 09:16:08 UTC
Pushed to autobuild-v4-[0|1|2]-test.
Comment 10 Karolin Seeger 2014-12-10 20:15:48 UTC
(In reply to Karolin Seeger from comment #9)
Pushed to v4-2-test and v4-1-test.
autobuild-v4-0-test failed, re-trying
Comment 11 Karolin Seeger 2014-12-20 15:53:04 UTC
(In reply to Karolin Seeger from comment #10)
Pushed.

Closing out bug report.

Thanks!