Merge lp:~sylvain-pineau/checkbox/eMMC_as_DISK into lp:checkbox

[Sylvain Pineau]

This MR proposes a way to detect eMMC drives and report them as DISKS using the udev parser of checkbox-support. Thus avoiding the linked (critical) bug.

Please test it on real hw before merging please

[Paul Larson (pwlars)]

It's probably worth updating the copyright in the other files you changed, since they changed and you're updating others.

I'm curious of the test results on real hardware too, and does it (or should it) make a difference whether you are running from a booted usb stick or from an installation to the emmc? Are there some disk tests that we don't want to run for good reason? In the past, I think a lot of testing on things like panda have intentionally avoided disk tests on emmc because they are so slow. Something that took only a few minutes on x86 would suddenly take hours and just wind up hitting the timeout.

review: Needs Information

[Sylvain Pineau (sylvain-pineau)]

Rejecting as using GPT is finally not a good idea

review: Disapprove

Unmerged revisions

4152. By Sylvain Pineau on 2016-01-08

checkbox-support:parsers:udevadm: Allow eMMC drives to be treated as disks

First from a udev point of view it's impossible to distinguish an eMMC drive
from a simple MMC card. both are using the same driver.

The proposed workaround is to detect drives using GPT and treat them as disks.
SD cards using VFAT/MBR won't interfere with disk tests then.

Fixes: https://bugs.launchpad.net/plainbox-provider-checkbox/+bug/1522768

=== modified file 'checkbox-support/checkbox_support/parsers/tests/test_submission.py'
--- checkbox-support/checkbox_support/parsers/tests/test_submission.py	2015-10-19 19:03:48 +0000
+++ checkbox-support/checkbox_support/parsers/tests/test_submission.py	2016-01-08 14:45:26 +0000
@@ -202,7 +202,7 @@
         """
         result = self.getResult("submission_udev_armhf.xml")
         self.assertTrue("device_states" in result)
-        self.assertEqual(len(result["device_states"]), 14)
+        self.assertEqual(len(result["device_states"]), 15)
 
     def test_device_udevadm(self):
         """Device states can be in a udevadm info element."""

=== modified file 'checkbox-support/checkbox_support/parsers/tests/test_udevadm.py'
--- checkbox-support/checkbox_support/parsers/tests/test_udevadm.py	2015-06-10 15:53:02 +0000
+++ checkbox-support/checkbox_support/parsers/tests/test_udevadm.py	2016-01-08 14:45:26 +0000
@@ -555,15 +555,15 @@
 
     def test_PANDABOARD(self):
         devices = self.parse("PANDABOARD")
-        self.assertEqual(len(devices), 14)
+        self.assertEqual(len(devices), 15)
         # Check that the wireless product name is extracted from the platform
         # modalias
-        self.assertEqual(devices[2].product, "wl12xx")
+        self.assertEqual(devices[3].product, "wl12xx")
         self.assertEqual(self.count(devices, "VIDEO"), 0)
         self.assertEqual(self.count(devices, "AUDIO"), 0)
         self.assertEqual(self.count(devices, "KEYBOARD"), 1)
         self.assertEqual(self.count(devices, "TOUCHPAD"), 0)
-        self.assertEqual(self.count(devices, "CARDREADER"), 0)
+        self.assertEqual(self.count(devices, "CARDREADER"), 1)
         self.assertEqual(self.count(devices, "CDROM"), 0)
         self.assertEqual(self.count(devices, "FIREWIRE"), 0)
         self.assertEqual(self.count(devices, "MOUSE"), 0)

=== modified file 'checkbox-support/checkbox_support/parsers/udevadm.py'
--- checkbox-support/checkbox_support/parsers/udevadm.py	2015-11-19 22:05:14 +0000
+++ checkbox-support/checkbox_support/parsers/udevadm.py	2016-01-08 14:45:26 +0000
@@ -1,7 +1,7 @@
 #
 # This file is part of Checkbox.
 #
-# Copyright 2011-2013 Canonical Ltd.
+# Copyright 2011-2016 Canonical Ltd.
 #
 # Checkbox is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3,
@@ -302,7 +302,9 @@
             if self.driver.startswith("sdhci"):
                 return "CARDREADER"
             if self.driver.startswith("mmc"):
-                return "CARDREADER"
+                table_type = self._environment.get("ID_PART_TABLE_TYPE")
+                if table_type and table_type != "gpt":
+                    return "CARDREADER"
             if self.driver == "rts_pstor":
                 return "CARDREADER"
             # See http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=702145
@@ -364,6 +366,10 @@
                     # we need to test, and is a valid disk device
                     # we need to report.
                     return "DISK"
+                if (self.driver == 'mmcblk' and
+                    self._environment.get("ID_PART_TABLE_TYPE") == "gpt"):
+                    # eMMC drives using GPT should also be treated as disks
+                    return "DISK"
             if devtype == "scsi_device":
                 match = SCSI_RE.match(self._environment.get("MODALIAS", ""))
                 type = int(match.group("type"), 16) if match else -1
@@ -567,6 +573,13 @@
             if "ID_MODEL_ENC" in self._environment:
                 return decode_id(self._environment["ID_MODEL_ENC"])
 
+        if self.driver == 'mmcblk':
+            # Check parent device for MMC name
+            if self._stack:
+                parent = self._stack[-1]
+                if "MMC_NAME" in parent._environment:
+                    return parent._environment["MMC_NAME"]
+
         for element in ("NAME", "POWER_SUPPLY_MODEL_NAME"):
             if element in self._environment:
                 return self._environment[element].strip('"')
@@ -674,6 +687,13 @@
         # product/vendor ID, yet still constitute valid ethX interfaces.
         if device.bus == "net" and "ID_NET_NAME_MAC" in device._environment:
             return False
+
+        # Do not ignore eMMC drives using GPT
+        if ("ID_PART_TABLE_TYPE" in device._environment and
+           device.driver == 'mmcblk'):
+            if device._environment["ID_PART_TABLE_TYPE"] == 'gpt':
+                return False
+
         # Do not ignore nvme devices on the pci bus, these are to be treated
         # as disks (categorization is done elsewhere). Note that the *parent*
         # device will have no category, though it's not ignored per se.