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

[Alexander Turenko <alexander.turenko@tarantool.org>]

This description becomes much better!

On Wed, Aug 19, 2020 at 09:17:39AM +0300, Alexander V. Tikhonov wrote:
> 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 behaviour 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
> whould 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 than the check was wrong. The previous logic of the
> check was to catch the error ENOENT from inside the xdir_scan()
> function to handle the situation when vinyl_dir was not exist. In this
> way errno should be reseted before xdir_scan() call to give the ability
> for xdir_scan() to use return value without errno set and correctly
> handle errno from inside the xdir_scan().

'from inside' can be interpreted as 'to catch from inside', not as
'ENOENT from inside'. The same for 'handle errno for inside': 'to handle
from inside' or 'errno from inside'?

'to use return value without errno set' is unclear for me. Are not it is
intended to give the same idea as the previous 'errno should be reset
before xdir_scan()'? If so, it look redundant for me.

Typo: whould.

Typo: reseted -> reset (irregular verb).

> 
> After discussions found that there was alternative better solution to
> fix it. As mentioned above xdir_scan() function is not system call and
> can be changed inside it in any possible way. So check outside of this
> function on errno could be broken, because of the xdir_scan() changes.
> To avoid of it we must avoid of errno checks outside of the function.
> Better solution was 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@tarantool.org>
> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4562-suse-pack-full-ci
> Issue: https://github.com/tarantool/tarantool/issues/4594
> Issue: https://github.com/tarantool/tarantool/issues/4562