Skip to contentSkip to conversation

devbind: conform to PEP 8 recommended style

RFC Applied Stephen Hemminger 4 participants 7 messages Sep 1, 2021 – Oct 25, 2021

lore
Share & tools
lore
#0 lore raw

RFC 1/1 devbind: conform to PEP 8 recommended style

This fixes most of the warnings from the Flake8 style checker. The ones remaing are long lines (we allow > 79 characters) and a line break warning. The line break style changed in later versions of PEP 8 and the tool is not updated.

https://www.flake8rules.com/rules/W503.html

---
 usertools/dpdk-devbind.py | 70 ++++++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 31 deletions(-)
usertools/dpdk-devbind.py +39 -31
index 74d16e4c4b13..540138842bd5 100755
@@ -91,6 +91,7 @@
 force_flag = False
 args = []
 
+
 # check if a specific kernel module is loaded
 def module_is_loaded(module):
     global loaded_modules
@@ -178,11 +179,13 @@ def get_pci_device_details(dev_id, probe_lspci):
 
     return device
 
+
 def clear_data():
     '''This function clears any old data'''
     global devices
     devices = {}
 
+
 def get_device_details(devices_type):
     '''This function populates the "devices" dictionary. The keys used are
     the pci addresses (domain:bus:slot.func). The values are themselves
@@ -228,7 +231,7 @@ def get_device_details(devices_type):
         rt_info = route.split()
         for i in range(len(rt_info) - 1):
             if rt_info[i] == "dev":
-                ssh_if.append(rt_info[i+1])
+                ssh_if.append(rt_info[i + 1])
 
     # based on the basic info, get extended text details
     for d in devices.keys():
@@ -282,6 +285,7 @@ def device_type_match(dev, devices_type):
                 return True
     return False
 
+
 def dev_id_from_dev_name(dev_name):
     '''Take a device "name" - a string passed in by user to identify a NIC
     device, and determine the device id - i.e. the domain:bus:slot.func - for
@@ -321,9 +325,9 @@ def unbind_one(dev_id, force):
     filename = "/sys/bus/pci/drivers/%s/unbind" % dev["Driver_str"]
     try:
         f = open(filename, "a")
-    except:
-        sys.exit("Error: unbind failed for %s - Cannot open %s" %
-                 (dev_id, filename))
+    except OSError as err:
+        sys.exit("Error: unbind failed for %s - Cannot open %s: %s" %
+                 (dev_id, filename, err))
     f.write(dev_id)
     f.close()
 
@@ -361,58 +365,58 @@ def bind_one(dev_id, driver, force):
         if exists(filename):
             try:
                 f = open(filename, "w")
-            except:
-                print("Error: bind failed for %s - Cannot open %s"
-                      % (dev_id, filename), file=sys.stderr)
+            except OSError as err:
+                print("Error: bind failed for %s - Cannot open %s: %s"
+                      % (dev_id, filename, err), file=sys.stderr)
                 return
             try:
                 f.write("%s" % driver)
                 f.close()
-            except:
+            except OSError as err:
                 print("Error: bind failed for %s - Cannot write driver %s to "
-                      "PCI ID " % (dev_id, driver), file=sys.stderr)
+                      "PCI ID: %s" % (dev_id, driver, err), file=sys.stderr)
                 return
         # For kernels < 3.15 use new_id to add PCI id's to the driver
         else:
             filename = "/sys/bus/pci/drivers/%s/new_id" % driver
             try:
                 f = open(filename, "w")
-            except:
-                print("Error: bind failed for %s - Cannot open %s"
-                      % (dev_id, filename), file=sys.stderr)
+            except OSError as err:
+                print("Error: bind failed for %s - Cannot open %s: %s"
+                      % (dev_id, filename, err), file=sys.stderr)
                 return
             try:
                 # Convert Device and Vendor Id to int to write to new_id
                 f.write("%04x %04x" % (int(dev["Vendor"], 16),
                                        int(dev["Device"], 16)))
                 f.close()
-            except:
+            except OSError as err:
                 print("Error: bind failed for %s - Cannot write new PCI ID to "
-                      "driver %s" % (dev_id, driver), file=sys.stderr)
+                      "driver %s: %s" % (dev_id, driver, err), file=sys.stderr)
                 return
 
     # do the bind by writing to /sys
     filename = "/sys/bus/pci/drivers/%s/bind" % driver
     try:
         f = open(filename, "a")
-    except:
-        print("Error: bind failed for %s - Cannot open %s"
-              % (dev_id, filename), file=sys.stderr)
+    except OSError as err:
+        print("Error: bind failed for %s - Cannot open %s: %s"
+              % (dev_id, filename, err), file=sys.stderr)
         if saved_driver is not None:  # restore any previous driver
             bind_one(dev_id, saved_driver, force)
         return
     try:
         f.write(dev_id)
         f.close()
-    except:
+    except OSError as err:
         # for some reason, closing dev_id after adding a new PCI ID to new_id
         # results in IOError. however, if the device was successfully bound,
         # we don't care for any errors and can safely ignore IOError
         tmp = get_pci_device_details(dev_id, True)
         if "Driver_str" in tmp and tmp["Driver_str"] == driver:
             return
-        print("Error: bind failed for %s - Cannot bind to driver %s"
-              % (dev_id, driver), file=sys.stderr)
+        print("Error: bind failed for %s - Cannot bind to driver %s: %s"
+              % (dev_id, driver, err), file=sys.stderr)
         if saved_driver is not None:  # restore any previous driver
             bind_one(dev_id, saved_driver, force)
         return
@@ -424,15 +428,15 @@ def bind_one(dev_id, driver, force):
     if exists(filename):
         try:
             f = open(filename, "w")
-        except:
-            sys.exit("Error: unbind failed for %s - Cannot open %s"
-                     % (dev_id, filename))
+        except OSError as err:
+            sys.exit("Error: unbind failed for %s - Cannot open %s: %s"
+                     % (dev_id, filename, err))
         try:
             f.write("\00")
             f.close()
-        except:
-            sys.exit("Error: unbind failed for %s - Cannot open %s"
-                     % (dev_id, filename))
+        except OSError as err:
+            sys.exit("Error: unbind failed for %s - Cannot write %s: %s"
+                     % (dev_id, filename, err))
 
 
 def unbind_all(dev_list, force=False):
@@ -466,7 +470,7 @@ def bind_all(dev_list, driver, force=False):
         dev_id_from_dev_name(driver)
         # if we've made it this far, this means that the "driver" was a valid
         # device string, so it's probably not a valid driver name.
-        sys.exit("Error: Driver '%s' does not look like a valid driver. " \
+        sys.exit("Error: Driver '%s' does not look like a valid driver. "
                  "Did you forget to specify the driver to bind devices to?" % driver)
     except ValueError:
         # driver generated error - it's not a valid device ID, so all is well
@@ -496,8 +500,8 @@ def bind_all(dev_list, driver, force=False):
                 continue
 
             # update information about this device
-            devices[d] = dict(devices[d].items() +
-                              get_pci_device_details(d, True).items())
+            devices[d] = dict(devices[d].items()
+                              + get_pci_device_details(d, True).items())
 
             # check if updated information indicates that the device was bound
             if "Driver_str" in devices[d]:
@@ -511,7 +515,7 @@ def display_devices(title, dev_list, extra_params=None):
      device's dictionary.'''
     strings = []  # this holds the strings to print. We sort before printing
     print("\n%s" % title)
-    print("="*len(title))
+    print("=" * len(title))
     if not dev_list:
         strings.append("<none>")
     else:
@@ -527,6 +531,7 @@ def display_devices(title, dev_list, extra_params=None):
     strings.sort()
     print("\n".join(strings))  # print one per line
 
+
 def show_device_status(devices_type, device_name, if_field=False):
     global dpdk_drivers
     kernel_drv = []
@@ -569,6 +574,7 @@ def show_device_status(devices_type, device_name, if_field=False):
         display_devices("Other %s devices" % device_name, no_drv,
                         "unused=%(Module_str)s")
 
+
 def show_status():
     '''Function called when the script is passed the "--status" option.
     Displays to the user what devices are bound to the igb_uio driver, the
@@ -652,7 +658,7 @@ def parse_args():
         '--status-dev',
         help="Print the status of given device group.",
         choices=['baseband', 'compress', 'crypto', 'event',
-                'mempool', 'misc', 'net', 'regex'])
+                 'mempool', 'misc', 'net', 'regex'])
     bind_group = parser.add_mutually_exclusive_group()
     bind_group.add_argument(
         '-b',
@@ -714,6 +720,7 @@ def parse_args():
         new_args.extend(pci_glob(arg))
     args = new_args
 
+
 def do_arg_actions():
     '''do the actual action requested by the user'''
     global b_flag
@@ -761,5 +768,6 @@ def main():
     get_device_details(misc_devices)
     do_arg_actions()
 
+
 if __name__ == "__main__":
     main()
Signature · 1 line
2.30.2
Acked-by on v1 1/1
#1 lore raw
Quoted 5 lines · Stephen Hemminger

This fixes most of the warnings from the Flake8 style checker. The ones remaing are long lines (we allow > 79 characters) and a line break warning. The line break style changed in later versions of PEP 8 and the tool is not updated.

Quoted 7 lines

https://www.flake8rules.com/rules/W503.html

---
 usertools/dpdk-devbind.py | 70 ++++++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 31 deletions(-)

Thanks for the cleanup.

#2 lore raw
Quoted 8 lines · On Thu, 2 Sep 2021 09:10:47 +0100 Bruce Richardson…
Quoted 7 lines · Stephen Hemminger

This fixes most of the warnings from the Flake8 style checker. The ones remaing are long lines (we allow > 79 characters) and a line break warning. The line break style changed in later versions of PEP 8 and the tool is not updated.

https://www.flake8rules.com/rules/W503.html

Quoted 8 lines
Quoted 5 lines
---
 usertools/dpdk-devbind.py | 70 ++++++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 31 deletions(-)

Thanks for the cleanup.

I don't use devbind (except in artifical testing), could someone with hardware make sure that nothing changed here.

#3 lore raw
Quoted 16 lines · Stephen Hemminger
Quoted 13 lines · On Thu, 2 Sep 2021 09:10:47 +0100 Bruce Richardson…
Quoted 12 lines · Stephen Hemminger

This fixes most of the warnings from the Flake8 style checker. The ones remaing are long lines (we allow > 79 characters) and a line break warning. The line break style changed in later versions of PEP 8 and the tool is not updated.

https://www.flake8rules.com/rules/W503.html

---
 usertools/dpdk-devbind.py | 70 ++++++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 31 deletions(-)
Quoted 6 lines
Quoted 3 lines

Thanks for the cleanup.

I don't use devbind (except in artifical testing), could someone with hardware make sure that nothing changed here.

I didn't go testing with all possible options and inputs, but basic operation for binding unbinding devices and giving error messages for invalid device names works fine with this patch applied.

#4 lore raw
Quoted 21 lines · On Thu, 2 Sep 2021 16:50:14 +0100 Bruce Richardson…
Quoted 20 lines · Stephen Hemminger
Quoted 16 lines · On Thu, 2 Sep 2021 09:10:47 +0100 Bruce Richardson…
Quoted 5 lines · Stephen Hemminger

This fixes most of the warnings from the Flake8 style checker. The ones remaing are long lines (we allow > 79 characters) and a line break warning. The line break style changed in later versions of PEP 8 and the tool is not updated.

Quoted 7 lines

https://www.flake8rules.com/rules/W503.html

---
 usertools/dpdk-devbind.py | 70 ++++++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 31 deletions(-)

Thanks for the cleanup.

Quoted 8 lines
Quoted 2 lines

I don't use devbind (except in artifical testing), could someone with hardware make sure that nothing changed here.

I didn't go testing with all possible options and inputs, but basic operation for binding unbinding devices and giving error messages for invalid device names works fine with this patch applied.

Thanks

#5 lore raw

02/09/2021 18:04, Stephen Hemminger:

Quoted 26 lines
Quoted 24 lines · On Thu, 2 Sep 2021 16:50:14 +0100 Bruce Richardson…
Quoted 22 lines · Stephen Hemminger
Quoted 8 lines · On Thu, 2 Sep 2021 09:10:47 +0100 Bruce Richardson…
Quoted 7 lines · Stephen Hemminger

This fixes most of the warnings from the Flake8 style checker. The ones remaing are long lines (we allow > 79 characters) and a line break warning. The line break style changed in later versions of PEP 8 and the tool is not updated.

https://www.flake8rules.com/rules/W503.html

Quoted 8 lines
Quoted 5 lines
---
 usertools/dpdk-devbind.py | 70 ++++++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 31 deletions(-)

Thanks for the cleanup.

I don't use devbind (except in artifical testing), could someone with hardware make sure that nothing changed here.

Quoted 7 lines
Quoted 5 lines

I didn't go testing with all possible options and inputs, but basic operation for binding unbinding devices and giving error messages for invalid device names works fine with this patch applied.

Thanks

Applied, thanks.

Acked-by on v1 1/1
#6 lore raw
In reply to Stephen Hemminger
Quoted 9 lines · Stephen Hemminger

This fixes most of the warnings from the Flake8 style checker. The ones remaing are long lines (we allow > 79 characters) and a line break warning. The line break style changed in later versions of PEP 8 and the tool is not updated.

https://www.flake8rules.com/rules/W503.html

---

We could improve a lot of code in there, but this is a good baseline.

Signature · 2 lines
Thanks,
Anatoly