ZABBIX BUGS AND ISSUES
  1. ZABBIX BUGS AND ISSUES
  2. ZBX-7856

Windows agent doesn't support symbolic links for log monitoring

    Details

      Description

      Windows agent doesn't support symbolic links for log[] and logrt[] key.

      agent uses _wstat64() to get log file information, but _wstat64() doesn't support symbolic links.
      http://msdn.microsoft.com/en-us/library/14h5k7ff.aspx

        Activity

        Hide
        Andris Zeila added a comment -

        Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-7856

        Show
        Andris Zeila added a comment - Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-7856
        Hide
        Aleksandrs Saveljevs added a comment - - edited

        (1) It is a bit unclear where "struct stat" should be used and where the new "zbx_stat_t" should be.

        For instance, __zbx_zabbix_log() uses "zbx_stat_t" for a call to stat(), but src/libs/zbxsysinfo/common/file.c uses it the other way round: "struct stat" for a call to zbx_stat().

        It seems that the stat()-ing interface should be made more intuitive.

        While at it, please see r43975.

        Andris Zeila It should be used at least in code that is used also on windows. But I missed the sysinfo/common/ directory.

        Andris Zeila RESOLVED in r43980

        Aleksandrs Saveljevs These are our current definitions for zbx_stat():

        #if defined(_WINDOWS)
        #ifdef _UNICODE
        #	define zbx_stat(path, buf)	__zbx_stat(path, buf)
        #else
        #	define zbx_stat(path, buf)	_stat64(path, buf)
        #endif
        #else
        #	define zbx_stat(path, buf)	stat(path, buf)
        #endif
        

        The first definition accepts "zbx_stat_t", the second "struct __stat64", the third "struct stat".

        Here are our current definitions for "zbx_stat_t":

        #if defined(_WINDOWS) && defined(_UNICODE)
        typedef struct __stat64	zbx_stat_t;
        int	__zbx_stat(const char *path, zbx_stat_t *buf);
        #else
        typedef struct stat	zbx_stat_t;
        #endif	/* _WINDOWS && _UNICODE */
        

        It seems that these definitions do not match the second case above, because "zbx_stat_t" will be "struct stat", but _stat64() expects "struct __stat64".

        Aleksandrs Saveljevs Also, as pointed above, __zbx_zabbix_log() uses "zbx_stat_t" for a call to stat(). For Windows and Unicode, "zbx_stat_t" will be "struct __stat64". Can stat() really be called on that structure? REOPENED.

        Andris Zeila Actually it would call _stat64 because of a define we had in zbxtypes (and now I understood why 'struct stat' worked with _wstat64 before). But it's really confusing, so I removed this defined and replaced all stat() calls used in Windows code with zbx_stat().

        Andris Zeila RESOLVED in r44022

        Aleksandrs Saveljevs Looks good, but please see 44027.

        Andris Zeila Thanks, CLOSED

        Show
        Aleksandrs Saveljevs added a comment - - edited (1) It is a bit unclear where "struct stat" should be used and where the new "zbx_stat_t" should be. For instance, __zbx_zabbix_log() uses "zbx_stat_t" for a call to stat(), but src/libs/zbxsysinfo/common/file.c uses it the other way round: "struct stat" for a call to zbx_stat(). It seems that the stat()-ing interface should be made more intuitive. While at it, please see r43975. Andris Zeila It should be used at least in code that is used also on windows. But I missed the sysinfo/common/ directory. Andris Zeila RESOLVED in r43980 Aleksandrs Saveljevs These are our current definitions for zbx_stat(): #if defined(_WINDOWS) #ifdef _UNICODE # define zbx_stat(path, buf) __zbx_stat(path, buf) #else # define zbx_stat(path, buf) _stat64(path, buf) #endif #else # define zbx_stat(path, buf) stat(path, buf) #endif The first definition accepts "zbx_stat_t", the second "struct __stat64", the third "struct stat". Here are our current definitions for "zbx_stat_t": #if defined(_WINDOWS) && defined(_UNICODE) typedef struct __stat64 zbx_stat_t; int __zbx_stat(const char *path, zbx_stat_t *buf); #else typedef struct stat zbx_stat_t; #endif /* _WINDOWS && _UNICODE */ It seems that these definitions do not match the second case above, because "zbx_stat_t" will be "struct stat", but _stat64() expects "struct __stat64". Aleksandrs Saveljevs Also, as pointed above, __zbx_zabbix_log() uses "zbx_stat_t" for a call to stat(). For Windows and Unicode, "zbx_stat_t" will be "struct __stat64". Can stat() really be called on that structure? REOPENED. Andris Zeila Actually it would call _stat64 because of a define we had in zbxtypes (and now I understood why 'struct stat' worked with _wstat64 before). But it's really confusing, so I removed this defined and replaced all stat() calls used in Windows code with zbx_stat(). Andris Zeila RESOLVED in r44022 Aleksandrs Saveljevs Looks good, but please see 44027. Andris Zeila Thanks, CLOSED
        Hide
        Aleksandrs Saveljevs added a comment - - edited

        (2) It might be that using _wstat64() is more efficient that opening a file, doing _fstat64() and closing the file.

        Since symlinks are probably not used very often, would it make sense to only use the _fstat64() sequence if _wstat64() fails?

        For instance, http://msdn.microsoft.com/en-us/library/14h5k7ff.aspx says "In these cases, _wstat will always report a file size of 0". Can we fall back to _fstat64() if _wstat64() reports a file size of 0?

        Andris Zeila Yes, that sounds like a good idea. RESOLVED in r44043

        Aleksandrs Saveljevs Looks good, but please see r44062. It replaces close() with _close(). RESOLVED.

        Aleksandrs Saveljevs Actually, it does not seem to be that simple. The new code (doing _wstat64() first, then falling back to _fstat64()) does not work when there is a symlinked directory in the path. In that case, _wstat64() returns -1, saying that there is "No such file or directory". See "C:\zabbix\symlinks\" directory for test cases. REOPENED.

        Andris Zeila It appears that directory was symlinked without /D switch. Could you please try it again?

        Aleksandrs Saveljevs Might be, the issue is not reproducible anymore. CLOSED.

        Show
        Aleksandrs Saveljevs added a comment - - edited (2) It might be that using _wstat64() is more efficient that opening a file, doing _fstat64() and closing the file. Since symlinks are probably not used very often, would it make sense to only use the _fstat64() sequence if _wstat64() fails? For instance, http://msdn.microsoft.com/en-us/library/14h5k7ff.aspx says "In these cases, _wstat will always report a file size of 0". Can we fall back to _fstat64() if _wstat64() reports a file size of 0? Andris Zeila Yes, that sounds like a good idea. RESOLVED in r44043 Aleksandrs Saveljevs Looks good, but please see r44062. It replaces close() with _close(). RESOLVED. Aleksandrs Saveljevs Actually, it does not seem to be that simple. The new code (doing _wstat64() first, then falling back to _fstat64()) does not work when there is a symlinked directory in the path. In that case, _wstat64() returns -1, saying that there is "No such file or directory". See "C:\zabbix\symlinks\" directory for test cases. REOPENED. Andris Zeila It appears that directory was symlinked without /D switch. Could you please try it again? Aleksandrs Saveljevs Might be, the issue is not reproducible anymore. CLOSED.
        Hide
        Aleksandrs Saveljevs added a comment - - edited

        (3) In src/libs/zbxsysinfo/linux/proc.c, there is a function open_proc_file():

        static FILE	*open_proc_file(const char *filename)
        {
        	zbx_stat_t	s;
        
        	if (0 != zbx_stat(filename, &s))
        		return NULL;
        
        	return fopen(filename, "r");
        }
        

        It might be that stat() is unnecessary here. Hence, open_proc_file() can be replaced with fopen().

        Andris Zeila Nobody had a good idea why it was there, so removed it.
        RESOLVED in r44033

        Aleksandrs Saveljevs The code did not compile. Fixed that in r44061. RESOLVED.

        Andris Zeila thanks, CLOSED

        Show
        Aleksandrs Saveljevs added a comment - - edited (3) In src/libs/zbxsysinfo/linux/proc.c, there is a function open_proc_file(): static FILE *open_proc_file( const char *filename) { zbx_stat_t s; if (0 != zbx_stat(filename, &s)) return NULL; return fopen(filename, "r" ); } It might be that stat() is unnecessary here. Hence, open_proc_file() can be replaced with fopen(). Andris Zeila Nobody had a good idea why it was there, so removed it. RESOLVED in r44033 Aleksandrs Saveljevs The code did not compile. Fixed that in r44061. RESOLVED. Andris Zeila thanks, CLOSED
        Hide
        Aleksandrs Saveljevs added a comment - - edited

        (4) Should "/D UNICODE" be removed from Windows makefiles or is it a system define?

        Andris Zeila UNICODE is required to use wide character version of Windows API, _UNICODE is required for tchar macros - TEXT(), _tcs functions. I understand that Windows defines UNICODE if _UNICODE is defined, so in theory we might omit it.

        Aleksandrs Saveljevs This seems to say that the safest way is to define both: http://www.winehq.org/pipermail/wine-devel/2002-January/003565.html . So let's leave as is. CLOSED.

        Show
        Aleksandrs Saveljevs added a comment - - edited (4) Should "/D UNICODE" be removed from Windows makefiles or is it a system define? Andris Zeila UNICODE is required to use wide character version of Windows API, _UNICODE is required for tchar macros - TEXT(), _tcs functions. I understand that Windows defines UNICODE if _UNICODE is defined, so in theory we might omit it. Aleksandrs Saveljevs This seems to say that the safest way is to define both: http://www.winehq.org/pipermail/wine-devel/2002-January/003565.html . So let's leave as is. CLOSED.
        Hide
        Aleksandrs Saveljevs added a comment - - edited

        (5) Please also see r44063. This fixes some warnings (unrelated to this task) on Windows.

        Andris Zeila Good, CLOSED

        Show
        Aleksandrs Saveljevs added a comment - - edited (5) Please also see r44063. This fixes some warnings (unrelated to this task) on Windows. Andris Zeila Good, CLOSED
        Hide
        Andris Zeila added a comment -

        Released in:
        pre-2.0.12rc1 r44211
        pre-2.2.4rc1 r44213
        pre-2.3.0 r44214

        Show
        Andris Zeila added a comment - Released in: pre-2.0.12rc1 r44211 pre-2.2.4rc1 r44213 pre-2.3.0 r44214
        Hide
        Aleksandrs Saveljevs added a comment - - edited

        (6) There is a call to _wstat() in src/libs/zbxconf/cfg.c in trunk that did not exist in 2.0 and was thus missed when merging from 2.0 into trunk.

        The problem with that call is that it cannot be replaced with the current version of zbx_stat() in a straightforward way, because current zbx_stat() fails on directories: _wstat64() is successful, "buf->st_size" is zero, then _wopen() fails.

        Andris Zeila RESOLVED in r44897

        Aleksandrs Saveljevs Looks almost good, except one initialization was missing. Please see r44898 before merging. RESOLVED.

        Andris Zeila Thanks, CLOSED

        Show
        Aleksandrs Saveljevs added a comment - - edited (6) There is a call to _wstat() in src/libs/zbxconf/cfg.c in trunk that did not exist in 2.0 and was thus missed when merging from 2.0 into trunk. The problem with that call is that it cannot be replaced with the current version of zbx_stat() in a straightforward way, because current zbx_stat() fails on directories: _wstat64() is successful, "buf->st_size" is zero, then _wopen() fails. Andris Zeila RESOLVED in r44897 Aleksandrs Saveljevs Looks almost good, except one initialization was missing. Please see r44898 before merging. RESOLVED. Andris Zeila Thanks, CLOSED
        Hide
        Alexander Vladishev added a comment - - edited

        (7) Cannot compile 2.0 zabbix_sender and zabbix_get.

        log.o : error LNK2019: unresolved external symbol ___zbx_stat referenced in function ___zbx_zabbix_log ..\..\..\bin\win32\zabbix_sender.exe : fatal error LNK1120: 1 unresolved externals
        NMAKE : fatal error U1077: '"C:\Program Files\Microsoft Visual Studio 9.0\VC\Bin\link.exe"' : return code '0x460'
        

        Andris Zeila Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-7856_2.0
        RESOLVED

        Alexander Vladishev CLOSED

        Andris Zeila Released in:
        pre-2.0.12rc1 r44887
        pre-2.2.4rc1 44888
        pre-2.3.0 r44889

        Show
        Alexander Vladishev added a comment - - edited (7) Cannot compile 2.0 zabbix_sender and zabbix_get. log.o : error LNK2019: unresolved external symbol ___zbx_stat referenced in function ___zbx_zabbix_log ..\..\..\bin\win32\zabbix_sender.exe : fatal error LNK1120: 1 unresolved externals NMAKE : fatal error U1077: '"C:\Program Files\Microsoft Visual Studio 9.0\VC\Bin\link.exe"' : return code '0x460' Andris Zeila Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-7856_2.0 RESOLVED Alexander Vladishev CLOSED Andris Zeila Released in: pre-2.0.12rc1 r44887 pre-2.2.4rc1 44888 pre-2.3.0 r44889
        Hide
        Andris Zeila added a comment -

        The (6) fix is released in:
        pre-2.3.0 r44923

        Show
        Andris Zeila added a comment - The (6) fix is released in: pre-2.3.0 r44923
        Hide
        Andris Zeila added a comment - - edited

        (8) zbx_stat() is used for directories (logrt[]) also in 2.0. So the (6) zbx_stat() related fix must be backported to 2.0

        Andris Zeila RESOLVED in r45103

        Aleksandrs Saveljevs Looks good. Works on symlinked directories, too. CLOSED.

        Show
        Andris Zeila added a comment - - edited (8) zbx_stat() is used for directories (logrt[]) also in 2.0. So the (6) zbx_stat() related fix must be backported to 2.0 Andris Zeila RESOLVED in r45103 Aleksandrs Saveljevs Looks good. Works on symlinked directories, too. CLOSED.
        Hide
        Andris Zeila added a comment -

        Released in:
        pre-2.0.12rc2 r45110
        pre-2.2.4rc1 r45111
        pre-2.3.0 r44923

        Show
        Andris Zeila added a comment - Released in: pre-2.0.12rc2 r45110 pre-2.2.4rc1 r45111 pre-2.3.0 r44923

          People

          • Assignee:
            Unassigned
            Reporter:
            Kodai Terashima
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: