From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 6BA27430409 for ; Thu, 20 Aug 2020 17:25:41 +0300 (MSK) Date: Thu, 20 Aug 2020 17:25:38 +0300 From: "Alexander V. Tikhonov" Message-ID: <20200820142538.GA26887@hpalx> References: <344b8b406afa1464aae59f9be231ff78f053b996.1597817683.git.avtikhon@tarantool.org> <20200819223552.nplchepqb67tojxr@tkn_work_nb> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200819223552.nplchepqb67tojxr@tkn_work_nb> Subject: Re: [Tarantool-patches] [PATCH v6] vinyl: fix check vinyl_dir existence at bootstrap List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Turenko , Nikita Pettik , Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org 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. On Thu, Aug 20, 2020 at 01:35:52AM +0300, Alexander Turenko wrote: > 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 > > --- > > > > 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