Merge lp:~bladernr/checkbox/1257308 into lp:checkbox

[Jeff Lane ]

scripts/network Handle ZeroDivisionError better (LP: #1257308)

scripts/network Handle ZeroDivisionError better (LP: #1257308)

[Zygmunt Krynicki (zyga)]

37 - print("theoretical max %sMb/s" % int(self.iface.max_speed))
38 + print("theoretical max %sMb/s\n" % int(self.iface.max_speed))

Is this intended?

review: Needs Information

[Jeff Lane  (bladernr)]

Yes... without the newlines (note the preceding newline above it) all
the output was wedged together and made it more difficult to my eyes
to pick out the failure and quickly parse the output visually.

I can remove those if you prefer, it was just a bit of extra
formatting I threw in (and forgot to mention in the MR).

On Tue, Dec 3, 2013 at 11:05 AM, Zygmunt Krynicki
<email address hidden> wrote:
> Review: Needs Information
>
> 37 - print("theoretical max %sMb/s" % int(self.iface.max_speed))
> 38 + print("theoretical max %sMb/s\n" % int(self.iface.max_speed))
>
> Is this intended?
> --
> https://code.launchpad.net/~bladernr/checkbox/1257308/+merge/197554
> You are the owner of lp:~bladernr/checkbox/1257308.

--
Jeff Lane - Server and Cloud Certification and Tools Development
Ubuntu Ham: W4KDH
Freenode IRC: bladernr or bladernr_
gpg: 1024D/3A14B2DD 8C88 B076 0DD7 B404 1417 C466 4ABD 3635 3A14 B2DD

[Jeff Lane  (bladernr)]

Note, this is the original output:
bladernr@klaatu:~/development/1257308/checkbox-old/scripts$ sudo ./network test -t iperf -i eth0 --target transit.local
DEBUG:root:Shutting down interface:virbr0
DEBUG:root:Shutting down interface:wlan0
DEBUG:root:timeout 180 iperf -c transit.local -n 1024M
------------------------------------------------------------
Client connecting to transit.local, TCP port 5001
TCP window size: 22.9 KByte (default)
------------------------------------------------------------
[ 3] local 192.168.0.100 port 57642 connected with 192.168.0.10 port 5001
[ ID] Interval Transfer Bandwidth
[ 3] 0.0- 9.9 sec 1.00 GBytes 871 Mbits/sec

ERROR:root:Max Speed was not reported properly. Run ethtool and verify that the card is properly reporting its capabilities.
Transfer speed: 871 Mb/s
0.00% of theoretical max 1000Mb/s
WARNING:root:Poor network performance detected
DEBUG:root:Restoring interface:virbr0
DEBUG:root:Restoring interface:wlan0

and this is the slighly revised output:
bladernr@klaatu:~/development/1257308/checkbox-old/scripts$ sudo ./network tes -t iperf -i eth0 --target transit.local
[sudo] password for bladernr:
DEBUG:root:Shutting down interface:virbr0
DEBUG:root:Shutting down interface:wlan0
DEBUG:root:timeout 180 iperf -c transit.local -n 1024M
------------------------------------------------------------
Client connecting to transit.local, TCP port 5001
TCP window size: 22.9 KByte (default)
------------------------------------------------------------
[ 3] local 192.168.0.100 port 57851 connected with 192.168.0.10 port 5001
[ ID] Interval Transfer Bandwidth
[ 3] 0.0- 9.6 sec 1.00 GBytes 893 Mbits/sec

ERROR:root:Max Speed was not reported properly. Run ethtool and verify that the card is properly reporting its capabilities.

Transfer speed: 893 Mb/s
0.00% of theoretical max 1000Mb/s

WARNING:root:Poor network performance detected
DEBUG:root:Restoring interface:virbr0
DEBUG:root:Restoring interface:wlan0

[Jeff Lane  (bladernr)]

Do I need a resubmit for Needs Information? I can't remember... anyway, re-submitting in any case.

review: Needs Resubmitting

[Daniel Manrique (roadmr)]

Looks good to me, save for the instructions that advise running ethtool. If you run this against a wireless interface, ethtool will not be of use, which will leave the user head-scratching. I'll just leave this as a comment, final approval is up to Zygmunt who was the initial reviewer.

[Zygmunt Krynicki (zyga)]

+1

review: Approve

=== modified file 'checkbox-old/debian/changelog'
--- checkbox-old/debian/changelog	2013-11-29 21:32:46 +0000
+++ checkbox-old/debian/changelog	2013-12-03 15:50:47 +0000
@@ -1,6 +1,8 @@
 checkbox (0.17.1) UNRELEASED; urgency=low
 
-  * Incremented changelog
+  [ Jeff Lane ]
+  * scripts/network Handle ZeroDivisionError better (LP: #1257308)
+
 
  -- Sylvain Pineau <[e-mail]>  Fri, 29 Nov 2013 22:30:45 +0100
 

=== modified file 'checkbox-old/scripts/network'
--- checkbox-old/scripts/network	2013-11-13 15:30:48 +0000
+++ checkbox-old/scripts/network	2013-12-03 15:50:47 +0000
@@ -97,10 +97,20 @@
                 scaled_throughput *= 1000
             if units == 'K':
                 scaled_throughput /= 1000
-            percent = scaled_throughput / int(self.iface.max_speed) * 100
-            print("Transfer speed: {} {}b/s".format(throughput, units))
+            try:
+                percent = scaled_throughput / int(self.iface.max_speed) * 100
+            except ZeroDivisionError:
+                # Catches a condition where the interface functions fine but
+                # ethtool fails to properly report max speed. In this case
+                # it's up to the reviewer to pass or fail.
+                logging.error("Max Speed was not reported properly.  Run "
+                              "ethtool and verify that the card is properly "
+                              "reporting its capabilities.")
+                percent = 0
+
+            print("\nTransfer speed: {} {}b/s".format(throughput, units))
             print("%3.2f%% of " % percent, end="")
-            print("theoretical max %sMb/s" % int(self.iface.max_speed))
+            print("theoretical max %sMb/s\n" % int(self.iface.max_speed))
 
             if percent < 40:
                 logging.warn("Poor network performance detected")