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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <SN1PR0301MB19835DD28CCB381DCA384DC9CE0C0@SN1PR0301MB1983.namprd03.prod.outlook.com>
Date:	Fri, 4 Dec 2015 02:55:41 +0000
From:	Long Li <longli@...rosoft.com>
To:	Joe Perches <joe@...ches.com>, KY Srinivasan <kys@...rosoft.com>,
	"Haiyang Zhang" <haiyangz@...rosoft.com>,
	"James E.J. Bottomley" <JBottomley@...n.com>
CC:	"devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
	"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] storvsc: add more logging for error and warning messages

Thanks Joe.

I'll send out another patch.

> -----Original Message-----
> From: Joe Perches [mailto:joe@...ches.com]
> Sent: Thursday, December 3, 2015 6:28 PM
> To: Long Li <longli@...rosoft.com>; KY Srinivasan <kys@...rosoft.com>;
> Haiyang Zhang <haiyangz@...rosoft.com>; James E.J. Bottomley
> <JBottomley@...n.com>
> Cc: devel@...uxdriverproject.org; linux-scsi@...r.kernel.org; linux-
> kernel@...r.kernel.org
> Subject: Re: [PATCH] storvsc: add more logging for error and warning
> messages
> 
> On Thu, 2015-12-03 at 19:47 -0800, Long Li wrote:
> > 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.
> []
> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> []
> > +static inline bool do_logging(int level) {
> > +	return (logging_level >= level) ? true : false;
> 
> The ternary is not necessary
> 
> 	return logging_level >= level;
> 
> is enough
> 
> > +}
> > +
> > +
> >  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",
> 
> Is it appropriate to make this scsi_normalize_sense call conditional on
> do_logging here?
> 
> > @@ -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);
> 
> It might make some sense to use another macro indirection like
> 
> #define svc_log_warn(dev, level, fmt, ...)			\
> do {								\
> 	if (do_logging(STORSVC_LOGGING_##level)			\
> 		dev_warn(&(dev)->device, fmt, ##__VA_ARGS__);	\
> } while (0)
> 
> So a use could be:
> 
> 	if (vstore_packet...)
> 		svc_log_warn(device, WARN, ...);
> 
> >
--
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/