Bug 13425 - VFS modules that implement pread/pwrite must also implement pread_send/pwrite_send.
VFS modules that implement pread/pwrite must also implement pread_send/pwrite...
Status: RESOLVED FIXED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules
unspecified
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2018-05-08 23:14 UTC by Jeremy Allison
Modified: 2018-05-17 11:36 UTC (History)
1 user (show)

See Also:


Attachments
git-am fix for 4.8.next (5.04 KB, patch)
2018-05-09 21:04 UTC, Jeremy Allison
ddiss: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2018-05-08 23:14:53 UTC
As 'aio read size = 1' and 'aio write size = 1' by default in 4.8.0 and above, all VFS modules that implement pread/pwrite *must* also implement at least a fallback pread_send/pwrite_send that returns an error.

The reason is that with aio turned on as the the default smbd code tries async pread_send/pwrite_send first, and only if that fails falls back to the synchronous pread/pwrite path.

As VFS modules stack on top of vfs_default by default, any module that implements pread/pwrite without also implementing pread_send/pwrite_send will be calling the default vfs pread_send/pwrite_send instead of the custom versions.

For most modules this doesn't matter, but for modules like CEPH which should never call down to the default POSIX layer this can cause error.

Fix to follow.
Comment 1 Jeremy Allison 2018-05-09 21:04:21 UTC
Created attachment 14191 [details]
git-am fix for 4.8.next

Back-ported from master.
Comment 2 Jeremy Allison 2018-05-09 21:05:46 UTC
Checked carefully, and the only other modules that implement pread/pwrite without pread_send/pwrite_send are non-stackable (readahead etc. that call directly into underlying POSIX functions as does vfs_default). So I think this is the only patch needed to fix this on all VFS modules in 4.8.x.
Comment 3 Jeremy Allison 2018-05-09 21:48:34 UTC
Re-assigning to Karolin for inclusion in 4.8.next.
Comment 4 David Disseldorp 2018-05-11 17:30:18 UTC
@Jeremy: I'd like to pull in the same patch for (maintenance mode) 4.7, despite the differing "aio read/write size" defaults.
I've done sanity testing locally - are you okay with that?
Comment 5 Jeremy Allison 2018-05-11 17:32:53 UTC
Sure, I'm happy with that. If anyone sets aio read/write size to anything other than 0 then it's also needed for 4.7.x.
Comment 6 Karolin Seeger 2018-05-14 07:56:23 UTC
Pushed to autobuild-v4-{8,7}-test.
Comment 7 Karolin Seeger 2018-05-17 11:36:07 UTC
(In reply to Karolin Seeger from comment #6)
Pushed to both branches.
Closing out bug report.

Thanks!