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'?
~~~
- 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)?
Best regards,
Z.
> 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 22 Mar 2013 13:33:41 -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 22 Mar 2013 13:33:42 -0000
> @@ -1292,7 +1292,7 @@
> * 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;
> @@ -1305,8 +1305,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 +1533,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 +1679,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 +1689,7 @@
> exit(1);
> }
> else
> - ipmi_sol_deactivate(intf);
> + ipmi_sol_deactivate(intf, instance);
>
> return 0;
> }
> @@ -1701,7 +1701,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;
> @@ -1737,7 +1738,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 +1844,7 @@
>
> if(looptest == 1)
> {
> - ipmi_sol_deactivate(intf);
> + ipmi_sol_deactivate(intf, instance);
> usleep(interval*1000);
> return 0;
> }
> @@ -1854,7 +1855,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,8 +1875,8 @@
> 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, " activate
> [<usesolkeepalive|nokeepalive>] [instance=<number>]");
> + lprintf(LOG_NOTICE, " deactivate [instance=<number>]");
> lprintf(LOG_NOTICE, " looptest [<loop times>] [<loop
> interval(in ms)>]");
> }
>
> @@ -2029,32 +2030,60 @@
> * Activate
> */
> else if (!strncmp(argv[0], "activate", 8)) {
> + int i;
> + unsigned int 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)) {
> + char *end;
> + instance = strtoul(argv[i] + 9, &end, 0);
> + if (*end != 0 || instance == 0 || instance > 15)
> + {
> + 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;
> + unsigned int instance = 1;
> +
> + for (i = 1; i < argc; i++) {
> + if (!strncmp(argv[i], "instance=", 9)) {
> + char *end;
> + instance = strtoul(argv[i] + 9, &end, 0);
> + if (*end != 0 || instance == 0 || instance > 15)
> + {
> + 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
> @@ -2083,7 +2112,7 @@
> 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, 1);
> 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
------------------------------------------------------------------------------
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
