DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] devbind: check for lspci
@ 2018-11-07 13:56 Anatoly Burakov
  2018-11-07 16:01 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
  2018-11-13 16:06 ` [dpdk-dev] [PATCH v2] " Anatoly Burakov
  0 siblings, 2 replies; 12+ messages in thread
From: Anatoly Burakov @ 2018-11-07 13:56 UTC (permalink / raw)
  To: dev; +Cc: john.mcnamara, stable

On some distributions (such as CentOS 7) lspci may not be installed
by default, causing exceptions which are difficult to interpret.

Fix devbind script to check if lspci is installed at script startup.

Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 usertools/dpdk-devbind.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index 7d564634c..74bf514c0 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -655,6 +655,13 @@ def do_arg_actions():
 
 def main():
     '''program main function'''
+    # check if lspci is installed, suppress any output
+    with open(os.devnull, 'w') as devnull:
+        ret = subprocess.call(['which', 'lspci'],
+                              stdout=devnull, stderr=devnull)
+        if ret != 0:
+            print("'lspci' not found - please install 'lspci'")
+            sys.exit(1)
     parse_args()
     check_modules()
     clear_data()
-- 
2.17.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH] devbind: check for lspci
  2018-11-07 13:56 [dpdk-dev] [PATCH] devbind: check for lspci Anatoly Burakov
@ 2018-11-07 16:01 ` Ferruh Yigit
  2018-11-07 16:30   ` Burakov, Anatoly
  2018-11-13 16:03   ` Burakov, Anatoly
  2018-11-13 16:06 ` [dpdk-dev] [PATCH v2] " Anatoly Burakov
  1 sibling, 2 replies; 12+ messages in thread
From: Ferruh Yigit @ 2018-11-07 16:01 UTC (permalink / raw)
  To: Anatoly Burakov, dev; +Cc: john.mcnamara, stable

On 11/7/2018 1:56 PM, Anatoly Burakov wrote:
> On some distributions (such as CentOS 7) lspci may not be installed
> by default, causing exceptions which are difficult to interpret.
> 
> Fix devbind script to check if lspci is installed at script startup.

I guess we need lspci for `--status`, bind/unbind can be done without `lspci`,
what about adding check only display path?

> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  usertools/dpdk-devbind.py | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
> index 7d564634c..74bf514c0 100755
> --- a/usertools/dpdk-devbind.py
> +++ b/usertools/dpdk-devbind.py
> @@ -655,6 +655,13 @@ def do_arg_actions():
>  
>  def main():
>      '''program main function'''
> +    # check if lspci is installed, suppress any output
> +    with open(os.devnull, 'w') as devnull:
> +        ret = subprocess.call(['which', 'lspci'],
> +                              stdout=devnull, stderr=devnull)
> +        if ret != 0:
> +            print("'lspci' not found - please install 'lspci'")
> +            sys.exit(1)
>      parse_args()
>      check_modules()
>      clear_data()
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH] devbind: check for lspci
  2018-11-07 16:01 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
@ 2018-11-07 16:30   ` Burakov, Anatoly
  2018-11-07 18:07     ` Ferruh Yigit
  2018-11-08 20:38     ` Rami Rosen
  2018-11-13 16:03   ` Burakov, Anatoly
  1 sibling, 2 replies; 12+ messages in thread
From: Burakov, Anatoly @ 2018-11-07 16:30 UTC (permalink / raw)
  To: Ferruh Yigit, dev; +Cc: john.mcnamara, stable

On 07-Nov-18 4:01 PM, Ferruh Yigit wrote:
> On 11/7/2018 1:56 PM, Anatoly Burakov wrote:
>> On some distributions (such as CentOS 7) lspci may not be installed
>> by default, causing exceptions which are difficult to interpret.
>>
>> Fix devbind script to check if lspci is installed at script startup.
> 
> I guess we need lspci for `--status`, bind/unbind can be done without `lspci`,
> what about adding check only display path?

IMO it's not worth respinning over that minor detail, but if you feel 
strongly about it - sure, i can do a v2 with this on display path only :)

> 
>>
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>>   usertools/dpdk-devbind.py | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
>> index 7d564634c..74bf514c0 100755
>> --- a/usertools/dpdk-devbind.py
>> +++ b/usertools/dpdk-devbind.py
>> @@ -655,6 +655,13 @@ def do_arg_actions():
>>   
>>   def main():
>>       '''program main function'''
>> +    # check if lspci is installed, suppress any output
>> +    with open(os.devnull, 'w') as devnull:
>> +        ret = subprocess.call(['which', 'lspci'],
>> +                              stdout=devnull, stderr=devnull)
>> +        if ret != 0:
>> +            print("'lspci' not found - please install 'lspci'")
>> +            sys.exit(1)
>>       parse_args()
>>       check_modules()
>>       clear_data()
>>
> 
> 


-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH] devbind: check for lspci
  2018-11-07 16:30   ` Burakov, Anatoly
@ 2018-11-07 18:07     ` Ferruh Yigit
  2018-11-08 20:38     ` Rami Rosen
  1 sibling, 0 replies; 12+ messages in thread
From: Ferruh Yigit @ 2018-11-07 18:07 UTC (permalink / raw)
  To: Burakov, Anatoly, dev; +Cc: john.mcnamara, stable

On 11/7/2018 4:30 PM, Burakov, Anatoly wrote:
> On 07-Nov-18 4:01 PM, Ferruh Yigit wrote:
>> On 11/7/2018 1:56 PM, Anatoly Burakov wrote:
>>> On some distributions (such as CentOS 7) lspci may not be installed
>>> by default, causing exceptions which are difficult to interpret.
>>>
>>> Fix devbind script to check if lspci is installed at script startup.
>>
>> I guess we need lspci for `--status`, bind/unbind can be done without `lspci`,
>> what about adding check only display path?
> 
> IMO it's not worth respinning over that minor detail, but if you feel 
> strongly about it - sure, i can do a v2 with this on display path only :)

No strong opinion, was just an idea, OK to go as it is if you prefer, not having
lspci installed but still want to use devbind may not be common usecase..

> 
>>
>>>
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>> ---
>>>   usertools/dpdk-devbind.py | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
>>> index 7d564634c..74bf514c0 100755
>>> --- a/usertools/dpdk-devbind.py
>>> +++ b/usertools/dpdk-devbind.py
>>> @@ -655,6 +655,13 @@ def do_arg_actions():
>>>   
>>>   def main():
>>>       '''program main function'''
>>> +    # check if lspci is installed, suppress any output
>>> +    with open(os.devnull, 'w') as devnull:
>>> +        ret = subprocess.call(['which', 'lspci'],
>>> +                              stdout=devnull, stderr=devnull)
>>> +        if ret != 0:
>>> +            print("'lspci' not found - please install 'lspci'")
>>> +            sys.exit(1)
>>>       parse_args()
>>>       check_modules()
>>>       clear_data()
>>>
>>
>>
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH] devbind: check for lspci
  2018-11-07 16:30   ` Burakov, Anatoly
  2018-11-07 18:07     ` Ferruh Yigit
@ 2018-11-08 20:38     ` Rami Rosen
  2018-11-09 12:03       ` Burakov, Anatoly
  1 sibling, 1 reply; 12+ messages in thread
From: Rami Rosen @ 2018-11-08 20:38 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: Ferruh Yigit, dev, john.mcnamara, stable

Hi, Anatoly,

This is really minor nitpick, but in case you decide to send V2, I believe
it should say
>> + if ret != 0:
>> +            print("'lspci' not found - please install pciutils')

as the name of the package containing lspci is pciutils in centos.

Regards,
Rami Rosen


>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH] devbind: check for lspci
  2018-11-08 20:38     ` Rami Rosen
@ 2018-11-09 12:03       ` Burakov, Anatoly
  2018-11-10 11:03         ` Rami Rosen
  0 siblings, 1 reply; 12+ messages in thread
From: Burakov, Anatoly @ 2018-11-09 12:03 UTC (permalink / raw)
  To: Rami Rosen; +Cc: Ferruh Yigit, dev, john.mcnamara, stable

On 08-Nov-18 8:38 PM, Rami Rosen wrote:
> Hi, Anatoly,
> 
> This is really minor nitpick, but in case you decide to send V2, I 
> believe it should say
>  >> + if ret != 0:
>  >> +            print("'lspci' not found - please install pciutils')
> 
> as the name of the package containing lspci is pciutils in centos.

Do all other distros have lspci in package called pciutils? If not, i 
prefer to keep it the way it is.

> 
> Regards,
> Rami Rosen
> 
> 


-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH] devbind: check for lspci
  2018-11-09 12:03       ` Burakov, Anatoly
@ 2018-11-10 11:03         ` Rami Rosen
  2018-11-12  9:18           ` Burakov, Anatoly
  0 siblings, 1 reply; 12+ messages in thread
From: Rami Rosen @ 2018-11-10 11:03 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: Ferruh Yigit, dev, john.mcnamara, stable

HI Anatoly,

>Do all other distros have lspci in package called pciutils? If not, i
>prefer to keep it the way it is.

Your original patch have:

+        if ret != 0:
+            print("'lspci' not found - please install 'lspci'")

And I suggest to consider changing it to:

 >> + if ret != 0:
 >> +            print("'lspci' not found - please install pciutils')
>

Sorry about my ignorance: which distro has a package named "lspci", if at all?

The official project that include the lspci utility is called
"pciutils": see: http://mj.ucw.cz/sw/pciutils/
You can see that a package named "pciutils" is available in great many
distros, like:
Fedora, OpenSuSE, CentOS, RHEL, Ubuntu, Debian, Mandriva and Mageia,
according to the following links:

http://www.rpmfind.net/linux/rpm2html/search.php?query=pciutils&submit=Search+...
https://packages.ubuntu.com/trusty/pciutils
https://packages.debian.org/search?keywords=pciutils
https://rpms.remirepo.net/rpmphp/zoom.php?rpm=pciutils

Regards,
Rami Rosen

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH] devbind: check for lspci
  2018-11-10 11:03         ` Rami Rosen
@ 2018-11-12  9:18           ` Burakov, Anatoly
  0 siblings, 0 replies; 12+ messages in thread
From: Burakov, Anatoly @ 2018-11-12  9:18 UTC (permalink / raw)
  To: Rami Rosen; +Cc: Ferruh Yigit, dev, john.mcnamara, stable

On 10-Nov-18 11:03 AM, Rami Rosen wrote:
> HI Anatoly,
> 
>> Do all other distros have lspci in package called pciutils? If not, i
>> prefer to keep it the way it is.
> 
> Your original patch have:
> 
> +        if ret != 0:
> +            print("'lspci' not found - please install 'lspci'")
> 
> And I suggest to consider changing it to:
> 
>   >> + if ret != 0:
>   >> +            print("'lspci' not found - please install pciutils')
>>
> 
> Sorry about my ignorance: which distro has a package named "lspci", if at all?
> 
> The official project that include the lspci utility is called
> "pciutils": see: http://mj.ucw.cz/sw/pciutils/
> You can see that a package named "pciutils" is available in great many
> distros, like:
> Fedora, OpenSuSE, CentOS, RHEL, Ubuntu, Debian, Mandriva and Mageia,
> according to the following links:
> 
> http://www.rpmfind.net/linux/rpm2html/search.php?query=pciutils&submit=Search+...
> https://packages.ubuntu.com/trusty/pciutils
> https://packages.debian.org/search?keywords=pciutils
> https://rpms.remirepo.net/rpmphp/zoom.php?rpm=pciutils
> 
> Regards,
> Rami Rosen
> 

Hi,

Thanks, that's good to know, will fix in v2.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH] devbind: check for lspci
  2018-11-07 16:01 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
  2018-11-07 16:30   ` Burakov, Anatoly
@ 2018-11-13 16:03   ` Burakov, Anatoly
  1 sibling, 0 replies; 12+ messages in thread
From: Burakov, Anatoly @ 2018-11-13 16:03 UTC (permalink / raw)
  To: Ferruh Yigit, dev; +Cc: john.mcnamara, stable

On 07-Nov-18 4:01 PM, Ferruh Yigit wrote:
> On 11/7/2018 1:56 PM, Anatoly Burakov wrote:
>> On some distributions (such as CentOS 7) lspci may not be installed
>> by default, causing exceptions which are difficult to interpret.
>>
>> Fix devbind script to check if lspci is installed at script startup.
> 
> I guess we need lspci for `--status`, bind/unbind can be done without `lspci`,
> what about adding check only display path?

This is actually incorrect. While we *use* the lspci output only on 
display paths, we actually gather info about devices using lspci in 
get_device_details(). So, i'll leave the code as is and just fix the 
package name for v2.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [dpdk-dev] [PATCH v2] devbind: check for lspci
  2018-11-07 13:56 [dpdk-dev] [PATCH] devbind: check for lspci Anatoly Burakov
  2018-11-07 16:01 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
@ 2018-11-13 16:06 ` Anatoly Burakov
  2018-11-16 16:54   ` Rami Rosen
  1 sibling, 1 reply; 12+ messages in thread
From: Anatoly Burakov @ 2018-11-13 16:06 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, roszenrami, stable

On some distributions (such as CentOS 7) lspci may not be installed
by default, causing exceptions which are difficult to interpret.

Fix devbind script to check if lspci is installed at script startup.

Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    v2: correct package name to "pciutils"

 usertools/dpdk-devbind.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index 7d564634c..40dc28a7d 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -655,6 +655,13 @@ def do_arg_actions():
 
 def main():
     '''program main function'''
+    # check if lspci is installed, suppress any output
+    with open(os.devnull, 'w') as devnull:
+        ret = subprocess.call(['which', 'lspci'],
+                              stdout=devnull, stderr=devnull)
+        if ret != 0:
+            print("'lspci' not found - please install 'pciutils'")
+            sys.exit(1)
     parse_args()
     check_modules()
     clear_data()
-- 
2.17.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH v2] devbind: check for lspci
  2018-11-13 16:06 ` [dpdk-dev] [PATCH v2] " Anatoly Burakov
@ 2018-11-16 16:54   ` Rami Rosen
  2018-11-18 23:05     ` Thomas Monjalon
  0 siblings, 1 reply; 12+ messages in thread
From: Rami Rosen @ 2018-11-16 16:54 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev, Ferruh Yigit, stable

Reviewed-by: Rami Rosen <roszenrami@gmail.com>
On Tue, 13 Nov 2018 at 18:06, Anatoly Burakov <anatoly.burakov@intel.com> wrote:
>
> On some distributions (such as CentOS 7) lspci may not be installed
> by default, causing exceptions which are difficult to interpret.
>
> Fix devbind script to check if lspci is installed at script startup.
>
> Cc: stable@dpdk.org
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>
> Notes:
>     v2: correct package name to "pciutils"
>
>  usertools/dpdk-devbind.py | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
> index 7d564634c..40dc28a7d 100755
> --- a/usertools/dpdk-devbind.py
> +++ b/usertools/dpdk-devbind.py
> @@ -655,6 +655,13 @@ def do_arg_actions():
>
>  def main():
>      '''program main function'''
> +    # check if lspci is installed, suppress any output
> +    with open(os.devnull, 'w') as devnull:
> +        ret = subprocess.call(['which', 'lspci'],
> +                              stdout=devnull, stderr=devnull)
> +        if ret != 0:
> +            print("'lspci' not found - please install 'pciutils'")
> +            sys.exit(1)
>      parse_args()
>      check_modules()
>      clear_data()
> --
> 2.17.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH v2] devbind: check for lspci
  2018-11-16 16:54   ` Rami Rosen
@ 2018-11-18 23:05     ` Thomas Monjalon
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Monjalon @ 2018-11-18 23:05 UTC (permalink / raw)
  To: Burakov, Anatoly; +Cc: dev, Rami Rosen, Ferruh Yigit, stable

16/11/2018 17:54, Rami Rosen:
> On Tue, 13 Nov 2018 at 18:06, Anatoly Burakov <anatoly.burakov@intel.com> wrote:
> >
> > On some distributions (such as CentOS 7) lspci may not be installed
> > by default, causing exceptions which are difficult to interpret.
> >
> > Fix devbind script to check if lspci is installed at script startup.
> >
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Reviewed-by: Rami Rosen <roszenrami@gmail.com>

Applied, thanks

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-11-18 23:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-07 13:56 [dpdk-dev] [PATCH] devbind: check for lspci Anatoly Burakov
2018-11-07 16:01 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
2018-11-07 16:30   ` Burakov, Anatoly
2018-11-07 18:07     ` Ferruh Yigit
2018-11-08 20:38     ` Rami Rosen
2018-11-09 12:03       ` Burakov, Anatoly
2018-11-10 11:03         ` Rami Rosen
2018-11-12  9:18           ` Burakov, Anatoly
2018-11-13 16:03   ` Burakov, Anatoly
2018-11-13 16:06 ` [dpdk-dev] [PATCH v2] " Anatoly Burakov
2018-11-16 16:54   ` Rami Rosen
2018-11-18 23:05     ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).