mc: watchdog set: Refactor to reduce complexity

Move option parsing for `mc wathdog set` to a separate
function, add a function for host to ipmi integer
conversion (reduce manual byte juggling).
This commit is contained in:
Alexander Amelkin 2018-06-17 12:38:05 +03:00
parent e49a20eece
commit e8e94d8976
No known key found for this signature in database
GPG Key ID: E893587B5B74178D
2 changed files with 75 additions and 55 deletions

View File

@ -111,8 +111,8 @@ uint16_t ipmi_get_oem_id(struct ipmi_intf *intf);
#define IS_SET(v, b) ((v) & (1 << (b))) #define IS_SET(v, b) ((v) & (1 << (b)))
/* le16toh() doesn't exist for Windows or Apple */ /* le16toh(), hto16le(), et. al. don't exist for Windows or Apple */
/* For portability, let's simply define our own version of it here */ /* For portability, let's simply define our own versions here */
static inline uint16_t ipmi16toh(uint16_t le) static inline uint16_t ipmi16toh(uint16_t le)
{ {
uint8_t *data = (uint8_t *)&le; uint8_t *data = (uint8_t *)&le;
@ -124,6 +124,12 @@ static inline uint16_t ipmi16toh(uint16_t le)
return h; return h;
} }
static inline void htoipmi16(uint16_t h, uint8_t *ipmi)
{
ipmi[0] = h & 0xFF; /* LSB */
ipmi[1] = h >> 8; /* MSB */
}
#define ipmi_open_file_read(file) ipmi_open_file(file, 0) #define ipmi_open_file_read(file) ipmi_open_file(file, 0)
#define ipmi_open_file_write(file) ipmi_open_file(file, 1) #define ipmi_open_file_write(file) ipmi_open_file(file, 1)

View File

@ -778,36 +778,26 @@ ipmi_mc_get_watchdog(struct ipmi_intf * intf)
return 0; return 0;
} }
/* ipmi_mc_set_watchdog /* Configuration to set with ipmi_mc_set_watchdog() */
* typedef struct ipmi_mc_set_wdt_conf_s {
* @intf: ipmi interface uint16_t timeout;
* @argc: argument count uint8_t pretimeout;
* @argv: arguments uint8_t intr;
* uint8_t use;
* returns 0 on success uint8_t clear;
* returns non-zero (-1 or IPMI completion code) on error uint8_t action;
*/ bool nolog;
static int bool dontstop;
ipmi_mc_set_watchdog(struct ipmi_intf * intf, int argc, char *argv[]) } wdt_conf_t;
/* Options parser for ipmi_mc_set_watchdog() */
static bool
parse_set_wdt_options(wdt_conf_t *conf, int argc, char *argv[])
{ {
const int MAX_TIMEOUT = 6553; /* Seconds, makes almost USHRT_MAX when const int MAX_TIMEOUT = 6553; /* Seconds, makes almost USHRT_MAX when
converted to 100ms intervals */ converted to 100ms intervals */
const int MAX_PRETIMEOUT = 255; /* Seconds */ const int MAX_PRETIMEOUT = 255; /* Seconds */
bool error = true;
struct ipmi_rs * rsp;
struct ipmi_rq req = {0};
unsigned char msg_data[6] = {0};
struct ipm_get_watchdog_rsp * wdt_res;
uint16_t timeout = 0;
uint8_t pretimeout = 0;
uint8_t intr = 0;
uint8_t use = 0;
uint8_t clear = 0;
uint8_t action = 0;
bool nolog = false;
bool dontstop = false;
int rc = -1;
bool options_error = true;
int i; int i;
if (!argc || !strcmp(argv[0], "help")) { if (!argc || !strcmp(argv[0], "help")) {
@ -830,50 +820,51 @@ ipmi_mc_set_watchdog(struct ipmi_intf * intf, int argc, char *argv[])
val, MAX_TIMEOUT); val, MAX_TIMEOUT);
goto out; goto out;
} }
timeout = val * 10; /* Convert seconds to 100ms intervals */ conf->timeout = val * 10; /* Convert seconds to 100ms intervals */
break; break;
case 'p': /* pretimeout */ case 'p': /* pretimeout */
val = strtoul(vstr, NULL, 10); val = strtoul(vstr, NULL, 10);
if (val < 1 || val > MAX_PRETIMEOUT) { if (val < 1 || val > MAX_PRETIMEOUT) {
lprintf(LOG_ERR, "Pretimeout value %lu is out of range (1-%d)\n", lprintf(LOG_ERR,
"Pretimeout value %lu is out of range (1-%d)\n",
val, MAX_PRETIMEOUT); val, MAX_PRETIMEOUT);
goto out; goto out;
} }
pretimeout = val; /* Convert seconds to 100ms intervals */ conf->pretimeout = val; /* Convert seconds to 100ms intervals */
break; break;
case 'i': /* int */ case 'i': /* int */
if (0 > (val = find_set_wdt_string(wdt_int, vstr))) { if (0 > (val = find_set_wdt_string(wdt_int, vstr))) {
lprintf(LOG_ERR, "Interrupt type '%s' is not valid\n", vstr); lprintf(LOG_ERR, "Interrupt type '%s' is not valid\n", vstr);
goto out; goto out;
} }
intr = val; conf->intr = val;
break; break;
case 'u': /* use */ case 'u': /* use */
if (0 > (val = find_set_wdt_string(wdt_use, vstr))) { if (0 > (val = find_set_wdt_string(wdt_use, vstr))) {
lprintf(LOG_ERR, "Use '%s' is not valid\n", vstr); lprintf(LOG_ERR, "Use '%s' is not valid\n", vstr);
goto out; goto out;
} }
use = val; conf->use = val;
break; break;
case 'a': /* action */ case 'a': /* action */
if (0 > (val = find_set_wdt_string(wdt_action, vstr))) { if (0 > (val = find_set_wdt_string(wdt_action, vstr))) {
lprintf(LOG_ERR, "Use '%s' is not valid\n", vstr); lprintf(LOG_ERR, "Use '%s' is not valid\n", vstr);
goto out; goto out;
} }
action = val; conf->action = val;
break; break;
case 'c': /* clear */ case 'c': /* clear */
if (0 > (val = find_set_wdt_string(wdt_use, vstr))) { if (0 > (val = find_set_wdt_string(wdt_use, vstr))) {
lprintf(LOG_ERR, "Use '%s' is not valid\n", vstr); lprintf(LOG_ERR, "Use '%s' is not valid\n", vstr);
goto out; goto out;
} }
clear |= 1 << val; conf->clear |= 1 << val;
break; break;
case 'n': /* nolog */ case 'n': /* nolog */
nolog = true; conf->nolog = true;
break; break;
case 'd': /* dontstop */ case 'd': /* dontstop */
dontstop = true; conf->dontstop = true;
break; break;
default: default:
@ -882,22 +873,45 @@ ipmi_mc_set_watchdog(struct ipmi_intf * intf, int argc, char *argv[])
} }
} }
options_error = false; error = false;
out:
return error;
}
/* ipmi_mc_set_watchdog
*
* @intf: ipmi interface
* @argc: argument count
* @argv: arguments
*
* returns 0 on success
* returns non-zero (-1 or IPMI completion code) on error
*/
static int
ipmi_mc_set_watchdog(struct ipmi_intf * intf, int argc, char *argv[])
{
struct ipmi_rs * rsp;
struct ipmi_rq req = {0};
unsigned char msg_data[6] = {0};
struct ipm_get_watchdog_rsp * wdt_res;
int rc = -1;
wdt_conf_t conf;
bool options_error = parse_set_wdt_options(&conf, argc, argv);
/* Fill data bytes according to IPMI 2.0 Spec section 27.6 */ /* Fill data bytes according to IPMI 2.0 Spec section 27.6 */
msg_data[0] = nolog << IPMI_WDT_USE_NOLOG_SHIFT; msg_data[0] = conf.nolog << IPMI_WDT_USE_NOLOG_SHIFT;
msg_data[0] |= dontstop << IPMI_WDT_USE_DONTSTOP_SHIFT; msg_data[0] |= conf.dontstop << IPMI_WDT_USE_DONTSTOP_SHIFT;
msg_data[0] |= use & IPMI_WDT_USE_MASK; msg_data[0] |= conf.use & IPMI_WDT_USE_MASK;
msg_data[1] = (intr & IPMI_WDT_INTR_MASK) << IPMI_WDT_INTR_SHIFT; msg_data[1] = (conf.intr & IPMI_WDT_INTR_MASK) << IPMI_WDT_INTR_SHIFT;
msg_data[1] |= action & IPMI_WDT_ACTION_MASK; msg_data[1] |= conf.action & IPMI_WDT_ACTION_MASK;
msg_data[2] = pretimeout; msg_data[2] = conf.pretimeout;
msg_data[3] = clear; msg_data[3] = conf.clear;
msg_data[4] = timeout & 0xFF; /* LSB */ htoipmi16(conf.timeout, &msg_data[4]);
msg_data[5] = timeout >> 8; /* MSB */
req.msg.netfn = IPMI_NETFN_APP; req.msg.netfn = IPMI_NETFN_APP;
req.msg.cmd = BMC_SET_WATCHDOG_TIMER; req.msg.cmd = BMC_SET_WATCHDOG_TIMER;
@ -909,14 +923,14 @@ ipmi_mc_set_watchdog(struct ipmi_intf * intf, int argc, char *argv[])
, msg_data[0], msg_data[1], msg_data[2] , msg_data[0], msg_data[1], msg_data[2]
, msg_data[3], msg_data[4], msg_data[5] , msg_data[3], msg_data[4], msg_data[5]
); );
lprintf(LOG_INFO, " - nolog = %d", nolog); lprintf(LOG_INFO, " - nolog = %d", conf.nolog);
lprintf(LOG_INFO, " - dontstop = %d", dontstop); lprintf(LOG_INFO, " - dontstop = %d", conf.dontstop);
lprintf(LOG_INFO, " - use = 0x%02hhX", use); lprintf(LOG_INFO, " - use = 0x%02hhX", conf.use);
lprintf(LOG_INFO, " - intr = 0x%02hhX", intr); lprintf(LOG_INFO, " - intr = 0x%02hhX", conf.intr);
lprintf(LOG_INFO, " - action = 0x%02hhX", action); lprintf(LOG_INFO, " - action = 0x%02hhX", conf.action);
lprintf(LOG_INFO, " - pretimeout = %hhu", pretimeout); lprintf(LOG_INFO, " - pretimeout = %hhu", conf.pretimeout);
lprintf(LOG_INFO, " - clear = 0x%02hhX", clear); lprintf(LOG_INFO, " - clear = 0x%02hhX", conf.clear);
lprintf(LOG_INFO, " - timeout = %hu", timeout); lprintf(LOG_INFO, " - timeout = %hu", conf.timeout);
rsp = intf->sendrecv(intf, &req); rsp = intf->sendrecv(intf, &req);
if (rsp == NULL) { if (rsp == NULL) {