Re: (ITS#7682) LMDB: [PATCH] mdb_env_copy should retry open() if O_DIRECT fails

[sog@msg.com.mx]

On 09/05/2013 06:33 PM, Howard Chu wrote:
> sog@msg.com.mx wrote:
>> Full_Name: Salvador Ortiz
>> Version: 24
>> OS: Linux
>> URL: 
>> ftp://ftp.msg.com.mx/pub/varios/0001-In-mdb_env_copy-retry-open-if-O_DIRECT-fails.patch
>> Submission from: (NULL) (187.162.45.111)
>>
>>
>> If the OS defines O_DIRECT mdb_env_copy unconditionally uses it, but 
>> according
>> to open(2): "Some  file  systems  may  not implement the flag and 
>> open() will
>> fail with EINVAL if it is used."
>>
>> In this cases mdb_env_copy should retry without the flag.
>>
>> The attached patch implements that.
>>
>>
> Thanks, applied.
>

Thank you Howard, but I found an unexpected problem with the patch as 
commited.
If the call fails because O_DIRECT, in a "tmpfs" in my Fedora 19 '/tmp', 
the file is left created anyway! so the retry fails because O_EXCL|O_CREAT.
I consider this undocumented behavior for "open", buggy or at least ugly.
(A simple test program in ftp://ftp.msg.com.mx/pub/varios/o_direct.c)

I propose the following:
========
--- a/liblmdb/mdb.c    2013-09-05 17:31:01.498090281 -0500
+++ b/liblmdb/mdb.c    2013-09-13 14:45:00.918717292 -0500
@@ -4250,9 +4250,16 @@
      /* The OS supports O_DIRECT, try with it */
      newfd = open(lpath, O_WRONLY|O_CREAT|O_EXCL|O_DIRECT, 0666);
      /* But open can fail if O_DIRECT isn't supported by the file system
-     * so retry without the flag
+     * so retry without the flag.
+     *
+     * Sept 13 2013.
+     * On a tmpfs, at least, an open with O_DIRECT fails,
+     * BUT the file is created anyway!!
+     * Fortunately seems that other errors are reported before EINVAL
+     * So, we need to remove it before retry open.
       */
      if (newfd == INVALID_HANDLE_VALUE && ErrCode() == EINVAL)
+        (void)unlink(lpath)  ,
  #endif
      newfd = open(lpath, O_WRONLY|O_CREAT|O_EXCL, 0666);
  #endif
=========

What do you think?

Regards.