From 22b283b21069afcc367e04d0e4c1cf69eb64fbeb Mon Sep 17 00:00:00 2001 From: Zdenek Styblik Date: Wed, 9 Jan 2013 11:17:42 +0000 Subject: [PATCH] ID: 3598203 - 'mc getsysinfo|setsysinfo' needs a bit of re-work ID: 3597782 - 'lib/ipmi_mc' - sysinfo_param() has two consecutive returns ID: 3597781 - 'mc getsysinfo|setsysinfo' help has typos Commit changes/fixes issues described in tickets #3598203, #3597782 and #3597781. Typos in help text of 'getsysinfo'/'setsysinfo' got fixed and function printf_sysinfo_usage() has been created. Consecutive return in function sysinfo_param() has been removed. Some code has been re-worked: * ipmi_getsysinfo() -> ipmi_mc_getsysinfo(), * ipmi_setsysinfo() -> ipmi_mc_setsysinfo(); * ipmi_sysinfo_main() extended for 'is_set' parameter to signal whether it's a get/set operation * return codes changed * code styling clean-up * a bit of code clean-up in ipmi_sysinfo_main(), ipmi_mc_getsysinfo(), ipmi_mc_setsysinfo() --- ipmitool/include/ipmitool/ipmi_mc.h | 4 +- ipmitool/lib/ipmi_delloem.c | 53 +++--- ipmitool/lib/ipmi_mc.c | 243 ++++++++++++++-------------- 3 files changed, 160 insertions(+), 140 deletions(-) diff --git a/ipmitool/include/ipmitool/ipmi_mc.h b/ipmitool/include/ipmitool/ipmi_mc.h index f499fb3..5546a3a 100644 --- a/ipmitool/include/ipmitool/ipmi_mc.h +++ b/ipmitool/include/ipmitool/ipmi_mc.h @@ -163,8 +163,8 @@ struct ipm_get_watchdog_rsp { #define IPMI_SYSINFO_DELL_OS_VERSION 0xe4 #define IPMI_SYSINFO_DELL_URL 0xde -int ipmi_getsysinfo(struct ipmi_intf * intf, int param, int block, int set, +int ipmi_mc_getsysinfo(struct ipmi_intf * intf, int param, int block, int set, int len, void *buffer); -int ipmi_setsysinfo(struct ipmi_intf * intf, int len, void *buffer); +int ipmi_mc_setsysinfo(struct ipmi_intf * intf, int len, void *buffer); #endif /*IPMI_MC_H */ diff --git a/ipmitool/lib/ipmi_delloem.c b/ipmitool/lib/ipmi_delloem.c index 5e99a85..891b8c6 100644 --- a/ipmitool/lib/ipmi_delloem.c +++ b/ipmitool/lib/ipmi_delloem.c @@ -670,9 +670,8 @@ ipmi_lcd_get_platform_model_name (struct ipmi_intf * intf, { int bytes_to_copy; - rc = ipmi_getsysinfo(intf, field_type, ii, 0, - sizeof(lcdstringblock), - &lcdstringblock); + rc = ipmi_mc_getsysinfo(intf, field_type, ii, 0, + sizeof(lcdstringblock), &lcdstringblock); if (rc < 0) { lprintf(LOG_ERR, " Error getting platform model name"); } else if (rc > 0) { @@ -726,7 +725,8 @@ ipmi_idracvalidator_command (struct ipmi_intf * intf) int rc; uint8_t data[11]; - rc = ipmi_getsysinfo(intf, IPMI_DELL_IDRAC_VALIDATOR, 2, 0, sizeof(data), data); + rc = ipmi_mc_getsysinfo(intf, IPMI_DELL_IDRAC_VALIDATOR, 2, 0, sizeof(data), + data); if (rc < 0) { /*lprintf(LOG_ERR, " Error getting IMC type"); */ return -1; @@ -774,7 +774,8 @@ ipmi_lcd_get_configure_command_wh (struct ipmi_intf * intf) uint8_t data[4]; int rc; - rc = ipmi_getsysinfo(intf, IPMI_DELL_LCD_CONFIG_SELECTOR, 0, 0, sizeof(lcd_mode), &lcd_mode); + rc = ipmi_mc_getsysinfo(intf, IPMI_DELL_LCD_CONFIG_SELECTOR, 0, 0, + sizeof(lcd_mode), &lcd_mode); if (rc < 0) { lprintf(LOG_ERR, " Error getting LCD configuration"); return -1; @@ -808,7 +809,8 @@ ipmi_lcd_get_configure_command (struct ipmi_intf * intf, uint8_t data[4]; int rc; - rc = ipmi_getsysinfo(intf, IPMI_DELL_LCD_CONFIG_SELECTOR, 0, 0, sizeof(data), data); + rc = ipmi_mc_getsysinfo(intf, IPMI_DELL_LCD_CONFIG_SELECTOR, 0, 0, + sizeof(data), data); if (rc < 0) { lprintf(LOG_ERR, " Error getting LCD configuration"); @@ -854,7 +856,7 @@ ipmi_lcd_set_configure_command (struct ipmi_intf * intf, int command) data[0] = IPMI_DELL_LCD_CONFIG_SELECTOR; data[1] = command; /* command - custom, default, none */ - rc = ipmi_setsysinfo(intf, 2, data); + rc = ipmi_mc_setsysinfo(intf, 2, data); if (rc < 0) { lprintf(LOG_ERR, " Error setting LCD configuration"); @@ -949,7 +951,7 @@ ipmi_lcd_set_configure_command_wh (struct ipmi_intf * intf, data[11]=lcd_mode.error_display; } - rc = ipmi_setsysinfo(intf, 13, data); + rc = ipmi_mc_setsysinfo(intf, 13, data); if (rc < 0) { lprintf(LOG_ERR, " Error setting LCD configuration"); @@ -995,7 +997,8 @@ ipmi_lcd_get_single_line_text (struct ipmi_intf * intf, char* lcdstring, uint8_t for (ii = 0; ii < 4; ii++) { int bytes_to_copy; - rc = ipmi_getsysinfo(intf, IPMI_DELL_LCD_STRING_SELECTOR, ii, 0, sizeof(lcdstringblock), &lcdstringblock); + rc = ipmi_mc_getsysinfo(intf, IPMI_DELL_LCD_STRING_SELECTOR, ii, 0, + sizeof(lcdstringblock), &lcdstringblock); if (rc < 0) { lprintf(LOG_ERR, " Error getting text data"); return -1; @@ -1080,7 +1083,8 @@ ipmi_lcd_get_info_wh(struct ipmi_intf * intf) else if (lcd_mode.lcdmode == IPMI_DELL_LCD_CONFIG_USER_DEFINED) { printf(" Setting: User defined\n"); - rc = ipmi_getsysinfo(intf, IPMI_DELL_LCD_GET_CAPS_SELECTOR, 0, 0, sizeof(lcd_caps), &lcd_caps); + rc = ipmi_mc_getsysinfo(intf, IPMI_DELL_LCD_GET_CAPS_SELECTOR, 0, 0, + sizeof(lcd_caps), &lcd_caps); if (rc < 0) { lprintf(LOG_ERR, " Error getting LCD capabilities."); @@ -1201,7 +1205,8 @@ static int ipmi_lcd_get_info(struct ipmi_intf * intf) { printf(" Setting: custom\n"); - rc = ipmi_getsysinfo(intf, IPMI_DELL_LCD_GET_CAPS_SELECTOR, 0, 0, sizeof(lcd_caps), &lcd_caps); + rc = ipmi_mc_getsysinfo(intf, IPMI_DELL_LCD_GET_CAPS_SELECTOR, 0, 0, + sizeof(lcd_caps), &lcd_caps); if (rc < 0) { lprintf(LOG_ERR, " Error getting LCD capabilities."); @@ -1248,7 +1253,8 @@ ipmi_lcd_get_status_val(struct ipmi_intf * intf, LCD_STATUS* lcdstatus) { int rc; - rc = ipmi_getsysinfo(intf, IPMI_DELL_LCD_STATUS_SELECTOR, 0, 0, sizeof(*lcdstatus), lcdstatus); + rc = ipmi_mc_getsysinfo(intf, IPMI_DELL_LCD_STATUS_SELECTOR, 0, 0, + sizeof(*lcdstatus), lcdstatus); if (rc < 0) { lprintf(LOG_ERR, " Error getting LCD Status"); @@ -1297,7 +1303,7 @@ static void CheckLCDSupport(struct ipmi_intf * intf) int rc; LcdSupported = 0; - rc = ipmi_getsysinfo(intf, IPMI_DELL_LCD_STATUS_SELECTOR, 0, 0, 0, NULL); + rc = ipmi_mc_getsysinfo(intf, IPMI_DELL_LCD_STATUS_SELECTOR, 0, 0, 0, NULL); if (rc == 0) { LcdSupported = 1; } @@ -1525,7 +1531,7 @@ ipmi_lcd_set_single_line_text (struct ipmi_intf * intf, char * text) memcpy (data+2, text+bytes_stored, size_of_copy); bytes_stored += size_of_copy; } - rc = ipmi_setsysinfo(intf, 18, data); + rc = ipmi_mc_setsysinfo(intf, 18, data); if (rc < 0) { lprintf(LOG_ERR, " Error setting text data"); rc = -1; @@ -1559,7 +1565,8 @@ ipmi_lcd_set_text(struct ipmi_intf * intf, char * text, int line_number) int rc = 0; IPMI_DELL_LCD_CAPS lcd_caps; - rc = ipmi_getsysinfo(intf, IPMI_DELL_LCD_GET_CAPS_SELECTOR, 0, 0, sizeof(lcd_caps), &lcd_caps); + rc = ipmi_mc_getsysinfo(intf, IPMI_DELL_LCD_GET_CAPS_SELECTOR, 0, 0, + sizeof(lcd_caps), &lcd_caps); if (rc < 0) { lprintf(LOG_ERR, " Error getting LCD capabilities"); @@ -3796,7 +3803,7 @@ static int ipmi_get_avgpower_consmpt_history(struct ipmi_intf* intf,IPMI_AVGPOWE int rc; uint8_t *rdata; - rc = ipmi_getsysinfo(intf, 0xeb, 0, 0, sizeof(*pavgpower), pavgpower); + rc = ipmi_mc_getsysinfo(intf, 0xeb, 0, 0, sizeof(*pavgpower), pavgpower); if (rc < 0) { lprintf(LOG_ERR, " Error getting average power consumption history data .\n"); @@ -3852,7 +3859,8 @@ static int ipmi_get_peakpower_consmpt_history(struct ipmi_intf* intf,IPMI_POWER_ uint8_t *rdata; int rc; - rc = ipmi_getsysinfo(intf, 0xEC, 0, 0, sizeof(*pstPeakpower), pstPeakpower); + rc = ipmi_mc_getsysinfo(intf, 0xec, 0, 0, sizeof(*pstPeakpower), + pstPeakpower); if (rc < 0) { lprintf(LOG_ERR, " Error getting peak power consumption history data .\n"); @@ -3916,7 +3924,8 @@ static int ipmi_get_minpower_consmpt_history(struct ipmi_intf* intf,IPMI_POWER_C uint8_t *rdata; int rc; - rc = ipmi_getsysinfo(intf, 0xED, 0, 0, sizeof(*pstMinpower), pstMinpower); + rc = ipmi_mc_getsysinfo(intf, 0xed, 0, 0, sizeof(*pstMinpower), + pstMinpower); if (rc < 0) { lprintf(LOG_ERR, " Error getting peak power consumption history data .\n"); @@ -4135,7 +4144,8 @@ static int ipmi_get_power_cap(struct ipmi_intf* intf,IPMI_POWER_CAP* ipmipowerca uint8_t *rdata; int rc; - rc = ipmi_getsysinfo(intf, IPMI_DELL_POWER_CAP, 0, 0, sizeof(*ipmipowercap), ipmipowercap); + rc = ipmi_mc_getsysinfo(intf, IPMI_DELL_POWER_CAP, 0, 0, + sizeof(*ipmipowercap), ipmipowercap); if (rc < 0) { lprintf(LOG_ERR, " Error getting power cap .\n"); return -1; @@ -4248,7 +4258,8 @@ static int ipmi_set_power_cap(struct ipmi_intf* intf,int unit,int val ) return -1; } - rc = ipmi_getsysinfo(intf, IPMI_DELL_POWER_CAP, 0, 0, sizeof(ipmipowercap), &ipmipowercap); + rc = ipmi_mc_getsysinfo(intf, IPMI_DELL_POWER_CAP, 0, 0, + sizeof(ipmipowercap), &ipmipowercap); if (rc < 0) { lprintf(LOG_ERR, " Error getting power cap .\n"); @@ -4340,7 +4351,7 @@ static int ipmi_set_power_cap(struct ipmi_intf* intf,int unit,int val ) return -1; } - rc = ipmi_setsysinfo(intf, 13, data); + rc = ipmi_mc_setsysinfo(intf, 13, data); if (rc < 0) { lprintf(LOG_ERR, " Error setting power cap"); diff --git a/ipmitool/lib/ipmi_mc.c b/ipmitool/lib/ipmi_mc.c index 4feb0ce..f845f8a 100644 --- a/ipmitool/lib/ipmi_mc.c +++ b/ipmitool/lib/ipmi_mc.c @@ -45,7 +45,9 @@ extern int verbose; -static int ipmi_sysinfo_main(struct ipmi_intf *intf, int argc, char ** argv); +static int ipmi_sysinfo_main(struct ipmi_intf *intf, int argc, char ** argv, + int is_set); +static void printf_sysinfo_usage(int full_help); /* ipmi_mc_reset - attempt to reset an MC * @@ -190,19 +192,40 @@ printf_mc_usage(void) for (bf = mc_enables_bf; bf->name != NULL; bf++) { lprintf(LOG_NOTICE, " %-20s %s", bf->name, bf->desc); } - lprintf(LOG_NOTICE, " getsysinfo argument"); - lprintf(LOG_NOTICE, " setsysinfo argument string"); - lprintf(LOG_NOTICE, " Valid arguments are:"); - lprintf(LOG_NOTICE, - " primary_os_name Primary operating system name"); - lprintf(LOG_NOTICE, " os_name Operating system name"); - lprintf(LOG_NOTICE, - " system_name System Name of server (vendor dependent"); - lprintf(LOG_NOTICE, - " delloem_os_version Running version of operating system"); - lprintf(LOG_NOTICE, " delloem_url Url of bmc webserver"); + printf_sysinfo_usage(0); } +static void +printf_sysinfo_usage(int full_help) +{ + if (full_help != 0) + lprintf(LOG_NOTICE, "usage:"); + + lprintf(LOG_NOTICE, " getsysinfo "); + + if (full_help != 0) { + lprintf(LOG_NOTICE, + " Retrieves system info from BMC for given argument"); + } + + lprintf(LOG_NOTICE, " setsysinfo "); + + if (full_help != 0) { + lprintf(LOG_NOTICE, + " Stores system info string for given argument to BMC"); + lprintf(LOG_NOTICE, ""); + lprintf(LOG_NOTICE, " Valid arguments are:"); + } + lprintf(LOG_NOTICE, + " primary_os_name Primary operating system name"); + lprintf(LOG_NOTICE, " os_name Operating system name"); + lprintf(LOG_NOTICE, + " system_name System Name of server(vendor dependent)"); + lprintf(LOG_NOTICE, + " delloem_os_version Running version of operating system"); + lprintf(LOG_NOTICE, " delloem_url URL of BMC webserver"); + lprintf(LOG_NOTICE, ""); +} static void print_watchdog_usage(void) @@ -846,9 +869,11 @@ ipmi_mc_main(struct ipmi_intf * intf, int argc, char ** argv) rc = (-1); } } - else if (strncmp(argv[0], "getsysinfo", 10) == 0 - || strncmp(argv[0], "setsysinfo", 10) == 0) { - rc = ipmi_sysinfo_main(intf, argc, argv); + else if (strncmp(argv[0], "getsysinfo", 10) == 0) { + rc = ipmi_sysinfo_main(intf, argc, argv, 0); + } + else if (strncmp(argv[0], "setsysinfo", 10) == 0) { + rc = ipmi_sysinfo_main(intf, argc, argv, 1); } else { lprintf(LOG_ERR, "Invalid mc/bmc command: %s", argv[0]); @@ -858,75 +883,59 @@ ipmi_mc_main(struct ipmi_intf * intf, int argc, char ** argv) return rc; } - +/* + * sysinfo_param() - function converts sysinfo param to int + * + * @str - user input string + * @maxset - ? + * + * returns (-1) on error + * returns > 0 on success + */ static int -sysinfo_param(struct ipmi_intf *intf, const char *str, int *maxset) +sysinfo_param(const char *str, int *maxset) { + if (!str || !maxset) + return (-1); + *maxset = 4; if (!strcmp(str, "system_name")) return IPMI_SYSINFO_HOSTNAME; - if (!strcmp(str, "primary_os_name")) + else if (!strcmp(str, "primary_os_name")) return IPMI_SYSINFO_PRIMARY_OS_NAME; - if (!strcmp(str, "os_name")) + else if (!strcmp(str, "os_name")) return IPMI_SYSINFO_OS_NAME; - - if (!strcmp(str, "delloem_os_version")) { - *maxset = 4; + else if (!strcmp(str, "delloem_os_version")) return IPMI_SYSINFO_DELL_OS_VERSION; - } - if (!strcmp(str, "delloem_url")) { + else if (!strcmp(str, "delloem_url")) { *maxset = 2; return IPMI_SYSINFO_DELL_URL; } - return strtoul(str, 0, 0); - return -1; + + return (-1); } -static void -ipmi_sysinfo_usage() -{ - lprintf(LOG_NOTICE, "usage:"); - lprintf(LOG_NOTICE, " getsysinfo argument"); - lprintf(LOG_NOTICE, " Retrieves system info from bmc for given argument"); - lprintf(LOG_NOTICE, " setsysinfo argument string"); - lprintf(LOG_NOTICE, - " Stores system info string for given argument to bmc"); - lprintf(LOG_NOTICE, ""); - lprintf(LOG_NOTICE, " Valid arguments are:"); - lprintf(LOG_NOTICE, - " primary_os_name Primary operating system name"); - lprintf(LOG_NOTICE, " os_name Operating system name"); - lprintf(LOG_NOTICE, - " system_name System Name of server (vendor dependent"); - lprintf(LOG_NOTICE, - " delloem_os_version Running version of operating system"); - lprintf(LOG_NOTICE, " delloem_url Url of bmc webserver"); - lprintf(LOG_NOTICE, ""); -} - -/***************************************************************** - * Function Name: ipmi_getsysinfo +/* + * ipmi_mc_getsysinfo() - function processes the IPMI Get System Info command * - * Description: This function processes the IPMI Get System Info command - * Input: intf - ipmi interface - * param - Parameter # (0xC0..0xFF = OEM) - * block/set - Block/Set number of parameter - * len - Length of buffer - * buffer - Pointer to buffer - * Output: + * @intf - ipmi interface + * @param - parameter eg. 0xC0..0xFF = OEM + * @block - number of block parameters + * @set - number of set parameters + * @len - length of buffer + * @buffer - pointer to buffer * - * Return: return code 0 - success - * -1 - failure - * other = IPMI ccode - * - ******************************************************************/ + * returns (-1) on failure + * returns 0 on success + * returns > 0 IPMI code + */ int -ipmi_getsysinfo(struct ipmi_intf * intf, int param, int block, int set, +ipmi_mc_getsysinfo(struct ipmi_intf * intf, int param, int block, int set, int len, void *buffer) { uint8_t data[4]; struct ipmi_rs *rsp = NULL; - struct ipmi_rq req={0}; + struct ipmi_rq req = {0}; memset(buffer, 0, len); memset(data, 0, 4); @@ -953,37 +962,34 @@ ipmi_getsysinfo(struct ipmi_intf * intf, int param, int block, int set, * u8 data0[14] */ rsp = intf->sendrecv(intf, &req); - if (rsp != NULL) { - if (rsp->ccode == 0) { - if (len > rsp->data_len) - len = rsp->data_len; - if (len && buffer) - memcpy(buffer, rsp->data, len); - } - return rsp->ccode; + if (rsp == NULL) + return (-1); + + if (rsp->ccode == 0) { + if (len > rsp->data_len) + len = rsp->data_len; + if (len && buffer) + memcpy(buffer, rsp->data, len); } - return -1; + return rsp->ccode; } -/***************************************************************** - * Function Name: ipmi_setsysinfo +/* + * ipmi_mc_setsysinfo() - function processes the IPMI Set System Info command * - * Description: This function processes the IPMI Set System Info command - * Input: intf - ipmi interface - * len - Length of buffer - * buffer - Pointer to buffer - * Output: + * @intf - ipmi interface + * @len - length of buffer + * @buffer - pointer to buffer * - * Return: return code 0 - success - * -1 - failure - * other = IPMI ccode - * - ******************************************************************/ + * returns (-1) on failure + * returns 0 on success + * returns > 0 IPMI code + */ int -ipmi_setsysinfo(struct ipmi_intf * intf, int len, void *buffer) +ipmi_mc_setsysinfo(struct ipmi_intf * intf, int len, void *buffer) { struct ipmi_rs *rsp = NULL; - struct ipmi_rq req={0}; + struct ipmi_rq req = {0}; req.msg.netfn = IPMI_NETFN_APP; req.msg.lun = 0; @@ -1005,41 +1011,39 @@ ipmi_setsysinfo(struct ipmi_intf * intf, int len, void *buffer) } static int -ipmi_sysinfo_main(struct ipmi_intf *intf, int argc, char ** argv) +ipmi_sysinfo_main(struct ipmi_intf *intf, int argc, char ** argv, int is_set) { - int param, isset; char *str; unsigned char infostr[256]; unsigned char paramdata[18]; - int pos, set, rc, maxset, len; + int len, maxset, param, pos, rc, set; - /* Is this a setsysinfo or getsysinfo */ - isset = !strncmp(argv[0], "setsysinfo\0",11); - - if (argc == 1 || strcmp(argv[1], "help") == 0 - || argc < (isset ? 3 : 2)) { - ipmi_sysinfo_usage(); + if (argc == 2 && strcmp(argv[1], "help") == 0) { + printf_sysinfo_usage(1); return 0; } - - memset(infostr, 0, sizeof(infostr)); + else if (argc < 2 || (is_set == 1 && argc < 3)) { + lprintf(LOG_ERR, "Not enough parameters given."); + printf_sysinfo_usage(1); + return (-1); + } /* Get Parameters */ - param = sysinfo_param(intf, argv[1], &maxset); - if (param < 0) { - ipmi_sysinfo_usage(); - return 0; + if ((param = sysinfo_param(argv[1], &maxset)) < 0) { + lprintf(LOG_ERR, "Invalid mc/bmc %s command: %s", argv[0], argv[1]); + printf_sysinfo_usage(1); + return (-1); } - rc = 0; - if (isset) { + rc = 0; + if (is_set != 0) { str = argv[2]; set = pos = 0; len = strlen(str); /* first block holds 14 bytes, all others hold 16 */ - if (((len + 2) + 15) / 16 >= maxset) - len = maxset * 16 - 2; + if ((len + 2 + 15) / 16 >= maxset) + len = (maxset * 16) - 2; do { memset(paramdata, 0, sizeof(paramdata)); @@ -1049,24 +1053,27 @@ ipmi_sysinfo_main(struct ipmi_intf *intf, int argc, char ** argv) /* First block is special case */ paramdata[2] = 0; /* ascii encoding */ paramdata[3] = len; /* length */ - strncpy(paramdata+4, str+pos, IPMI_SYSINFO_SET0_SIZE); + strncpy(paramdata + 4, str + pos, IPMI_SYSINFO_SET0_SIZE); pos += IPMI_SYSINFO_SET0_SIZE; - } else { - strncpy(paramdata+2, str+pos, IPMI_SYSINFO_SETN_SIZE); + } + else { + strncpy(paramdata + 2, str + pos, IPMI_SYSINFO_SETN_SIZE); pos += IPMI_SYSINFO_SETN_SIZE; } - rc = ipmi_setsysinfo(intf, 18, paramdata); + rc = ipmi_mc_setsysinfo(intf, 18, paramdata); if (rc) break; set++; } while (pos < len); - } else { + } + else { + memset(infostr, 0, sizeof(infostr)); /* Read blocks of data */ pos = 0; - for (set=0; set 0) { - lprintf(LOG_ERR, "%s %s", argv[0], val2str(rc, completion_code_vals)); + lprintf(LOG_ERR, "%s command failed: %s", argv[0], + val2str(rc, completion_code_vals)); } return rc; }