8202116: (fc) FileChannel.map should ensure mapped region is backed by disk space

[Thomas Stüfe]

Hi Alan,

not a full review.

About defining SEEK_HOLE:

+#ifndef SEEK_HOLE
+#define SEEK_HOLE 4
+#endif

I always flinch a bit when I see defines like this. Are you sure this
won't clash on libc variants which do not define SEEK_HOLE and where 4
is relegated to something else? I know this is Linux only, but there
are other libc variants than glibc, for instance musl.

How about this instead:

#ifdef SEEK_HOLE
#define HAVE_SEEK_HOLE 1
#endif

JNIEXPORT jlong JNICALL
Java_sun_nio_ch_FileDispatcherImpl_seekHole0(JNIEnv *env, jclass clazz,
                                             jobject fdo, jlong offset)
{
#if defined(HAVE_SEEK_HOLE)
    off64_t result = lseek64(fdval(env, fdo), offset, SEEK_HOLE);
    if (result == -1 && errno == EINVAL) {
        return IOS_UNSUPPORTED;
    } else {
        return handle(env, (jlong)result, "lseek64 failed");
    }
#else
    return IOS_UNSUPPORTED;
#endif
}

This would have the added benefit to work for all OSes which define
SEEK_HOLE (does it not come from Solaris?).

Best Regards, Thomas



On Mon, Apr 23, 2018 at 9:32 PM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> This is a follow-up to the "Changes to RandomAccessFile in jdk9" thread and
> the change in JDK 9 (and 8u162) to RandomAccessFile.setLengthforce to force
> disk space to be allocated for otherwise sparse files.
>
> The patch here is essentially a re-visit of JDK-8168628.
> RandomAccessFile.setLength is reverted so that it doesn't allocate the disk
> space. Instead, FileChannel.map is changed to allocate the disk space for
> the case that the region to be mapped overlaps with a hole in a sparse file.
> Testing the changes is awkward as the conditions for JDK-8168628 require
> filing up the file system to prevent space being allocated. Brian created an
> @ignore-ed test for JDK-8168628 so that is useful to re-create the
> conditions. In addition, several of the existing tests exercise this code as
> they (unknowingly) create spare files.
>
> The webrev with the changes is here:
>    http://cr.openjdk.java.net/~alanb/8202116/webrev/
>
> -Alan