Add option to restart usb on device status check before performing check.

build/android/buildbot/bb_device_status_check.py

Index: build/android/buildbot/bb_device_status_check.py
diff --git a/build/android/buildbot/bb_device_status_check.py b/build/android/buildbot/bb_device_status_check.py
index a5ef54b34ce8c9a435c6fbb7d31569e38c64a2d7..f402ea66dcf4e4575d78a767adcaf387e1e51667 100755
--- a/build/android/buildbot/bb_device_status_check.py
+++ b/build/android/buildbot/bb_device_status_check.py
@@ -9,11 +9,13 @@ import logging
 import optparse
 import os
 import smtplib
+import subprocess
 import sys
 import re
 import urllib
 
 import bb_annotations
+import bb_utils
 
 sys.path.append(os.path.join(os.path.dirname(__file__),
                              os.pardir, os.pardir, 'util', 'lib',
@@ -203,6 +205,36 @@ def SendDeviceStatusAlert(msg):
     print 'Failed to send alert email. Error: %s' % e
 
 
+def RestartUsb():

[Isaac (away), 2013/10/11 18:10:08]

I'd rather this return error than warning.

This check also needs a guard for testing mode.

[navabi, 2013/10/11 18:39:02]

I disagree. The failure of restart_usb.py should not make device status check
fail, as it is not a necessary step, just one that will hopefully make devices
more reliable. If we see a bot is not being more reliable, we can check this
step for the warning message to see if it is doing the restart usb.
Ok.

[Isaac (away), 2013/10/11 19:04:10]

If you're not failing here, what's your plan to ensure that missetup machines
don't go undetected?

[navabi, 2013/10/11 22:03:28]

The plan is to add error (or email notification) once 1) we find that
restart_usb is helpful, 2) we've found that restart_usb works on the select bots
we try it on first and 3) we believe all bots are set up correctly with
restart_usb

For 2) we can rely on the logs. The plan is to test first on upstream and
downstream perf bots first.

[navabi, 2013/10/11 22:03:28]

bb_device_status_check does not run in testing mode. Only bb_run_bot has testing
mode, and in that mode it will say that it is going to call
bb_device_status_check, but does not call it.

If we want to call bb_device_status_check in testing mode, then the option needs
to be added and all the steps need to have a testing mode condition. This is
probably best as another change.

+  if not os.path.isfile('/usr/bin/restart_usb'):
+    print ('WARNING: Could not restart usb. /usr/bin/restart_usb not installed '
+           'on host (see BUG=305769).')
+    return
+
+  lsusb_proc = bb_utils.SpawnCmd(['lsusb'], stdout=subprocess.PIPE)
+  lsusb_output, _ = lsusb_proc.communicate()
+  if lsusb_proc.returncode:

[Isaac (away), 2013/10/11 18:10:08]

Please return error.

[navabi, 2013/10/11 22:03:28]

Done.

+    print ('WARNING: Could not restart usb. Error getting list of USB ports.')
+    return
+
+  usb_devices = [re.findall('Bus (\d\d\d) Device (\d\d\d)', lsusb_line)[0]
+                 for lsusb_line in lsusb_output.strip().split('\n')]
+
+  # Walk USB devices from leaves up (i.e reverse sorted) restarting the
+  # connection. If a parent node (e.g. usb hub) is restarted before the
+  # devices connected to it, the (bus, dev) for the hub can change, making the
+  # output we have wrong. This way we restart the devices before the hub.
+  for (bus, dev) in reversed(sorted(usb_devices)):

[Isaac (away), 2013/10/11 18:10:08]

Is this possible to restrict to only android devices, and skip hubs, keyboards,
etc?

[navabi, 2013/10/11 18:39:02]

It is, but I don't think that's what we want. I think it makes sense to restart
the USB hub ports too.

[Isaac (away), 2013/10/11 19:04:10]

I'd be surprised if a USB hub needed to be reset.  I'm more inclined to have
this be surgical -- have you seen reseting a hub to be beneficial?

[navabi, 2013/10/11 22:03:28]

The script has always restarted all usb ports (including hubs), and has been
shown to fix device issues.

+    # Can not restart root usb connections
+    if dev != '001':
+      return_code = bb_utils.RunCmd(['/usr/bin/restart_usb', bus, dev],
+                                    flunk_on_failure=False)

[Isaac (away), 2013/10/11 18:10:08]

flunk on failure.

[navabi, 2013/10/11 18:39:02]

See first comment. I don't think this should flunk on failure.

[Isaac (away), 2013/10/11 19:04:10]

Do you expect this binary to occasionally fail?

[navabi, 2013/10/11 22:03:28]

Let's land it and see. We don't know yet as currently it is only run manually by
a few people that know about it.

+      if return_code:
+        print 'Error restarting USB device /dev/bus/usb/%s/%s' % (bus, dev)
+      else:
+        print 'Restarted USB device /dev/bus/usb/%s/%s' % (bus, dev)
+
+
 def main():
   parser = optparse.OptionParser()
   parser.add_option('', '--out-dir',
@@ -212,9 +244,15 @@ def main():
                     help='Will not check if devices are provisioned properly.')
   parser.add_option('--device-status-dashboard', action='store_true',
                     help='Output device status data for dashboard.')
+  parser.add_option('--restart-usb', action='store_true',
+                    help='Restart USB ports before running device check.')
   options, args = parser.parse_args()
   if args:
     parser.error('Unknown options %s' % args)
+
+  if options.restart_usb:
+    RestartUsb()
+
   devices = android_commands.GetAttachedDevices()
   # TODO(navabi): Test to make sure this fails and then fix call
   offline_devices = android_commands.GetAttachedDevices(hardware=False,