[ZBX-10548] Cannot compile agent on Solaris9 Created: 2016 Mar 18 Updated: 2017 May 30 Resolved: 2016 Apr 26 |
|
Status: | Closed |
Project: | ZABBIX BUGS AND ISSUES |
Component/s: | Agent (G) |
Affects Version/s: | 3.0.1 |
Fix Version/s: | 3.0.3rc1, 3.2.0alpha1 |
Type: | Incident report | Priority: | Trivial |
Reporter: | Kodai Terashima | Assignee: | Unassigned |
Resolution: | Fixed | Votes: | 0 |
Labels: | compilation, solaris | ||
Remaining Estimate: | Not Specified | ||
Time Spent: | Not Specified | ||
Original Estimate: | Not Specified |
Description |
3.0.1 agent cannot compile on Solaris 9 with the error below: proc.c: In function `PROC_CPU_UTIL': proc.c:683: `GLOBAL_ZONEID' undeclared (first use in this function) proc.c:683: (Each undeclared identifier is reported only once proc.c:683: for each function it appears in.) |
Comments |
Comment by Andris Mednis [ 2016 Mar 18 ] |
Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-10548 . |
Comment by Aleksandrs Saveljevs [ 2016 Mar 21 ] |
(1) There is the following diff: - SET_MSG_RESULT(result, zbx_strdup(NULL, "Unsupported sixth parameter.")); + SET_MSG_RESULT(result, zbx_strdup(NULL, "Unsupported sixth parameter, agent compiled on a Solaris" + " version without zones support.")); Would "zone support" be more correct? andris RESOLVED in r59072. asaveljevs CLOSED |
Comment by Aleksandrs Saveljevs [ 2016 Mar 21 ] |
(2) The code now also looks like this: /* proc.cpu.util[<procname>,<username>,(user|system),<cmdline>,(avg1|avg5|avg15),(current|all)] */ #ifndef HAVE_ZONE_H /* Case of Solaris 9 and earlier. Zones are supported in Solaris 10 and later.*/ if (6 == request->nparam) { SET_MSG_RESULT(result, zbx_strdup(NULL, "Unsupported sixth parameter, agent compiled on a Solaris" " version without zones support.")); return SYSINFO_RET_FAIL; } #endif It means that proc.cpu.util[<5 params>] and proc.cpu.util[<5 params>,current] (where "current" is the default) are no longer identical and it is not possible to use items with 6 parameters in a template for all versions of Solaris. Since zones are not supported in Solaris 9 and earlier, maybe it would be more convenient for users to treat it as having just one zone, where "current" equals "all" (same as Solaris 10 installation without zones)? andris Situation is more complicated if the agent has been compiled on Solaris 9 (zones are not supported) and copied to Solaris 10/11 (zones are supported but agent is not aware of them). It is necessary to prevent the agent from reporting wrong data in this situation. Solution: if the 6th parameter is "all", then Solaris9-compiled agent reports proc.cpu.util[] on Solaris10/11, if the 6th parameter is not specified or "current", then proc.cpu.util[] is NOTSUPPORTED. andris RESOLVED in a new development branch svn://svn.zabbix.com/branches/dev/ZBX-10548-2 r59130 . asaveljevs The error message currently says: /* But now this agent is running on a system with zone support. This agent cannot limit */ /* results to only current zone. */ SET_MSG_RESULT(result, zbx_strdup(NULL, "Unsupported sixth parameter, agent compiled on a" " Solaris version without zone support.")); I have a desire to paraphrase it somehow. First, it may be confusing that it refers to the sixth parameter, especially when a user has specified only one. Second, a user may think that the sixth parameter is not supported at all, whereas we actually want to tell him that "all" should be used here. andris RESOLVED in 59317. asaveljevs The new version says: SET_MSG_RESULT(result, zbx_strdup(NULL, "The sixth parameter value \"current\" cannot be used" " with agent compiled on a Solaris version without zone support. Consider using" " \"all\" or install agent with Solaris zone support.")); The message seems to imply that "current" cannot be used with agents compiled on Solaris 9 and earlier at all, but it is not true - only when they are running on Solaris 10 and above. How about the following? SET_MSG_RESULT(result, zbx_strdup(NULL, "The sixth parameter value \"current\" cannot be used" " with agent running on a Solaris version with zone support, but compiled on" " a Solaris version without zone support. Consider using \"all\" or install" " agent with Solaris zone support.")); asaveljevs You seem to have liked it, so committed that in r59398 and r59402 together with other fixes (e.g., header inclusion). Please take a look. andris Thanks! Reviewed, accepted. CLOSED |
Comment by Aleksandrs Saveljevs [ 2016 Apr 05 ] |
(3) According to "man sysinfo" at https://docs.oracle.com/cd/E23823_01/html/816-5167/sysinfo-2.html , function sysinfo() returns "long", but we try to store it in an "int". asaveljevs RESOLVED in r59274. asaveljevs As discovered by andris, sysinfo() returns "int" on Solaris 11. It should be investigated whether copying a binary from Solaris 10 to Solaris 11 will work as expected. andris r59274 reviewed, accepted. andris sysinfo() replaced with uname(). RESOLVED in r59310. asaveljevs Even though manual pages on older versions of Solaris list sysinfo() as returning "long", the actual /usr/include/sys/systeminfo.h headers have it returning "int". So it might be that they simply fixed the man page. If so, we can return to sysinfo(), although that is optional. andris It was decided to use uname(). CLOSED |
Comment by Aleksandrs Saveljevs [ 2016 Apr 05 ] |
(4) This is just to document that the same information can be obtained using uname() system call (https://docs.oracle.com/cd/E23823_01/html/816-5167/uname-2.html). The benefit of using sysinfo() is that it only returns the information we needed, whereas uname() returns more information. However, performance does not matter in this particular case, because we only need to obtain the information once, although potentially this function may be used in other places, too, and it might make sense to move zbx_solaris_version_get() to a more generic Solaris file. The downside of using sysinfo() is that it introduces API that we have not used until now. asaveljevs Changing this is totally optional. andris It makes sense to replace sysinfo() with uname(). Unlike sysinfo(), the uname() returns result of the same type ('int') on Solaris versions 8, 9. 10, 11. Less surprises if a binary is compiled on older Solaris version and copied to a newer one. andris sysinfo() replaced with uname(). RESOLVED in r59310. asaveljevs Looks good. Even though "./configure" outputs "solaris2.8" and "solaris2.11" (the "host_os" variable set by AC_CANONICAL_HOST, see https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Canonicalizing.html) as detected OS, I have not found any indication that "uname" returns 2.x values. See also https://en.wikipedia.org/wiki/Solaris_(operating_system)#Version_history . CLOSED. |
Comment by Aleksandrs Saveljevs [ 2016 Apr 05 ] |
(5) There is the following warning when compiling on Solaris 8: procstat.c: In function `zbx_procstat_get_util': procstat.c:1104: warning: integer constant is too large for "long" type asaveljevs RESOLVED in r59292. andris Reviewed, accepted. CLOSED. |
Comment by Aleksandrs Saveljevs [ 2016 Apr 05 ] |
(6) Something else to think about, maybe in a separate ZBX: if an agent is running in a zone and the user specifies "all", do we wish the item to become not supported? An agent running in a zone cannot return data for all zones. andris Specifying "all" (zones ) does not lead to NOTSUPPORTED item. NOTSUPPORTED is produced if agent has been compiled on Solaris without zones support but is running on a newer Solaris with zones support and user has specified "current". In this situation the agent running in the global zone cannot restrict results to "current" zone. So result would effectively be for "all" zones which is incorrect. If agent is unaware of zones it cannot determine is it running in global or non-global zone. Hence, NOTSUPPORTED in this case. andris It was decided to make a separate ZBX: Solaris agent in non-global zone should return NOTSUPPORTED if zone parameter is "all". |
Comment by Andris Mednis [ 2016 Apr 21 ] |
Fixed in versions:
|
Comment by Andris Mednis [ 2016 Apr 21 ] |
Documented in sasha CLOSED |
Comment by Aleksandrs Saveljevs [ 2016 Apr 25 ] |
(7) There was the following remark in (3):
If so, then the following source code comment is not precise: /* Initially sysinfo(SI_RELEASE, ...) was considered for getting Solaris release. Its result type */ /* depends from version: on Solaris 8,9,10 sysinfo() returns 'long', on Solaris 11 it returns 'int'. */ /* Therefore uname() was chosen for getting Solaris release. */ andris RESOLVED (comment deleted) in versions:
asaveljevs CLOSED |