* [PATCH] multipath-tools: reorder vendors in hwtable
@ 2018-12-19 21:23 Xose Vazquez Perez
2018-12-20 23:51 ` Martin Wilck
0 siblings, 1 reply; 4+ messages in thread
From: Xose Vazquez Perez @ 2018-12-19 21:23 UTC (permalink / raw)
Cc: DM-DEVEL ML, Xose Vazquez Perez
Xio was acquired by Violin, and add FlashSystem 9100 to Storwize in comments.
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
---
libmultipath/hwtable.c | 50 ++++++++++++++++++++----------------------
1 file changed, 24 insertions(+), 26 deletions(-)
diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
index d3a8d9b..3eeadde 100644
--- a/libmultipath/hwtable.c
+++ b/libmultipath/hwtable.c
@@ -604,7 +604,7 @@ static struct hwentry default_hw[] = {
.pgpolicy = MULTIBUS,
},
{
- /* Storwize family / SAN Volume Controller / Flex System V7000 / FlashSystem V840/V9000 */
+ /* Storwize family / SAN Volume Controller / Flex System V7000 / FlashSystem V840/V9000/9100 */
.vendor = "IBM",
.product = "^2145",
.no_path_retry = NO_PATH_RETRY_QUEUE,
@@ -1099,31 +1099,7 @@ static struct hwentry default_hw[] = {
.no_path_retry = 30,
},
/*
- * Xiotech
- */
- {
- /* Intelligent Storage Elements family */
- .vendor = "(XIOTECH|XIOtech)",
- .product = "ISE",
- .pgpolicy = MULTIBUS,
- .no_path_retry = 12,
- },
- {
- /* iglu blaze family */
- .vendor = "(XIOTECH|XIOtech)",
- .product = "IGLU DISK",
- .pgpolicy = MULTIBUS,
- .no_path_retry = 30,
- },
- {
- /* Magnitude family */
- .vendor = "(XIOTECH|XIOtech)",
- .product = "Magnitude",
- .pgpolicy = MULTIBUS,
- .no_path_retry = 30,
- },
- /*
- * Violin Memory
+ * Violin Systems
*/
{
/* 3000 / 6000 Series */
@@ -1148,6 +1124,28 @@ static struct hwentry default_hw[] = {
.product = "CONCERTO ARRAY",
.pgpolicy = MULTIBUS,
.no_path_retry = 30,
+ },
+ /* Xiotech */
+ {
+ /* Intelligent Storage Elements family */
+ .vendor = "(XIOTECH|XIOtech)",
+ .product = "ISE",
+ .pgpolicy = MULTIBUS,
+ .no_path_retry = 12,
+ },
+ {
+ /* iglu blaze family */
+ .vendor = "(XIOTECH|XIOtech)",
+ .product = "IGLU DISK",
+ .pgpolicy = MULTIBUS,
+ .no_path_retry = 30,
+ },
+ {
+ /* Magnitude family */
+ .vendor = "(XIOTECH|XIOtech)",
+ .product = "Magnitude",
+ .pgpolicy = MULTIBUS,
+ .no_path_retry = 30,
},
/*
* Promise Technology
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] multipath-tools: reorder vendors in hwtable
2018-12-19 21:23 [PATCH] multipath-tools: reorder vendors in hwtable Xose Vazquez Perez
@ 2018-12-20 23:51 ` Martin Wilck
2019-03-16 23:04 ` Xose Vazquez Perez
0 siblings, 1 reply; 4+ messages in thread
From: Martin Wilck @ 2018-12-20 23:51 UTC (permalink / raw)
To: Xose Vazquez Perez; +Cc: DM-DEVEL ML
On Wed, 2018-12-19 at 22:23 +0100, Xose Vazquez Perez wrote:
> Xio was acquired by Violin, and add FlashSystem 9100 to Storwize in
> comments.
>
> Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
> Cc: DM-DEVEL ML <dm-devel@redhat.com>
> Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
> ---
> libmultipath/hwtable.c | 50 ++++++++++++++++++++------------------
> ----
> 1 file changed, 24 insertions(+), 26 deletions(-)
>
How important is it to reflect this kind of company-A-owns-company-B
information in multipath-tools source code? IMO it makes it harder to
track code changes, for no obvious technical reason.
Regards
Martin
> diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
> index d3a8d9b..3eeadde 100644
> --- a/libmultipath/hwtable.c
> +++ b/libmultipath/hwtable.c
> @@ -604,7 +604,7 @@ static struct hwentry default_hw[] = {
> .pgpolicy = MULTIBUS,
> },
> {
> - /* Storwize family / SAN Volume Controller / Flex
> System V7000 / FlashSystem V840/V9000 */
> + /* Storwize family / SAN Volume Controller / Flex
> System V7000 / FlashSystem V840/V9000/9100 */
> .vendor = "IBM",
> .product = "^2145",
> .no_path_retry = NO_PATH_RETRY_QUEUE,
> @@ -1099,31 +1099,7 @@ static struct hwentry default_hw[] = {
> .no_path_retry = 30,
> },
> /*
> - * Xiotech
> - */
> - {
> - /* Intelligent Storage Elements family */
> - .vendor = "(XIOTECH|XIOtech)",
> - .product = "ISE",
> - .pgpolicy = MULTIBUS,
> - .no_path_retry = 12,
> - },
> - {
> - /* iglu blaze family */
> - .vendor = "(XIOTECH|XIOtech)",
> - .product = "IGLU DISK",
> - .pgpolicy = MULTIBUS,
> - .no_path_retry = 30,
> - },
> - {
> - /* Magnitude family */
> - .vendor = "(XIOTECH|XIOtech)",
> - .product = "Magnitude",
> - .pgpolicy = MULTIBUS,
> - .no_path_retry = 30,
> - },
> - /*
> - * Violin Memory
> + * Violin Systems
> */
> {
> /* 3000 / 6000 Series */
> @@ -1148,6 +1124,28 @@ static struct hwentry default_hw[] = {
> .product = "CONCERTO ARRAY",
> .pgpolicy = MULTIBUS,
> .no_path_retry = 30,
> + },
> + /* Xiotech */
> + {
> + /* Intelligent Storage Elements family */
> + .vendor = "(XIOTECH|XIOtech)",
> + .product = "ISE",
> + .pgpolicy = MULTIBUS,
> + .no_path_retry = 12,
> + },
> + {
> + /* iglu blaze family */
> + .vendor = "(XIOTECH|XIOtech)",
> + .product = "IGLU DISK",
> + .pgpolicy = MULTIBUS,
> + .no_path_retry = 30,
> + },
> + {
> + /* Magnitude family */
> + .vendor = "(XIOTECH|XIOtech)",
> + .product = "Magnitude",
> + .pgpolicy = MULTIBUS,
> + .no_path_retry = 30,
> },
> /*
> * Promise Technology
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] multipath-tools: reorder vendors in hwtable
2018-12-20 23:51 ` Martin Wilck
@ 2019-03-16 23:04 ` Xose Vazquez Perez
2019-03-18 9:42 ` Martin Wilck
0 siblings, 1 reply; 4+ messages in thread
From: Xose Vazquez Perez @ 2019-03-16 23:04 UTC (permalink / raw)
To: Martin Wilck, Christophe Varoqui; +Cc: DM-DEVEL ML
On 12/21/18 12:51 AM, Martin Wilck wrote:
> On Wed, 2018-12-19 at 22:23 +0100, Xose Vazquez Perez wrote:
>> Xio was acquired by Violin, and add FlashSystem 9100 to Storwize in
>> comments.
>>
>> Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
>> Cc: DM-DEVEL ML <dm-devel@redhat.com>
>> Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
>> ---
>> libmultipath/hwtable.c | 50 ++++++++++++++++++++------------------
>> ----
>> 1 file changed, 24 insertions(+), 26 deletions(-)
>>
> How important is it to reflect this kind of company-A-owns-company-B
> information in multipath-tools source code? IMO it makes it harder to
> track code changes, for no obvious technical reason.
Keep the file consistent with the real world.
There is no more Xiotech company, now their products are sold by Violin.
The same happened in the past with:
XIV : RamSan -> IBM
LSI_RDAC : SolidFir -> NetApp
DEC -> Compaq : 3PAR : LEFTHAND : Nimble : SGI -> HPE
DGC : XtremIO -> EMC : Compellent -> DELL
...
......
>> diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
>> index d3a8d9b..3eeadde 100644
>> --- a/libmultipath/hwtable.c
>> +++ b/libmultipath/hwtable.c
>> @@ -604,7 +604,7 @@ static struct hwentry default_hw[] = {
>> .pgpolicy = MULTIBUS,
>> },
>> {
>> - /* Storwize family / SAN Volume Controller / Flex
>> System V7000 / FlashSystem V840/V9000 */
>> + /* Storwize family / SAN Volume Controller / Flex
>> System V7000 / FlashSystem V840/V9000/9100 */
>> .vendor = "IBM",
>> .product = "^2145",
>> .no_path_retry = NO_PATH_RETRY_QUEUE,
>> @@ -1099,31 +1099,7 @@ static struct hwentry default_hw[] = {
>> .no_path_retry = 30,
>> },
>> /*
>> - * Xiotech
>> - */
>> - {
>> - /* Intelligent Storage Elements family */
>> - .vendor = "(XIOTECH|XIOtech)",
>> - .product = "ISE",
>> - .pgpolicy = MULTIBUS,
>> - .no_path_retry = 12,
>> - },
>> - {
>> - /* iglu blaze family */
>> - .vendor = "(XIOTECH|XIOtech)",
>> - .product = "IGLU DISK",
>> - .pgpolicy = MULTIBUS,
>> - .no_path_retry = 30,
>> - },
>> - {
>> - /* Magnitude family */
>> - .vendor = "(XIOTECH|XIOtech)",
>> - .product = "Magnitude",
>> - .pgpolicy = MULTIBUS,
>> - .no_path_retry = 30,
>> - },
>> - /*
>> - * Violin Memory
>> + * Violin Systems
>> */
>> {
>> /* 3000 / 6000 Series */
>> @@ -1148,6 +1124,28 @@ static struct hwentry default_hw[] = {
>> .product = "CONCERTO ARRAY",
>> .pgpolicy = MULTIBUS,
>> .no_path_retry = 30,
>> + },
>> + /* Xiotech */
>> + {
>> + /* Intelligent Storage Elements family */
>> + .vendor = "(XIOTECH|XIOtech)",
>> + .product = "ISE",
>> + .pgpolicy = MULTIBUS,
>> + .no_path_retry = 12,
>> + },
>> + {
>> + /* iglu blaze family */
>> + .vendor = "(XIOTECH|XIOtech)",
>> + .product = "IGLU DISK",
>> + .pgpolicy = MULTIBUS,
>> + .no_path_retry = 30,
>> + },
>> + {
>> + /* Magnitude family */
>> + .vendor = "(XIOTECH|XIOtech)",
>> + .product = "Magnitude",
>> + .pgpolicy = MULTIBUS,
>> + .no_path_retry = 30,
>> },
>> /*
>> * Promise Technology
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] multipath-tools: reorder vendors in hwtable
2019-03-16 23:04 ` Xose Vazquez Perez
@ 2019-03-18 9:42 ` Martin Wilck
0 siblings, 0 replies; 4+ messages in thread
From: Martin Wilck @ 2019-03-18 9:42 UTC (permalink / raw)
To: Xose Vazquez Perez, Christophe Varoqui; +Cc: DM-DEVEL ML
On Sun, 2019-03-17 at 00:04 +0100, Xose Vazquez Perez wrote:
> On 12/21/18 12:51 AM, Martin Wilck wrote:
> > On Wed, 2018-12-19 at 22:23 +0100, Xose Vazquez Perez wrote:
> > > Xio was acquired by Violin, and add FlashSystem 9100 to Storwize
> > > in
> > > comments.
> > >
> > > Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
> > > Cc: DM-DEVEL ML <dm-devel@redhat.com>
> > > Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
> > > ---
> > > libmultipath/hwtable.c | 50 ++++++++++++++++++++--------------
> > > ----
> > > ----
> > > 1 file changed, 24 insertions(+), 26 deletions(-)
> > >
> > How important is it to reflect this kind of company-A-owns-company-
> > B
> > information in multipath-tools source code? IMO it makes it harder
> > to
> > track code changes, for no obvious technical reason.
>
> Keep the file consistent with the real world.
> There is no more Xiotech company, now their products are sold by
> Violin.
> The same happened in the past with:
> XIV : RamSan -> IBM
> LSI_RDAC : SolidFir -> NetApp
> DEC -> Compaq : 3PAR : LEFTHAND : Nimble : SGI -> HPE
> DGC : XtremIO -> EMC : Compellent -> DELL
Fine with me, in general. But I dislike the "reordering vendors" type
of patch, leading to 22 insertions and 22 deletions without changing
any actual code. Sorting the hw entries by comments also makes little
sense to me. IMO we should switch using the actual .vendor/.product
strings for sorting, and perhaps move the "maintainers" information
into a separate file.
The fact that we did it in the past doesn't mean it's the right thing
to do. Anyone who needs to look up properties for a specific device in
hwtable.c will likely use the "search" functionality of her text editor
of choice, rather than rely on the alphabetic sorting of the entries;
and again it's usually the vendor/product string that matters for this
lookup rather than the comment on top of it. Therefore the benefit of
keeping the order is low. But following the history of changes (in
order to figure out when and with what rationale a certain setting was
added) is made significantly harder by patches of this kind.
Martin
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-03-18 9:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-12-19 21:23 [PATCH] multipath-tools: reorder vendors in hwtable Xose Vazquez Perez
2018-12-20 23:51 ` Martin Wilck
2019-03-16 23:04 ` Xose Vazquez Perez
2019-03-18 9:42 ` Martin Wilck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox