On Tue, Dec 17, 2019 at 05:28:28PM -0500, Martin K. Petersen wrote:
>
> Michal,
>
> > + {"VMware", "VMware", NULL, BLIST_NO_MATCH_VENDOR | BLIST_NO_TRAY},
>
> Please don't introduce a blist flag to work around deficiencies in the
> matching interface. I suggest you tweak the matching functions so they
> handle a NULL vendor string correctly.
I don't think that will work with the interface for dynamically adding
entries through sysfs.
Thanks
Michal
Michal,
>> Please don't introduce a blist flag to work around deficiencies in the
>> matching interface. I suggest you tweak the matching functions so they
>> handle a NULL vendor string correctly.
>
> I don't think that will work with the interface for dynamically adding
> entries through sysfs.
Please make it work :)
There's nothing conceptually wrong with being able to do:
echo ":Model:Flags" > /proc/scsi/device_info
We keep running into issues where the same device needs to be listed
many times because it gets branded by different vendors.
Brownie points for making all this less clunky. The libata globbing
blacklist works much better, fwiw.
--
Martin K. Petersen Oracle Linux Engineering
On Thu, Dec 19, 2019 at 06:31:12PM -0500, Martin K. Petersen wrote:
>
> Michal,
>
> >> Please don't introduce a blist flag to work around deficiencies in the
> >> matching interface. I suggest you tweak the matching functions so they
> >> handle a NULL vendor string correctly.
> >
> > I don't think that will work with the interface for dynamically adding
> > entries through sysfs.
>
> Please make it work :)
>
> There's nothing conceptually wrong with being able to do:
>
> echo ":Model:Flags" > /proc/scsi/device_info
>
> We keep running into issues where the same device needs to be listed
> many times because it gets branded by different vendors.
>
Is there any description of what the format is supposed to be?
From the current code it looks like comma separated list of blacklist
entries that may be optionally quoted in some way.
The quoting is basically ignored but it is not clear if the inidividual
entries are supposed to be quoted or the whole thing.
Thanks
Michal
On Thu, Dec 19, 2019 at 06:31:12PM -0500, Martin K. Petersen wrote:
>
> Michal,
>
> >> Please don't introduce a blist flag to work around deficiencies in the
> >> matching interface. I suggest you tweak the matching functions so they
> >> handle a NULL vendor string correctly.
> >
> > I don't think that will work with the interface for dynamically adding
> > entries through sysfs.
>
> Please make it work :)
>
> There's nothing conceptually wrong with being able to do:
>
> echo ":Model:Flags" > /proc/scsi/device_info
>
> We keep running into issues where the same device needs to be listed
> many times because it gets branded by different vendors.
Actually the flag is really needed. The vendor string is a field of
specific length, not a pointer. This makes sense because the vendor
string is fixed length. The code adding the blacklist entries cannot
handle NULL, and the matching code works with char array already.
What can be done differently is stop space-padding the values and
instead match the length of the string as provided. This will however
cause API break. Currently short entries are space-padded to match
exactly the provided vendor string. If we change the match to only the
provided string length it will potentially match and blacklist
additional devices.
If a flag is required to trigger the prefix matching it can be used
instead of the flag that disables vendor matching with empty vendor
string.
Thanks
Michal