Hi Mike/Ulrich

Attaching the updated patch based on your comments.

I did not paste the patch contents as it was pretty long.

On Tue, Aug 9, 2011 at 8:24 AM, Mike Christie <[email protected]> wrote:

> On 07/22/2011 12:09 PM, Vivek S wrote:
> > Changed the way iscsiadm displays usage help about its commands. Rather
> than
> > simply displaying each possible mode along with its options on a single
> > line,
> > the user can now ask help for each mode separately which describes the
> > various options and also provides some examples.
> >
>
>
> > +        printf("\n");
> > +        switch (mode)
> > +        {
> > +            case MODE_DISCOVERYDB:
> > +                printf("iscsiadm -m discoverydb [options...]\n\n");
> > +                options = CMD_LINE_OPTION_PORTAL | CMD_LINE_OPTION_IFACE
> |
> > CMD_LINE_OPTION_OP \
> > +
>
> Not sure if it is the mailer but just try to keep things around 80
> columns in the code like the other code is doing unless it is a string
> and breaking it up makes it more difficult to read.
>
> For example if you hit 80 cols in the middle of a target name
> "iqn.2005-06.com.mydomain.openiscsi.test1" then I would not split that
> up into "iqn.2005-06.com.mydom" and "ain.openiscsi.test1" just to make
> it 80 cols.
>
> But, something like the above could be
>
> options = CMD_LINE_OPTION_PORTAL | CMD_LINE_OPTION_IFACE | \
>                CMD_LINE_OPTION_OP \
>

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/open-iscsi?hl=en.

From e21619f7c344a9eb47925e1758cad2494fb64f6f Mon Sep 17 00:00:00 2001
From: Vivek Subbarao <[email protected]>
Date: Wed, 10 Aug 2011 23:15:42 +0530
Subject: [PATCH 1/1] Updated iscsiadm commad line help


Signed-off-by: Vivek Subbarao <[email protected]>
---
 usr/iscsiadm.c |  450 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 430 insertions(+), 20 deletions(-)

diff --git a/usr/iscsiadm.c b/usr/iscsiadm.c
index 91d886b..05efa21 100644
--- a/usr/iscsiadm.c
+++ b/usr/iscsiadm.c
@@ -28,6 +28,7 @@
 #include <string.h>
 #include <signal.h>
 #include <sys/stat.h>
+#include <sys/ioctl.h>
 
 #include "initiator.h"
 #include "discovery.h"
@@ -74,6 +75,8 @@ enum iscsiadm_op {
 	OP_NONPERSISTENT	= 0x10
 };
 
+#define MAX_CMD_OPTIONS	22
+
 static struct option const long_options[] =
 {
 	{"mode", required_argument, NULL, 'm'},
@@ -103,24 +106,431 @@ static struct option const long_options[] =
 };
 static char *short_options = "RlDVhm:p:P:T:H:I:U:k:L:d:r:n:v:o:sSt:u";
 
-static void usage(int status)
+/*
+ * Structure to hold all iscsiadm command line options and their description.
+ */
+struct option_help
+{
+	char *option;
+	const char *help_str;
+}opt_help[] = {
+	{"p", "Portal in ip:port format. If only ip address is specified then \
+the value 3260 is assumed for the port."},
+	{"T", "Iqn name of the target."},
+	{"I", "Interface to use for the operation. This is the name of file \
+in /etc/iscsi/ifaces/ directory. Multiple interfaces can be specified."},
+	{"o", "One of the new, delete, update, show or nonpersistent \
+operations."},
+	{"t", "Type of discovery. Sendtargets, slp, isns or fw."},
+	{"n", "Name of the field to use in the update operation."},
+	{"v", "Value to use for the specified field in the update operation."},
+	{"H", "SCSI host to use for the operation."},
+	{"r", "Session ID to use. This is either a positive integer or a \
+sysfs path like /sys/devices/platform/hostH/sessionS/targetH:B:I"},
+	{"R", "Rescan all sessions if no SID is specified, else scan only \
+that session."},
+	{"P", "Level of information output. 0 to display a single line and \
+1 to display detailed information in tree format."},
+	{"D", "Discover targets."},
+	{"l", "Login to the discovered or specified target."},
+	{"L", "Login to all sessions with the specified node or connection \
+startup value or all session if all is passed."},
+	{"u", "Logout of the specified node or session."},
+	{"U", "Logout of all sessions with the specified node or connection \
+startup value or all sessions if all is passed."},
+	{"s", "Display session statistics"},
+	{"k", "Kill iscsid."},
+	{"d", "Print debugging information. Valid values are 0 to 8."},
+	{"S", "Show masked values during information display."},
+	{"V", "Display iscsiadm version."},
+	{"h", "Display this help."},
+	{NULL, NULL}
+};
+
+/*
+ * Global defines for all iscsiadm command line options.
+ */
+#define CMD_LINE_OPTION_PORTAL		0
+#define CMD_LINE_OPTION_TGTNAME		1
+#define CMD_LINE_OPTION_IFACE		2
+#define CMD_LINE_OPTION_OP		3
+#define CMD_LINE_OPTION_TYPE		4
+#define CMD_LINE_OPTION_NAME		5
+#define CMD_LINE_OPTION_VALUE		6
+#define CMD_LINE_OPTION_HOST		7
+#define CMD_LINE_OPTION_SID		8
+#define CMD_LINE_OPTION_RESCAN		9
+#define CMD_LINE_OPTION_PRINT		10
+#define CMD_LINE_OPTION_DSCVR		11
+#define CMD_LINE_OPTION_LOGIN		12
+#define CMD_LINE_OPTION_LOGINALL	13
+#define CMD_LINE_OPTION_LOGOUT		14
+#define CMD_LINE_OPTION_LOGOUTALL	15
+#define CMD_LINE_OPTION_STATS		16
+#define CMD_LINE_OPTION_KILLISCSID	17
+#define CMD_LINE_OPTION_DEBUG		18
+#define CMD_LINE_OPTION_SHOW		19
+#define CMD_LINE_OPTION_VERSION		20
+#define CMD_LINE_OPTION_HELP		21
+
+
+/*
+ * Struture containing pointers to option_help structures describing all options
+ * for each mode supported by iscsiadm.
+ */
+struct mode_help
+{
+	struct option_help *op_h[MAX_CMD_OPTIONS];
+}m_help[] = {
+	{
+	{
+	/*
+	 * Mode discovery
+	 */
+	&opt_help[CMD_LINE_OPTION_PORTAL], &opt_help[CMD_LINE_OPTION_IFACE],
+	&opt_help[CMD_LINE_OPTION_OP], &opt_help[CMD_LINE_OPTION_TYPE],
+	&opt_help[CMD_LINE_OPTION_PRINT], &opt_help[CMD_LINE_OPTION_DSCVR],
+       	&opt_help[CMD_LINE_OPTION_LOGIN], &opt_help[CMD_LINE_OPTION_DEBUG],
+	&opt_help[CMD_LINE_OPTION_HELP], NULL
+	}
+	},
+
+	{
+	/*
+	 * Mode discoverydb
+	 */
+	{
+	&opt_help[CMD_LINE_OPTION_PORTAL], &opt_help[CMD_LINE_OPTION_IFACE],
+	&opt_help[CMD_LINE_OPTION_OP], &opt_help[CMD_LINE_OPTION_TYPE],
+	&opt_help[CMD_LINE_OPTION_PRINT], &opt_help[CMD_LINE_OPTION_DSCVR],
+	&opt_help[CMD_LINE_OPTION_LOGIN], &opt_help[CMD_LINE_OPTION_DEBUG], 
+	&opt_help[CMD_LINE_OPTION_HELP], &opt_help[CMD_LINE_OPTION_NAME],
+	&opt_help[CMD_LINE_OPTION_VALUE], NULL
+	}
+	},
+
+	{
+	/*
+	 * Mode node
+	 */
+	{
+	&opt_help[CMD_LINE_OPTION_PORTAL], &opt_help[CMD_LINE_OPTION_IFACE],
+	&opt_help[CMD_LINE_OPTION_DEBUG], &opt_help[CMD_LINE_OPTION_LOGOUTALL],
+	&opt_help[CMD_LINE_OPTION_LOGIN], &opt_help[CMD_LINE_OPTION_LOGINALL],
+	&opt_help[CMD_LINE_OPTION_LOGOUT], &opt_help[CMD_LINE_OPTION_PRINT],
+	&opt_help[CMD_LINE_OPTION_TGTNAME], &opt_help[CMD_LINE_OPTION_SHOW],
+	&opt_help[CMD_LINE_OPTION_RESCAN], &opt_help[CMD_LINE_OPTION_STATS],
+	&opt_help[CMD_LINE_OPTION_HELP], &opt_help[CMD_LINE_OPTION_OP],
+	&opt_help[CMD_LINE_OPTION_NAME], &opt_help[CMD_LINE_OPTION_VALUE], NULL
+	}
+	},
+
+	{
+	/*
+	 * Mode session
+	 */
+	{
+	&opt_help[CMD_LINE_OPTION_SID], &opt_help[CMD_LINE_OPTION_RESCAN],
+	&opt_help[CMD_LINE_OPTION_LOGOUT], &opt_help[CMD_LINE_OPTION_STATS],
+	&opt_help[CMD_LINE_OPTION_OP], &opt_help[CMD_LINE_OPTION_NAME],
+	&opt_help[CMD_LINE_OPTION_VALUE], &opt_help[CMD_LINE_OPTION_DEBUG],
+	&opt_help[CMD_LINE_OPTION_PRINT], &opt_help[CMD_LINE_OPTION_HELP], NULL
+	}
+	},
+
+	{
+	/*
+	 * Mode host
+	 */
+	{
+	&opt_help[CMD_LINE_OPTION_IFACE], &opt_help[CMD_LINE_OPTION_OP],
+	&opt_help[CMD_LINE_OPTION_NAME], &opt_help[CMD_LINE_OPTION_VALUE],
+	&opt_help[CMD_LINE_OPTION_DEBUG], &opt_help[CMD_LINE_OPTION_PRINT],
+	&opt_help[CMD_LINE_OPTION_HELP], NULL
+	}
+	},
+
+	{
+	/*
+	 * Mode iface
+	 */
+	{
+	&opt_help[CMD_LINE_OPTION_LOGIN], NULL
+	}
+	},
+
+	{
+	/*
+	 * Mode fw
+	 */
+	{
+	&opt_help[CMD_LINE_OPTION_HOST], &opt_help[CMD_LINE_OPTION_PRINT], NULL
+	}
+	}
+};
+
+
+/*
+ * Get terminal width or number of columns.
+ */
+int get_cols(void)
 {
-	if (status != 0)
-		fprintf(stderr, "Try `%s --help' for more information.\n",
-			program_name);
-	else {
-		printf("\
-iscsiadm -m discoverydb [ -hV ] [ -d debug_level ] [-P printlevel] [ -t type -p ip:port -I ifaceN ... [ -Dl ] ] | [ [ -p ip:port -t type] \
-[ -o operation ] [ -n name ] [ -v value ] [ -lD ] ] \n\
-iscsiadm -m discovery [ -hV ] [ -d debug_level ] [-P printlevel] [ -t type -p ip:port -I ifaceN ... [ -l ] ] | [ [ -p ip:port ] [ -l | -D ] ] \n\
-iiscsiadm -m node [ -hV ] [ -d debug_level ] [ -P printlevel ] [ -L all,manual,automatic ] [ -U all,manual,automatic ] [ -S ] [ [ -T targetname -p ip:port -I ifaceN ] [ -l | -u | -R | -s] ] \
-[ [ -o  operation  ] [ -n name ] [ -v value ] ]\n\
-iscsiadm -m session [ -hV ] [ -d debug_level ] [ -P  printlevel] [ -r sessionid | sysfsdir [ -R | -u | -s ] [ -o operation ] [ -n name ] [ -v value ] ]\n\
-iscsiadm -m iface [ -hV ] [ -d debug_level ] [ -P printlevel ] [ -I ifacename ] [ [ -o  operation  ] [ -n name ] [ -v value ] ]\n\
-iscsiadm -m fw [ -l ]\n\
-iscsiadm -m host [ -P printlevel ] [ -H hostno ]\n\
-iscsiadm -k priority\n");
+	struct winsize ws;
+	
+	if (ioctl(0,TIOCGWINSZ,&ws)!=0) 
+	{
+		return -1;
 	}
+	return ws.ws_col;
+} 
+
+
+/*
+ * Get pos of the space character in a string traversing backward
+ * from start_idx till end_idx is reached.
+ */
+int find_last_space(const char *str, int start_idx, int end_idx)
+{
+	int i = 0;
+	char *p = (char *)str;
+	for (i=start_idx ; i>=end_idx ; i--, p--)
+	{
+		if (*p == ' ')
+			return i;
+	}
+	return -1;
+}
+
+
+/*
+ * Print string to the terminal taking into account the terminal
+ * width.
+ */
+void print_str(const char *tabs, const char *str)
+{
+	int idx = 0;
+	char fmt[10] = {0};
+	int cols = get_cols();
+
+	if (strlen(str) > cols)
+	{
+		if (str[cols-1] != ' ')
+		{
+			idx = find_last_space(&str[cols-1], (cols-1), 0);
+			if (idx == -1)
+				idx = cols;
+			else
+				idx = idx + 1;
+		}
+		else
+			idx = cols;
+
+		sprintf(fmt, "%s%c%c%d%c%c", tabs, '%', '.', idx, 's', '\n');
+		printf(fmt, str);
+		print_str(tabs, &str[idx]);
+	}
+	else
+		printf("%s%s\n", tabs, str);
+}
+
+
+static void usage(int status, int mode)
+{
+	int i = 0;
+
+	if ((status != 0) || (mode == -1))
+	{
+		printf("\n");
+		print_str("", "Iscsiadm supports the following modes. "
+			"The user should enter atleast one of them. "
+			"For help about a particular mode, enter "
+			"'iscsiadm -m mode -h'.");
+		printf("\n");
+		printf("discoverydb\ndiscovery [DEPRECATED]\nnode\n"
+				"session\niface\nfw\nhost\n\n");
+		print_str("",
+		"NOTE: To kill iscsid, use 'iscsiadm -k priority' "
+				"with a priority of 0.");
+		printf("\n");
+	}
+	else
+	{
+		printf("\n");
+		switch (mode)
+		{
+		case MODE_DISCOVERYDB:
+		printf("iscsiadm -m discoverydb [options...]\n\n");
+		printf("Options\tDescription\n");
+		printf("-------\t-----------\n");
+
+		for (i=0 ; m_help[MODE_DISCOVERYDB].op_h[i] != 0 ; i++)
+		{
+			printf("%s",
+			m_help[MODE_DISCOVERYDB].op_h[i]->option); 
+			print_str("\t",
+			m_help[MODE_DISCOVERYDB].op_h[i]->help_str);
+		}
+				
+		printf("\nExample:\n");
+		printf("\tTo discover a target\n");
+		print_str("\t\t",
+		"iscsiadm -m discoverydb -t st -p 192.168.1.2:3260 -D");
+		printf("\tTo discover and login\n");
+		print_str("\t\t",
+		"iscsiadm -m discoverydb -t st -p 192.168.1.2:3260 -Dl");
+		print_str("\t",
+		"To specify an interface during discovery and login");
+		print_str("\t\t",
+		"iscsiadm -m discoverydb -t st -p 192.168.1.2:3260 -I eth0 "
+		"-Dl");
+		printf("\tTo add or delete a record\n");
+		print_str("\t\t",
+		"iscsiadm -m discoverydb -t st -p 192.168.1.2:3260 -o "
+		"new/delete");
+		printf("\tTo update a record\n");
+		print_str("\t\t",
+		"iscsiadm -m discoverydb -t st -p 192.168.1.2:3260 -o update "
+		"-n discovery.startup -v manual");
+		printf("\tTo display records from discovery database\n");
+		printf("\t\tiscsiadm -m discoverydb\n");
+		break;
+
+		case MODE_DISCOVERY:
+		printf("iscsiadm -m discovery [options...]\n\n");
+		printf("Options\tDescription\n");
+		printf("-------\t-----------\n");
+
+		for (i=0 ; m_help[MODE_DISCOVERY].op_h[i] != 0 ; i++)
+		{
+			printf("%s",
+			m_help[MODE_DISCOVERY].op_h[i]->option);
+			print_str("\t",
+			m_help[MODE_DISCOVERY].op_h[i]->help_str);
+		}
+				
+		printf("\nExample:\n\tTo discover a target\n");
+		print_str("\t\t",
+		"iscsiadm -m discovery -t slp -p 192.168.1.1:3260 -D");
+		printf("\tTo discover and login to a target\n");
+		print_str("\t\t",
+		"iscsiadm -m discovery -t slp -p 192.168.1.1:3260 -Dl");
+		printf("\tTo specify an interface\n");
+		print_str("\t\t",
+		"iscsiadm -m discovery -t slp -p 192.168.1.1:3260 -I eth0");
+		break;
+
+		case MODE_NODE:
+		printf("iscsiadm -m node [options...]\n\n");
+		printf("Options\tDescription\n");
+		printf("-------\t-----------\n");
+
+		for (i=0 ; m_help[MODE_NODE].op_h[i] != 0 ; i++)
+		{
+			printf("%s",
+			m_help[MODE_NODE].op_h[i]->option);
+			print_str("\t",
+			m_help[MODE_NODE].op_h[i]->help_str);
+		}
+			
+		printf("\nExample:\n\tTo login to a target\n");
+		print_str("\t\t",
+		"iscsiadm -m node -T iqn.2005-06.com.mydomain.openiscsi:test1 "
+		"-p 192.168.1.2:3260 -l");
+		print_str("\t",
+		"To login to all targets with node or connection startup type "
+		"as manual");
+		printf("\t\tiscsiadm -m node -L manual\n");
+		print_str("\t",
+		"To logout of all targets with node or connection startup type"
+	        " as automatic");
+		printf("\t\tiscsiadm -m node -U automatic\n");
+		print_str("\t",
+		"To rescan the session passing through a target");
+		print_str("\t\t",
+		"iscsiadm -m node -T iqn.2005-06.com.mydomain.openiscsi.test1 "
+		"-p 192.168.1.2:3260 -R");
+		print_str("\t",
+		"To display statistics about a session passing through a "
+		"target");
+		print_str("\t\t",
+		"iscsiadm -m node -T iqn.2005-06.com.mydomain.openiscsi:test1 "
+		"-p 192.168.1.2:3260 -s");
+		printf("\tTo add a new node record\n");
+		print_str("\t\t",
+		"iscsiadm -m node -T iqn.2005-06.com.mydomain.openiscsi:test1 "
+		"-p 192.168.1.2:3260 -o new");
+		break;
+
+		case MODE_SESSION:
+		printf("iscsiadm -m session [options...]\n\n");
+		printf("Options\tDescription\n");
+		printf("-------\t-----------\n");
+
+		for (i=0 ; m_help[MODE_SESSION].op_h[i] != 0 ; i++)
+		{
+			printf("%s",
+			m_help[MODE_SESSION].op_h[i]->option);
+			print_str("\t",
+			m_help[MODE_SESSION].op_h[i]->help_str);
+		}
+
+		printf("\nExample:\n\tTo logout of a session\n");
+		printf("\t\tiscsiadm -m session -r 1 -u\n");
+		printf("\tTo delete a session record\n");
+		printf("\t\tiscsiadm -m session -r 2 -o delete\n");
+		break;
+
+		case MODE_IFACE:
+		printf("iscsiadm -m iface [options...]\n\n");
+		printf("Options\tDescription\n");
+		printf("-------\t-----------\n");
+
+		for (i=0 ; m_help[MODE_IFACE].op_h[i] != 0 ; i++)
+		{
+			printf("%s",
+			m_help[MODE_IFACE].op_h[i]->option);
+			print_str("\t",
+			m_help[MODE_IFACE].op_h[i]->help_str);
+		}
+
+		printf("\nExample:\n\tTo add a iface record\n");
+		printf("\t\tiscsiadm -m iface -I eth1 -o new\n");
+		break;
+
+		case MODE_FW:
+		printf("iscsiadm -m fw [options...]\n\n");
+		printf("Options\tDescription\n");
+		printf("-------\t-----------\n");
+
+		for (i=0 ; m_help[MODE_FW].op_h[i] != 0 ; i++)
+		{
+			printf("%s",
+			m_help[MODE_FW].op_h[i]->option);
+			print_str("\t",
+			m_help[MODE_FW].op_h[i]->help_str);
+		}
+		break;
+
+		case MODE_HOST:
+		printf("iscsiadm -m host [options...]\n\n");
+		printf("Options\tDescription\n");
+		printf("-------\t-----------\n");
+
+		for (i=0 ; m_help[MODE_HOST].op_h[i] != 0 ; i++)
+		{
+			printf("%s",
+			m_help[MODE_HOST].op_h[i]->option);
+			print_str("\t",
+			m_help[MODE_HOST].op_h[i]->help_str);
+		}
+		break;
+			
+		default:
+		printf("Invalid mode\n");
+		}
+		printf("\nSEE MAN PAGE FOR MORE DETAILS\n\n");
+	}
+
 	exit(status);
 }
 
@@ -370,7 +780,7 @@ logout_by_startup(char *mode)
 	if (!mode || !(!strcmp(mode, "automatic") || !strcmp(mode, "all") ||
 	    !strcmp(mode,"manual"))) {
 		log_error("Invalid logoutall option %s.", mode);
-		usage(ISCSI_ERR_INVAL);
+		usage(ISCSI_ERR_INVAL, 0);
 		return ISCSI_ERR_INVAL;
 	}
 
@@ -440,7 +850,7 @@ login_by_startup(char *mode)
 	if (!mode || !(!strcmp(mode, "automatic") || !strcmp(mode, "all") ||
 		       !strcmp(mode,"manual") || !strcmp(mode, "onboot"))) {
 		log_error("Invalid loginall option %s.", mode);
-		usage(ISCSI_ERR_INVAL);
+		usage(ISCSI_ERR_INVAL, 0);
 		return ISCSI_ERR_INVAL;
 	}
 
@@ -2071,7 +2481,7 @@ main(int argc, char **argv)
 				ISCSI_VERSION_STR);
 			return 0;
 		case 'h':
-			usage(0);
+			usage(0, mode);
 		}
 	}
 
@@ -2087,7 +2497,7 @@ main(int argc, char **argv)
 	}
 
 	if (mode < 0)
-		usage(ISCSI_ERR_INVAL);
+		usage(ISCSI_ERR_INVAL, 0);
 
 	if (mode == MODE_FW) {
 		if ((rc = verify_mode_params(argc, argv, "ml", 0))) {
-- 
1.7.4.1

Reply via email to