On 06/28/2011 03:26 PM, Jim Ramsay wrote:
> This is populated by iscsiadm in 'session' mode when the '-r' options is
> given, and passed along in the session_rec_t. It limits the operation
> of exec_node_op to only the specific session specified by the user.
>
> This is a prerequisite for "multiple sessions per iface recond".
>
> Copyright (c) 2011 Dell Inc.
I have never seen that before. What is the copyright on or what is that
line for? I am more used to linux kernel development where people add
their copyright to the source/header files they change if they have
contributed something significant to them like a new feature or like 5%
of the code or something.
Just some minor coding style nits below. Thanks.
> diff --git a/usr/config.h b/usr/config.h
> index 5cb4d56..579c8ec 100644
> --- a/usr/config.h
> +++ b/usr/config.h
> @@ -193,6 +193,8 @@ typedef struct session_rec {
> struct iscsi_session_timeout_config timeo;
> struct iscsi_error_timeout_config err_timeo;
> struct iscsi_session_operational_config iscsi;
> + struct session_info* info;
You should follow the coding style in the code you are working on. So
the * would go by the field name
struct session_info *info;
>
> list_for_each_entry(rec, rec_list, list) {
> - if (__iscsi_match_session(rec, targetname, addr, port, iface))
> + if (__iscsi_match_session(rec, targetname, addr, port, iface,
> + MATCH_ANY_SID))
Same was above. This should be tabbed over as far as you can then add
spaces so it lines up with the "(" in "(rec".
So do it like you did below
> @@ -1611,7 +1611,8 @@ static iscsi_session_t* session_find_by_rec(node_rec_t
> *rec)
> if (__iscsi_match_session(rec, session->nrec.name,
> session->nrec.conn[0].address,
> session->nrec.conn[0].port,
> - &session->nrec.iface))
> + &session->nrec.iface,
> + MATCH_ANY_SID))
> @@ -323,5 +328,6 @@ int iscsi_match_session(void *data, struct session_info
> *info)
> {
> return __iscsi_match_session(data, info->targetname,
> info->persistent_address,
> - info->persistent_port, &info->iface);
> + info->persistent_port, &info->iface,
> + info->sid);
tabbing/spacing like above.
>
> extern char *strstrip(char *s);
> extern char *cfg_get_string_param(char *pathname, const char *key);
> diff --git a/usr/iscsiadm.c b/usr/iscsiadm.c
> index e3a6b81..937b99e 100644
> --- a/usr/iscsiadm.c
> +++ b/usr/iscsiadm.c
> @@ -292,7 +292,12 @@ for_each_session(struct node_rec *rec,
> iscsi_sysfs_session_op_fn *fn)
> {
> int err, num_found = 0;
>
> - err = iscsi_sysfs_for_each_session(rec, &num_found, fn);
> + if (rec && rec->session.info) {
> + num_found = 1;
> + err = fn(rec, rec->session.info);
> + } else {
> + err = iscsi_sysfs_for_each_session(rec, &num_found, fn);
> + }
We follow the linux kernel coding style (mostly) where we do not add {}s
if they are not needed. So since there is only one line here we would
not add them.
--
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.
