On 10/23/2014 10:33 PM, Douglas Gilbert wrote: > Changing a frequently hacked, big switch parser to being table > based is, of necessity, not a small patch. Testing showed up > some breakages which required extra code and re-factoring. > Since supporting the REPORT SUPPORTED OPERATION CODES command, > checking cdbs for non-zero values in reserved locations, and > the table based parser are closely related; implementing them > at the same time seemed to be practical. But some additions, > such as the COMPARE AND WRITE command, should really be in > their own patches (but adding new bugs was a useful technique > for finding existing ones). > > The first attachment is large and against Christoph's > drivers-for-3.19 branch. It will also apply to his > drivers-for-3.18 branch. The second, smaller attachment is > for anyone who wants to look at (or try) this patch on > lk 3.17.1; first apply the second patch, then the first. > Almost all of my testing has been on lk 3.17.0 and .1 . > > Perhaps contributors to this driver, such as Martin Petersen > who has written large parts of this driver (e.g. logical block > provisioning, DIF and error injection), might run any test > cases they have to determine what I have broken. > > Speed improvements at this stage are marginal at best. > > ChangeLog: > - remove big switch statement in queuecommand() and replace > with a table based parser > - implement REPORT SUPPORTED OPERATION CODES command which > reads that table > - add 'strict' option which when set will cause each incoming > cdb to be checked against the cdb usage mask held in the > table based parser > - add logic for ILLEGAL REQUEST sense key specific field > pointers, use for most ILLEGAL REQUESTs > - add 'capacity data has changed' unit attention since the > virtual_gb option can be changed on-the-fly > - implement REPORT SUPPORTED TASK MANAGEMENT FUNCTIONS command > - implement COMPARE AND WRITE command > - implement NDOB (no data-out buffer) in WRITE SAME(16) > - make GET LBA STATUS work when no logical block provisioning > > Todo: > - re-order teardown in scsi_debug_exit() > - make sdebug_dev_info::stopped atomic (add to end of uas_bm ?) > - review Rob Elliott's suggestions; look at speed ups > - remove host_lock logic and make the host_lock option a dummy > - update some mode page and VPD data to reflect more recent > devices > - changing remaining >> and << byte handling over to > get/put_unaligned_be*() > - set INFO field for COMPARE AND WRITE command MISCOMPAREs > > Signed-off-by: Douglas Gilbert <dgilbert@xxxxxxxxxxxx> Hmm. There are at least three things packed into this patch: - Table-based parsing - Sense code generation fixes for invalid field in cdb - Additional cdb decoding. Can you please split the patch into logical sections, eg patches for - sense code generation improvements - table-based parsing - Additional CDB decoding With that it should be easier to review. And please add a definition to XDWRITEREAD(10) to include/scsi/scsi.h instead of using the raw value in scsi_debug. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@xxxxxxx +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
