Mike Christie wrote:
> Boaz Harrosh wrote:
>> Remove the dark ages /* define debug_print */ in code, to use
>> a Kconfig option. With a system like Kconfig, in code, commented out,
>> configuration options are slavery and hard work.
>> (version control, manual edit ... need I say more)
>>
>> I've used an "int" config bit-mask so more areas of code can be
>> selected with one Koption, but mainly so that allmodconfig will
>> not turn it on.
>>
>> bit-1 - will turn on prints for libiscsi.
>> bit-2 - will turn on prints for libiscsi_tcp & iscsi_tcp.
>>
>> More iscsi drivers should use more bits.
>>
>
> I made this a per module compile time option. I also modified the macors
> so they added a sessionX or connectionX:Y prefix.
Nice! in Israel when we want to say that something is highly desirable, we say
"America"
One small nitpicking comment, we might like to change this code to not
compile at all if CONFIG_DEBUG_KERNEL is not defined something like:
static unsigned int iscsi_max_lun = 512;
module_param_named(max_lun, iscsi_max_lun, uint, S_IRUGO);
+ #ifdef CONFIG_DEBUG_KERNEL
+static int iscsi_sw_tcp_dbg;
+module_param_named(debug_iscsi_tcp, iscsi_sw_tcp_dbg, int,
+ S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(debug_iscsi_tcp, "Turn on debugging for iscsi_tcp module "
+ "Set to 1 to turn on, and zero to turn off. Default is off.");
+
+ #else
+ static const int iscsi_sw_tcp_dbg = 0;
+ #endif /*CONFIG_DEBUG_KERNEL*/
+#define ISCSI_SW_TCP_DBG(_conn, dbg_fmt, arg...) \
...
So the optimizer will discard all this code. Just as we have today.
(The nice thing is that the code still gets compiled only not included in the
output)
What ever you decide I don't have any strong feelings about it.
Also
> diff --git a/include/scsi/libiscsi_tcp.h b/include/scsi/libiscsi_tcp.h
> index 9e3182e..c11b2e1 100644
> --- a/include/scsi/libiscsi_tcp.h
> +++ b/include/scsi/libiscsi_tcp.h
> @@ -91,6 +91,16 @@ enum {
> ISCSI_TCP_SUSPENDED, /* conn is suspended */
> };
>
> +
> +#define ISCSI_DBG_TCP(_conn, dbg_fmt, arg...) \
> + do { \
> + if (iscsi_dbg_libtcp) \
> + iscsi_conn_printk(KERN_INFO, _conn, \
> + "%s " dbg_fmt, \
> + __func__, ##arg); \
> + } while (0);
> +
> +
Are you sure you need this exported in header? It looks like it is only used
in libiscsi_tcp.c? iscsi_tcp.c has it's own parameter.
But if you keep it at header I think it is best for future to also have
an:
+extern int iscsi_dbg_libtcp;
like in libiscsi.h.
BTW same comment for ISCSI_DBG_SESSION/ISCSI_DBG_CONN are they used outside
libiscsi.c ?
> extern void iscsi_tcp_hdr_recv_prep(struct iscsi_tcp_conn *tcp_conn);
> extern int iscsi_tcp_recv_skb(struct iscsi_conn *conn, struct sk_buff *skb,
> unsigned int offset, bool offloaded, int *status);
> --
> 1.6.0.6
Thanks mike this is very nice I added this to my test scripts and it is very
easy to turn on prints for debugging when problems happen.
if using module_param_named() does that means we can change that in runtime?
how?
Boaz
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~----------~----~----~----~------~----~------~--~---
