On Wed, Dec 16, 2009 at 5:31 PM, Erez Zilber <[email protected]> wrote:
> On Wed, Dec 2, 2009 at 10:55 AM, Erez Zilber <[email protected]> wrote:
>> I'd like to make some changes in the logging in open-iscsi. The
>> current status is as follows:
>>
>> kernel modules:
>>
>> * We use iscsi_cls_session_printk & iscsi_cls_conn_printk in
>> scsi_transport_iscsi.c. They are sometimes wrapped by macros (e.g.
>> ISCSI_DBG_TRANS_SESSION). These macros use KERN_INFO and are
>> controlled by module parameters.
>>
>> * We use iscsi_session_printk & iscsi_conn_printk for the rest of the
>> kernel code.These macros wrap iscsi_cls_session_printk &
>> iscsi_cls_conn_printk accordingly. They are sometimes wrapped by
>> macros (e.g. ISCSI_SW_TCP_DBG). These macros use KERN_INFO and are
>> controlled by module parameters.
>>
>> * We sometimes use printk calls.
>>
>> userspace:
>>
>> We use log_warning, log_error & log_debug. They depend on the logging
>> level that we use (0-8). if (log_level > level), the log is sent to
>> syslog with the appropriate log level (LOG_WARNING/LOG_ERR/LOG_DEBUG).
>>
>> My motivation: with the current logging mechanism, if an error occurs,
>> I'm unable to tell exactly what happened. The default logging level is
>> too low. Increasing it affects performance. Another problem is that
>> open-iscsi has too many logging mechanisms.
>>
>> I suggest that:
>> 1. For kernel modules, we will have 'events' (or any better name that
>> you suggest) like 'session', 'conn', 'eh', 'cmd' etc. For each event,
>> we will have a logging level. For example, the user may want to set
>> the 'conn' event to 'DEBUG'. It means that we will print all conn
>> related logs that are DEBUG and above (e.g. WARNING, ERROR).
>
> I suggest that each kernel module will have its own events. Each event
> will be represented by a module parameter (with some default value).
>
>> 2. For userspace code, we could do the same (i.e. have events and a
>> log level per event).
>
> Regarding the 'events' in userspace - we will have events A, B & C for
> iscsid and events D, E & F for iscsiadm. For each event, we will
> probably have a default logging level. The user may want to run with
> another logging level for each event. For iscsid, I suggest that we
> add this to iscsid.conf. For iscsiadm, the user will be able to do
> something like:
>
> iscsiadm -d some_level - this will set all events to 'some_level'
> iscsiadm -dE level_for_E -dF level_for_F - this will set the event 'E'
> to 'level_for_E' and the event 'F' to 'level_for_F'. The event 'D'
> will use the default logging level.
>
> Comments?
>
> Thanks,
> Erez
>
I've started working on the new logging. I've started with iscsi_tcp.
Here's a glance of the general idea. If you have comments on the
general implementation, let's discuss them now. Later, it will be much
more difficult for me.
Here it is:
Added the following code in libiscsi.h:
#define iscsi_log(log_level, dev, dbg_fmt, arg...) \
do { \
char *log_level_str; \
switch (log_level) { \
case ISCSI_LOG_ERROR: \
log_level_str = "ERROR"; \
break; \
case ISCSI_LOG_WARN: \
log_level_str = "WARN"; \
break; \
case ISCSI_LOG_INFO: \
log_level_str = "INFO"; \
break; \
case ISCSI_LOG_DEBUG: \
log_level_str = "DEBUG"; \
break; \
case ISCSI_LOG_TRACE: \
log_level_str = "TRACE"; \
break; \
} \
if (log_level > ISCSI_LOG_INFO) { \
if (dev) { \
dev_printk(KERN_DEBUG, \
(struct device *)dev, \
"%s:%d (%s) " dbg_fmt "\n", \
__func__, __LINE__, \
log_level_str, ##arg); \
} else { \
printk(KERN_DEBUG \
"%s:%d (%s) " dbg_fmt "\n", \
__func__, __LINE__, \
log_level_str, ##arg); \
} \
} else { \
if (dev) { \
dev_printk(KERN_INFO, \
(struct device *)dev, \
"%s:%d (%s) " dbg_fmt "\n", \
__func__, __LINE__, \
log_level_str, ##arg); \
} else { \
printk(KERN_INFO \
"%s:%d (%s) " dbg_fmt "\n", \
__func__, __LINE__, \
log_level_str, ##arg); \
} \
} \
} while (0);
added a new iscsi_log.h file:
enum iscsi_log_level {
ISCSI_LOG_ERROR = 1,
ISCSI_LOG_WARN,
ISCSI_LOG_INFO,
ISCSI_LOG_DEBUG,
ISCSI_LOG_TRACE
};
and in iscsi_tcp.c:
/* Logging contexts */
struct {
int general;
int conn;
} iscsi_sw_tcp_log_ctxt = {ISCSI_LOG_INFO, ISCSI_LOG_INFO};
enum iscsi_sw_tcp_ctxt {
ISCSI_SW_TCP_CTXT_GENERAL = 1,
ISCSI_SW_TCP_CTXT_CONN,
};
module_param_named(general_log_ctxt, iscsi_sw_tcp_log_ctxt.general, int,
S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(general_log_ctxt, "blah blah - general");
module_param_named(conn_log_ctxt, iscsi_sw_tcp_log_ctxt.conn, int,
S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(conn_log_ctxt, "blah blah - conn");
#define _iscsi_sw_tcp_log(log_level, ctxt, dev, dbg_fmt, arg...)
\
do { \
switch (ctxt) { \
case ISCSI_SW_TCP_CTXT_GENERAL: \
if (log_level <= iscsi_sw_tcp_log_ctxt.general) \
iscsi_log(log_level, dev, dbg_fmt, \
##arg); \
break; \
case ISCSI_SW_TCP_CTXT_CONN: \
if (log_level <= iscsi_sw_tcp_log_ctxt.conn) \
iscsi_log(log_level, dev, dbg_fmt, \
##arg); \
break; \
} \
} while (0);
#define iscsi_sw_tcp_log(log_level, ctxt, dbg_fmt, arg...) \
do { \
_iscsi_sw_tcp_log(log_level, ctxt, NULL, dbg_fmt, \
##arg); \
} while (0);
#define iscsi_sw_tcp_conn_log(log_level, ctxt, conn, dbg_fmt, arg...) \
do { \
struct device *dev; \
dev = &((struct iscsi_conn *)conn)->cls_conn->dev; \
_iscsi_sw_tcp_log(log_level, ctxt, dev, dbg_fmt, arg); \
} while (0);
#define iscsi_sw_tcp_sess_log(log_level, ctxt, sess, dbg_fmt, arg...) \
do { \
struct device *dev; \
dev = &((struct iscsi_session *)sess)->cls_session->dev;\
_iscsi_sw_tcp_log(log_level, ctxt, dev, dbg_fmt, arg); \
} while (0);
so now, instead of doing the following in iscsi_sw_tcp_send_hdr_done():
ISCSI_SW_TCP_DBG(tcp_conn->iscsi_conn,
"Header done. Next segment size %u total_size %u\n",
tcp_sw_conn->out.segment.size,
tcp_sw_conn->out.segment.total_size);
we will do the following:
iscsi_sw_tcp_conn_log(ISCSI_LOG_INFO, ISCSI_SW_TCP_CTXT_CONN,
tcp_conn->iscsi_conn,
"Header done. Next segment size %u total_size
%u\n",
tcp_sw_conn->out.segment.size,
tcp_sw_conn->out.segment.total_size);
--
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.
