[PATCH] add pread operation to vfs layer
Stefan Metzmacher
metze at metzemix.de
Wed Dec 17 05:56:20 GMT 2003
James Peach wrote:
>On Tue, 16 Dec 2003 15:27:47 -0800
>"Cole, Timothy D." <t.cole at ngc.com> wrote:
>
>>I wrote:
>>
>>>Would it be feasible to cache the effective and actual file
>>>offsets for a
>>>file descriptor, to reduce the number of seeks needed for sequential
>>>pwrites/preads?
>>
>>Thinking about it, those offsets would likely have to be stored in the
>>files_struct -- that's okay provided an fd with a files_struct attached
>>can only ever be read/written/lsought via that single files_struct.
>>
>>Is that a safe assumption to make, though?
>
>
>This seems quite complicated and difficult to test. There is already
pos and
>position_information in files_struct, and adding a third player into this
>isn't going to help the clarity of this code.
>
>Another different option is to test for p* at runtime, which would allow
>3rd-party vfs modules to add their owm p* ops (perhaps implementing your
>caching idea). This patch (untested and cumulative wrt my p* one) hacks up
>the vfs to allow operatios to be missing ...
>
>--
>James Peach | jpeach at sgi.com | SGI Australian Software Group
>I don't speak for SGI.
>
>
>===========================================================================
>source/include/vfs_macros.h
>===========================================================================
>
>--- /usr/tmp/TmpDir.1294301-0/src/source/include/vfs_macros.h_1.5
Wed Dec 17 13:38:10 2003
>+++ source/include/vfs_macros.h Wed Dec 17 13:33:53 2003
>@@ -27,6 +27,12 @@
> (Fixes should go also into the vfs_opaque_* and vfs_next_* macros!)
> ********************************************************************/
>
>+extern int vfswrap_enosys(struct vfs_handle_struct *, struct
files_struct *, ...);
>+
>+/* Define as operation as not present if there is no opaque version
of it. */
>+#define SMB_VFS_OP_MISSING(conn, op) \
>+ (((void **)&(conn)->vfs_opaque.ops)[op] == (void
*)vfswrap_enosys)
>+
> /* Disk operations */
> #define SMB_VFS_CONNECT(conn, service, user)
((conn)->vfs.ops.connect((conn)->vfs.handles.connect, (conn), (service),
(user)))
> #define SMB_VFS_DISCONNECT(conn)
((conn)->vfs.ops.disconnect((conn)->vfs.handles.disconnect, (conn)))
>
>===========================================================================
>source/smbd/fileio.c
>===========================================================================
>
>--- /usr/tmp/TmpDir.1294301-0/src/source/smbd/fileio.c_1.5 Wed Dec
17 13:38:10 2003
>+++ source/smbd/fileio.c Wed Dec 17 13:36:00 2003
>@@ -95,22 +95,20 @@
>
> flush_write_cache(fsp, READ_FLUSH);
>
>-#if !defined(HAVE_PREAD) && !defined(HAVE_PREAD64)
>- if (seek_file(fsp,pos) == -1) {
>+ if (SMB_VFS_OP_MISSING(fsp->conn, SMB_VFS_OP_PREAD) &&
>+ seek_file(fsp,pos) == -1) {
> DEBUG(3,("read_file: Failed to seek to %.0f\n",(double)pos));
> return(ret);
> }
>-#endif
>
> if (n > 0) {
> #ifdef DMF_FIX
> int numretries = 3;
> tryagain:
>-#if defined(HAVE_PREAD) || defined(HAVE_PREAD64)
>- readret = SMB_VFS_PREAD(fsp,fsp->fd,data,n,pos);
>-#else
>- readret = SMB_VFS_READ(fsp,fsp->fd,data,n);
>-#endif
>+ readret = SMB_VFS_OP_MISSING(&fsp->conn, SMB_VFS_OP_PREAD) ?
>+ SMB_VFS_READ(fsp,fsp->fd,data,n) :
>+ SMB_VFS_PREAD(fsp,fsp->fd,data,n,pos);
>+
> if (readret == -1) {
> if ((errno == EAGAIN) && numretries) {
> DEBUG(3,("read_file EAGAIN retry in 10 seconds\n"));
>@@ -121,11 +119,10 @@
> return -1;
> }
> #else /* NO DMF fix. */
>-#if defined(HAVE_PREAD) || defined(HAVE_PREAD64)
>- readret = SMB_VFS_PREAD(fsp,fsp->fd,data,n,pos);
>-#else
>- readret = SMB_VFS_READ(fsp,fsp->fd,data,n);
>-#endif
>+ readret = SMB_VFS_OP_MISSING(fsp->conn, SMB_VFS_OP_PREAD) ?
>+ SMB_VFS_READ(fsp,fsp->fd,data,n) :
>+ SMB_VFS_PREAD(fsp,fsp->fd,data,n,pos);
>+
> if (readret == -1)
> return -1;
> #endif
>@@ -153,17 +150,17 @@
> {
> ssize_t ret;
>
>-#if defined(HAVE_PREAD) || defined(HAVE_PREAD64)
>- if (pos == -1)
>- ret = vfs_write_data(fsp, data, n);
>- else
>- ret = vfs_pwrite_data(fsp, data, n, pos);
>-#else
>- if ((pos != -1) && (seek_file(fsp,pos) == -1))
>- return -1;
>+ if (SMB_VFS_OP_MISSING(fsp->conn, SMB_VFS_OP_PWRITE)) {
>+ if ((pos != -1) && (seek_file(fsp,pos) == -1))
>+ return -1;
>
>- ret = vfs_write_data(fsp,data,n);
>-#endif
>+ ret = vfs_write_data(fsp,data,n);
>+ } else {
>+ if (pos == -1)
>+ ret = vfs_write_data(fsp, data, n);
>+ else
>+ ret = vfs_pwrite_data(fsp, data, n, pos);
>+ }
>
> DEBUG(10,("real_write_file (%s): pos = %.0f, size = %lu, returned
%ld\n",
> fsp->fsp_name, (double)pos, (unsigned long)n, (long)ret ));
>
>===========================================================================
>source/smbd/vfs-wrap.c
>===========================================================================
>
>--- /usr/tmp/TmpDir.1294301-0/src/source/smbd/vfs-wrap.c_1.6 Wed
Dec 17 13:38:10 2003
>+++ source/smbd/vfs-wrap.c Wed Dec 17 12:56:16 2003
>@@ -562,6 +562,12 @@
> return 0;
> }
>
>+int vfswrap_enosys(struct vfs_handle_struct * handle, struct
files_struct * fsp, ...)
>+{
>+ errno = ENOSYS;
>+ return -1;
>+}
>+
> int vfswrap_ftruncate(vfs_handle_struct *handle, files_struct *fsp,
int fd, SMB_OFF_T len)
> {
> int result = -1;
>
>===========================================================================
>source/smbd/vfs.c
>===========================================================================
>
>--- /usr/tmp/TmpDir.1294301-0/src/source/smbd/vfs.c_1.10 Wed Dec 17
13:38:10 2003
>+++ source/smbd/vfs.c Wed Dec 17 13:36:56 2003
>@@ -46,6 +46,7 @@
> very important. They must be in the same order as defined in
> vfs.h. Change at your own peril. */
>
>+
> static struct vfs_ops default_vfs = {
>
> {
>@@ -71,9 +72,17 @@
> vfswrap_open,
> vfswrap_close,
> vfswrap_read,
>+#if defined(HAVE_PREAD) || defined(HAVE_PREAD64)
> vfswrap_pread,
>+#else
>+ vfswrap_enosys,
>+#endif
> vfswrap_write,
>+#if defined(HAVE_PWRITE) || defined(HAVE_PWRITE64)
> vfswrap_pwrite,
>+#else
>+ vfswrap_enosys,
>+#endif
> vfswrap_lseek,
> vfswrap_sendfile,
> vfswrap_rename,
>
I like the first patch more...
It's ugly to test if a function pointer has a specefic value.
What is when e.g. a database opaque vfs module overloads the pread fn
with
example_pread(vfs_handle_struct *handle,files_struct *f, ...)
{
errno =ENOSYS;
return -1;
}
(This is what such a module should do)
So just try SMB_VFS_PREAD and if it fails with ENOSYS try read.
So no if def's are needed
if (((readret=SMB_VFS_PREAD(fsp,fsp->fd,data,n,pos))==-1)&&errno==ENOSYS)
readret = SMB_VFS_READ(fsp,fsp->fd,data,n);
}
--
metze
-------------------------------------------
Stefan (metze) Metzmacher <metze at metzemix.de>
More information about the samba-technical
mailing list