On 03/25/2013 05:29 AM, Zdenek Styblik wrote:
> On Mon, Mar 25, 2013 at 10:16 AM, Zdenek Styblik
> <zdenek.styb...@gmail.com> wrote:
>> On Fri, Mar 22, 2013 at 2:40 PM, Corey Minyard <tcminy...@gmail.com> wrote:
>>> The SOL protocol supports multiple serial ports using the "instance",
>>> allow this to be passed in to ipmitool.
>>>
>> Corey,
>>
>> please, use str2*() functions from 'lib/helper.c' rather than
>> strtol(). Unless this is fixed, I'm against getting this one into CVS.
>> Is it really necessary to use 'unsigned int' for 'instance' since its
>> range is only <1..15>, resp. <1..255>? Wouldn't it be sufficient to
>> use 'uint8_t'?
>> Would it be possible to use int types from 'stdint.h'?
Well, yes, but that can often lead to subtle bugs and reduced
performance. It's generally best to use the natural integer unless you
really need a reduced size.
Plus, nothing else in that code works the way you suggest, so I was just
following style. Using unsigned was probably a mistake, as it was
passed signed into the function. The way you suggest is certainly
better than what is already there, though, so I'll make the change.
>>
>> ~~~
>> - retval = ipmi_sol_activate(intf, 1, interval);
>> + retval = ipmi_sol_activate(intf, 1, interval, 1);
>> ~~~
>> Hard-coded instance number to '1'; is this on purpose(just checking/asking)?
Laziness on my part :). I can fix that. The UI for that particular
option is, well, not ideal, and that's why I didn't fix this at first.
-corey
>> Best regards,
>> Z.
>>
> Ignore previous e-mail as it had 'instance > 14' in it which is wrong.
> I don't know why I put '14' there.
>
> One more thing. I think this check ``if (instance < 1 || instance >
> 15) { /* Error */ }'' should be moved into ipmi_sol_activate(), resp.
> ipmi_sol_deactivate() or perhaps even create new function(wrapper) for
> it.
Yes, that's a good idea. New patch is appended:
-corey
Index: doc/ipmitool.1
===================================================================
RCS file: /cvsroot/ipmitool/ipmitool/doc/ipmitool.1,v
retrieving revision 1.50
diff -u -r1.50 ipmitool.1
--- doc/ipmitool.1 5 Sep 2012 14:22:36 -0000 1.50
+++ doc/ipmitool.1 25 Mar 2013 15:56:51 -0000
@@ -2586,7 +2586,7 @@
by the IPMI over serial channel.
.RE
.TP
-\fIactivate\fP [\fIusesolkeepalive\fP | \fInokeepalive\fP]
+\fIactivate\fP [\fIusesolkeepalive\fP | \fInokeepalive\fP]
[\fIinstance=<number>\fP]
.br
Causes ipmitool to enter Serial Over LAN
@@ -2596,6 +2596,9 @@
sent to the serial console on the remote server.
On exit,the the SOL payload mode is deactivated and
the terminal is reset to its original settings.
+
+If the instance is given, it will activate using the given instance
+number. The default is 1.
.RS
Special escape sequences are provided to control the SOL session:
@@ -2617,7 +2620,7 @@
Note that escapes are only recognized immediately after newline.
.RE
.TP
-\fIdeactivate\fP
+\fIdeactivate\fP [\fIinstance=<number>\fP]
.br
Deactivates Serial Over LAN mode on the BMC.
@@ -2625,6 +2628,9 @@
this command to be sent to the BMC, but in the case of an
unintentional exit from SOL mode, this command may be
necessary to reset the state of the BMC.
+
+If the instance is given, it will deactivate the given instance
+number. The default is 1.
.RE
.TP
\fIspd\fP <\fBi2cbus\fR> <\fBi2caddr\fR> [<\fBchannel\fR>]
[<\fmaxread\fR>]
Index: lib/ipmi_sol.c
===================================================================
RCS file: /cvsroot/ipmitool/ipmitool/lib/ipmi_sol.c,v
retrieving revision 1.62
diff -u -r1.62 ipmi_sol.c
--- lib/ipmi_sol.c 18 Jan 2013 12:37:27 -0000 1.62
+++ lib/ipmi_sol.c 25 Mar 2013 15:56:51 -0000
@@ -1292,12 +1292,18 @@
* ipmi_sol_deactivate
*/
static int
-ipmi_sol_deactivate(struct ipmi_intf * intf)
+ipmi_sol_deactivate(struct ipmi_intf * intf, int instance)
{
struct ipmi_rs * rsp;
struct ipmi_rq req;
uint8_t data[6];
+ if ((instance <= 0) || (instance > 15))
+ {
+ lprintf(LOG_ERR, "Error: Instance must range from 1 to 15");
+ return -1;
+ }
+
memset(&req, 0, sizeof(req));
req.msg.netfn = IPMI_NETFN_APP;
req.msg.cmd = IPMI_DEACTIVATE_PAYLOAD;
@@ -1305,8 +1311,8 @@
req.msg.data = data;
bzero(data, sizeof(data));
- data[0] = IPMI_PAYLOAD_TYPE_SOL; /* payload type */
- data[1] = 1; /* payload instance. Guess! */
+ data[0] = IPMI_PAYLOAD_TYPE_SOL; /* payload type */
+ data[1] = instance; /* payload instance. */
/* Lots of important data */
data[2] = 0;
@@ -1533,7 +1539,7 @@
* ipmi_sol_red_pill
*/
static int
-ipmi_sol_red_pill(struct ipmi_intf * intf)
+ipmi_sol_red_pill(struct ipmi_intf * intf, int instance)
{
char * buffer;
int numRead;
@@ -1679,7 +1685,7 @@
{
lprintf(LOG_ERR, "Error: No response to keepalive -
Terminating session");
/* attempt to clean up anyway */
- ipmi_sol_deactivate(intf);
+ ipmi_sol_deactivate(intf, instance);
exit(1);
}
@@ -1689,7 +1695,7 @@
exit(1);
}
else
- ipmi_sol_deactivate(intf);
+ ipmi_sol_deactivate(intf, instance);
return 0;
}
@@ -1701,7 +1707,8 @@
* ipmi_sol_activate
*/
static int
-ipmi_sol_activate(struct ipmi_intf * intf, int looptest, int interval)
+ipmi_sol_activate(struct ipmi_intf * intf, int looptest, int interval,
+ int instance)
{
struct ipmi_rs * rsp;
struct ipmi_rq req;
@@ -1721,6 +1728,12 @@
return -1;
}
+ if ((instance <= 0) || (instance > 15))
+ {
+ lprintf(LOG_ERR, "Error: Instance must range from 1 to 15");
+ return -1;
+ }
+
/*
* Setup a callback so that the lanplus processing knows what
@@ -1737,7 +1750,7 @@
req.msg.data = data;
data[0] = IPMI_PAYLOAD_TYPE_SOL; /* payload type */
- data[1] = 1; /* payload instance */
+ data[1] = instance; /* payload instance */
/* Lots of important data. Most is default */
data[2] = bSolEncryption? 0x80 : 0;
@@ -1843,7 +1856,7 @@
if(looptest == 1)
{
- ipmi_sol_deactivate(intf);
+ ipmi_sol_deactivate(intf, instance);
usleep(interval*1000);
return 0;
}
@@ -1854,7 +1867,7 @@
* 1) STDIN for user input
* 2) The FD for incoming SOL packets
*/
- if (ipmi_sol_red_pill(intf))
+ if (ipmi_sol_red_pill(intf, instance))
{
lprintf(LOG_ERR, "Error in SOL session");
return -1;
@@ -1874,9 +1887,9 @@
lprintf(LOG_NOTICE, "SOL Commands: info [<channel number>]");
lprintf(LOG_NOTICE, " set <parameter> <value>
[channel]");
lprintf(LOG_NOTICE, " payload <enable|disable|status>
[channel] [userid]");
- lprintf(LOG_NOTICE, " activate
[<usesolkeepalive|nokeepalive>]");
- lprintf(LOG_NOTICE, " deactivate");
- lprintf(LOG_NOTICE, " looptest [<loop times>] [<loop
interval(in ms)>]");
+ lprintf(LOG_NOTICE, " activate
[<usesolkeepalive|nokeepalive>] [instance=<number>]");
+ lprintf(LOG_NOTICE, " deactivate [instance=<number>]");
+ lprintf(LOG_NOTICE, " looptest [<loop times> [<loop
interval(in ms)> [<instance>]]]");
}
@@ -2029,32 +2042,54 @@
* Activate
*/
else if (!strncmp(argv[0], "activate", 8)) {
+ int i;
+ uint8_t instance = 1;
- if (argc > 2) {
- print_sol_usage();
- return -1;
- }
-
- if (argc == 2) {
- if (!strncmp(argv[1], "usesolkeepalive", 15))
+ for (i = 1; i < argc; i++) {
+ if (!strncmp(argv[i], "usesolkeepalive", 15))
_use_sol_for_keepalive = 1;
- else if (!strncmp(argv[1], "nokeepalive", 11))
+ else if (!strncmp(argv[i], "nokeepalive", 11))
_disable_keepalive = 1;
- else {
+ else if (!strncmp(argv[i], "instance=", 9)) {
+ if (str2uchar(argv[i] + 9, &instance) != 0) {
+ lprintf(LOG_ERR,
+ "instance invalid: '%s'",
+ argv[i] + 9);
+ print_sol_usage();
+ return -1;
+ }
+ } else {
print_sol_usage();
return -1;
}
}
- retval = ipmi_sol_activate(intf, 0, 0);
+ retval = ipmi_sol_activate(intf, 0, 0, instance);
}
/*
* Dectivate
*/
- else if (!strncmp(argv[0], "deactivate", 10))
- retval = ipmi_sol_deactivate(intf);
-
+ else if (!strncmp(argv[0], "deactivate", 10)) {
+ int i;
+ uint8_t instance = 1;
+
+ for (i = 1; i < argc; i++) {
+ if (!strncmp(argv[i], "instance=", 9)) {
+ if (str2uchar(argv[i] + 9, &instance) != 0) {
+ lprintf(LOG_ERR,
+ "instance invalid: '%s'",
+ argv[i] + 9);
+ print_sol_usage();
+ return -1;
+ }
+ } else {
+ print_sol_usage();
+ return -1;
+ }
+ }
+ retval = ipmi_sol_deactivate(intf, instance);
+ }
/*
* SOL loop test: Activate and then Dectivate
@@ -2063,8 +2098,9 @@
{
int cnt = 200;
int interval = 100; /* Unit is: ms */
+ uint8_t instance;
- if (argc > 3)
+ if (argc > 4)
{
print_sol_usage();
return -1;
@@ -2074,16 +2110,25 @@
cnt = strtol(argv[1], NULL, 10);
if(cnt <= 0) cnt = 200;
}
- if (argc == 3)
+ if (argc >= 3)
{
interval = strtol(argv[2], NULL, 10);
if(interval < 0) interval = 0;
}
+ if (argc >= 4)
+ {
+ if (str2uchar(argv[3], &instance) != 0) {
+ lprintf(LOG_ERR, "instance invalid: '%s'",
+ argv[3]);
+ print_sol_usage();
+ return -1;
+ }
+ }
while (cnt > 0)
{
printf("remain loop test counter: %d\n", cnt);
- retval = ipmi_sol_activate(intf, 1, interval);
+ retval = ipmi_sol_activate(intf, 1, interval, instance);
if (retval)
{
printf("SOL looptest failed: %d\n", retval);
------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
Ipmitool-devel mailing list
Ipmitool-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
