linux-kernel - RE: [PATCH] storvsc: add more logging for error and warning messages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 4 Dec 2015 18:41:28 +0000
From:	Long Li <longli@...rosoft.com>
To:	Vitaly Kuznetsov <vkuznets@...hat.com>
CC:	KY Srinivasan <kys@...rosoft.com>,
	Haiyang Zhang <haiyangz@...rosoft.com>,
	"James E.J. Bottomley" <JBottomley@...n.com>,
	"devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>
Subject: RE: [PATCH] storvsc: add more logging for error and warning messages

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@...hat.com]
> Sent: Friday, December 4, 2015 1:53 AM
> To: Long Li <longli@...rosoft.com>
> Cc: KY Srinivasan <kys@...rosoft.com>; Haiyang Zhang
> <haiyangz@...rosoft.com>; James E.J. Bottomley <JBottomley@...n.com>;
> devel@...uxdriverproject.org; linux-kernel@...r.kernel.org; linux-
> scsi@...r.kernel.org
> Subject: Re: [PATCH] storvsc: add more logging for error and warning
> messages
> 
> Long Li <longli@...rosoft.com> writes:
> 
> > Introduce a logging level for storvsc to log certain error/warning
> > messages. Those messages are helpful in some environments, e.g.
> > Microsoft Azure, for customer support and troubleshooting purposes.
> 
> I have an alternative suggestion: let's use dynamic debug! Basically, we need
> to convert all non-error logging to using dev_dbg() and this can be enabled
> dynamically when needed, even reboot won't be required.

This is great idea for debugging!

I think the messages (srb errors) we want to log in this patch are real errors. They are not for debugging, but for customer support in production environment.

Those errors can be ignored in certain specific storage configurations due to some quirks. They are real errors on Azure, so we want to always log them.
 
> 
> >
> > Signed-off-by: Long Li <longli@...rosoft.com>
> > ---
> >  drivers/scsi/storvsc_drv.c | 30 +++++++++++++++++++++++++++++-
> >  1 file changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > index 40c43ae..afa1647 100644
> > --- a/drivers/scsi/storvsc_drv.c
> > +++ b/drivers/scsi/storvsc_drv.c
> > @@ -164,6 +164,21 @@ static int sense_buffer_size =
> > PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
> >  */
> >  static int vmstor_proto_version;
> >
> > +#define STORVSC_LOGGING_NONE	0
> > +#define STORVSC_LOGGING_ERROR	1
> > +#define STORVSC_LOGGING_WARN	2
> > +
> > +static int logging_level = STORVSC_LOGGING_ERROR;
> > +module_param(logging_level, int, S_IRUGO|S_IWUSR);
> > +MODULE_PARM_DESC(logging_level,
> > +	"Logging level, 0 - None, 1 - Error (default), 2 - Warning.");
> > +
> > +static inline bool do_logging(int level) {
> > +	return (logging_level >= level) ? true : false; }
> > +
> > +
> >  struct vmscsi_win8_extension {
> >  	/*
> >  	 * The following were added in Windows 8 @@ -1183,7 +1198,7 @@
> > static void storvsc_command_completion(struct storvsc_cmd_request
> > *cmd_request)
> >
> >  	scmnd->result = vm_srb->scsi_status;
> >
> > -	if (scmnd->result) {
> > +	if (scmnd->result && do_logging(STORVSC_LOGGING_ERROR)) {
> >  		if (scsi_normalize_sense(scmnd->sense_buffer,
> >  				SCSI_SENSE_BUFFERSIZE, &sense_hdr))
> >  			scsi_print_sense_hdr(scmnd->device, "storvsc", @@
> -1239,12
> > +1254,25 @@ static void storvsc_on_io_completion(struct hv_device
> *device,
> >  	stor_pkt->vm_srb.sense_info_length =
> >  	vstor_packet->vm_srb.sense_info_length;
> >
> > +	if (vstor_packet->vm_srb.scsi_status != 0 ||
> > +		vstor_packet->vm_srb.srb_status != SRB_STATUS_SUCCESS)
> > +		if (do_logging(STORVSC_LOGGING_WARN))
> > +			dev_warn(&device->device,
> > +				"cmd 0x%x scsi status 0x%x srb status
> 0x%x\n",
> > +				stor_pkt->vm_srb.cdb[0],
> > +				vstor_packet->vm_srb.scsi_status,
> > +				vstor_packet->vm_srb.srb_status);
> >
> >  	if ((vstor_packet->vm_srb.scsi_status & 0xFF) == 0x02) {
> >  		/* CHECK_CONDITION */
> >  		if (vstor_packet->vm_srb.srb_status &
> >  			SRB_STATUS_AUTOSENSE_VALID) {
> >  			/* autosense data available */
> > +			if (do_logging(STORVSC_LOGGING_WARN))
> > +				dev_warn(&device->device,
> > +					"stor pkt %p autosense data valid -
> len %d\n",
> > +					request,
> > +					vstor_packet-
> >vm_srb.sense_info_length);
> >
> >  			memcpy(request->cmd->sense_buffer,
> >  			       vstor_packet->vm_srb.sense_data,
> 
> --
>   Vitaly
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/