[Tarantool-patches] [PATCH v6] vinyl: fix check vinyl_dir existence at bootstrap

Alexander Turenko alexander.turenko at tarantool.org
Thu Aug 20 22:48:06 MSK 2020


Please, resend the patchset to Nikita Pettik and Aleksandr Lyapunov (because it
is related to vinyl).

Or extract this particular path to its own branch and send it as a singleton
patch.

WBR, Alexander Turenko.

On Thu, Aug 20, 2020 at 05:25:38PM +0300, Alexander V. Tikhonov wrote:
> Hi Alexander, thanks a lot for your help. As we discussed the commit
> message I've made all the changes in it and commited to branch.

I think it is good enough. LGTM.

Cited below for the history.

> commit e18b7e8f56787a9099429ff9e6fc6c1185d74723
> Author: Alexander V. Tikhonov <avtikhon at tarantool.org>
> Date:   Fri Aug 14 11:18:25 2020 +0300
> 
>     vinyl: fix check vinyl_dir existence at bootstrap
>     
>     During implementation of openSUSE build with testing got failed test
>     box-tap/cfg.test.lua. Found that when memtx_dir didn't exist and
>     vinyl_dir existed and also errno was set to ENOENT, box configuration
>     succeeded, but it shouldn't. Reason of this wrong behavior was that
>     not all of the failure paths in xdir_scan() set errno, but the caller
>     assumed it.
>     
>     Debugging the issue found that after xdir_scan() there was incorrect
>     check for errno when it returned negative values. xdir_scan() is not
>     system call and negative return value from it doesn't mean that errno
>     would be set too. Found that in situations when errno was left from
>     previous commands before xdir_scan() and xdir_scan() returned negative
>     value by itself it produced the wrong check.
>     
>     The previous failed logic of the check was to catch the error ENOENT
>     which set in the xdir_scan() function to handle the situation when
>     vinyl_dir was not exist. It failed, because checking ENOENT outside
>     the xdir_scan() function, we had to be sure that ENOENT had come from
>     xdir_scan() function call indeed and not from any other functions
>     before. To be sure in it possible fix could be reset errno before
>     xdir_scan() call, because errno could be passed from any other function
>     before call to xdir_scan().
>     
>     As mentioned above xdir_scan() function is not system call and can be
>     changed in any possible way and it can return any result value without
>     need to setup errno. So check outside of this function on errno could
>     be broken.
>     
>     To avoid of it we must avoid of errno checks outside of the function.
>     Better solution is to use the flag in xdir_scan(), to check if the
>     directory should exist. So errno check was removed and instead of it
>     the check for vinyl_dir existence using flag added.
>     
>     Closes #4594
>     Needed for #4562
>     
>     Co-authored-by: Alexander Turenko <alexander.turenko at tarantool.org>


More information about the Tarantool-patches mailing list