Skip to contentSkip to conversation

scsi_devinfo: devices and pattern matching

DISCUSSION Xose Vazquez Perez 2 participants 2 messages Dec 16, 2017 – Dec 18, 2017 SCSI

lore
Share & tools
lore
#0 lore raw

Hi,

After last pattern matching changes, maybe some entries should be revised:

Empty strings(current meaning?):

	{"", "Scanner", "1.80", BLIST_NOLUN},	/* responds to all lun */
	{"Promise", "", NULL, BLIST_SPARSELUN},
	{"HP", "C1790A", "", BLIST_NOLUN},		/* scanjet iip */
	{"HP", "C2500A", "", BLIST_NOLUN},		/* scanjet iicx */

Overlapping(?) strings(remove one???):

	{"DELL", "PV660F", NULL, BLIST_SPARSELUN},
	{"DELL", "PV660F   PSEUDO", NULL, BLIST_SPARSELUN},

	{"NRC", "MBR-7", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
	{"NRC", "MBR-7.4", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
	
	{"Promise", "", NULL, BLIST_SPARSELUN},
	{"Promise", "VTrak E610f", NULL, BLIST_SPARSELUN | BLIST_NO_RSOC},

Odd "revision level" string(is it right? EOT?):

        {"SEAGATE", "ST157N", "\004|j", BLIST_NOLUN},

Duplicate strings(They should be merged):

	{"TEXEL", "CD-ROM", "1.06", BLIST_BORKEN},
	{"TEXEL", "CD-ROM", "1.06", BLIST_NOLUN},

Keep in mind, for future patches, that there are regex metacharacters(!!):

        {"IOMEGA", "Io20S         *F", NULL, BLIST_KEY},
        {"INSITE", "Floptical   F*8I", NULL, BLIST_KEY},
        {"DELL", "PSEUDO DEVICE .", NULL, BLIST_SPARSELUN},     /* Dell PV 530F */

For homogeneity reasons, should "*" revision level be replaced by NULL?

And the current pattern matching algorithm should be described clearer, right now it's a bit confused.

Thank you.

#1 lore raw
Quoted 11 lines · Xose Vazquez Perez

Hi,

After last pattern matching changes, maybe some entries should be revised:

Empty strings(current meaning?):

	{"", "Scanner", "1.80", BLIST_NOLUN},	/* responds to all lun */
	{"Promise", "", NULL, BLIST_SPARSELUN},
	{"HP", "C1790A", "", BLIST_NOLUN},		/* scanjet iip */
	{"HP", "C2500A", "", BLIST_NOLUN},		/* scanjet iicx */

We would need to check what the hardware actually returns, whether it _really_ is an empty string or whether it's meant to be a catch-all entry.

Quoted 10 lines

Overlapping(?) strings(remove one???):

	{"DELL", "PV660F", NULL, BLIST_SPARSELUN},
	{"DELL", "PV660F   PSEUDO", NULL, BLIST_SPARSELUN},>
	{"NRC", "MBR-7", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
	{"NRC", "MBR-7.4", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
	
	{"Promise", "", NULL, BLIST_SPARSELUN},
	{"Promise", "VTrak E610f", NULL, BLIST_SPARSELUN | BLIST_NO_RSOC},

That is by design, and nothing to worry about, as the specific entries are subsets of the more generic entries.

Quoted 5 lines

Odd "revision level" string(is it right? EOT?):

        {"SEAGATE", "ST157N", "\004|j", BLIST_NOLUN},

We might need to check what the ST157N actually returns; if we can't I fear the entry need to stay as it is.

Quoted 4 lines

Duplicate strings(They should be merged):

	{"TEXEL", "CD-ROM", "1.06", BLIST_BORKEN},
	{"TEXEL", "CD-ROM", "1.06", BLIST_NOLUN},

Ah. Good point. Yes, it should be merged.

Quoted 6 lines

Keep in mind, for future patches, that there are regex metacharacters(!!):

        {"IOMEGA", "Io20S         *F", NULL, BLIST_KEY},
        {"INSITE", "Floptical   F*8I", NULL, BLIST_KEY},
        {"DELL", "PSEUDO DEVICE .", NULL, BLIST_SPARSELUN},     /* Dell PV 530F */

Hmm. Yes, but again we need to validate this with real hardware before making any changes here.

Quoted 7 lines

For homogeneity reasons, should "*" revision level be replaced by NULL?

And the current pattern matching algorithm should be described clearer, right now it's a bit confused.

I'll be sending out a blktest entry for the devinfo string matching; that will give us a proper regression test suite.

Cheers,

Hannes

Signature · 5 lines
Dr. Hannes Reinecke		   Teamlead Storage & Networking
[email protected]			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)