Merge lp:~salgado/linaro-image-tools/bug-697824 into lp:linaro-image-tools/11.11

[Guilherme Salgado]

Change OMAP configs to use the correct serial tty depending on the kernel version that is installed on the rootfs. This is a better fix for bug 697824, but is still a temporary one which we hope to get rid of soon.

[Loïc Minier (lool)]

It's much longer than I had hoped for a temporary workaround, but it's all tested so that's nice!

One thing which is a bit odd is that we do the runtime serialtty only for OMAP boards, so that it looks like:
+ _serial_tty = 'ttyO2'
+ _extra_serial_opts = 'console=tty0 console=%s,115200n8'
+ _live_serial_opts = 'serialtty=%s'

for OMAP boards and like this for non-OMAP boards:
+ serial_tty = 'ttyAMA2'
+ extra_serial_opts = 'console=tty0 console=%s,115200n8' % serial_tty
+ live_serial_opts = 'serialtty=%s' % serial_tty

could we just use the same approach for all boards (i.e. do all boards at runtime) as to have the same structure for all configs?

review: Approve

[Guilherme Salgado (salgado)]

> It's much longer than I had hoped for a temporary workaround, but it's all
> tested so that's nice!
>
> One thing which is a bit odd is that we do the runtime serialtty only for OMAP
> boards, so that it looks like:
> + _serial_tty = 'ttyO2'
> + _extra_serial_opts = 'console=tty0 console=%s,115200n8'
> + _live_serial_opts = 'serialtty=%s'
>
> for OMAP boards and like this for non-OMAP boards:
> + serial_tty = 'ttyAMA2'
> + extra_serial_opts = 'console=tty0 console=%s,115200n8' % serial_tty
> + live_serial_opts = 'serialtty=%s' % serial_tty
>
> could we just use the same approach for all boards (i.e. do all boards at
> runtime) as to have the same structure for all configs?

That's because OMAP configs are the exception, but once this hack is reverted they will look just like the other ones. The special case in the OMAP config is even described in an XXX comment (with a mention to the bug) to make it clear why it's different. But maybe you're anticipating some sort of problems we might have because of the different structure?

[James Westby (james-w)]

On Fri, 28 Jan 2011 15:52:44 -0000, Guilherme Salgado <email address hidden> wrote:
> Change OMAP configs to use the correct serial tty depending on the
> kernel version that is installed on the rootfs. This is a better fix
> for bug 697824, but is still a temporary one which we hope to get rid
> of soon.

Can we gate this based on the hwpack version? Or will that happen as
part of the new version support going in.

Thanks,

James

[Guilherme Salgado (salgado)]

On Fri, 2011-01-28 at 11:14 -0500, James Westby wrote:
> On Fri, 28 Jan 2011 15:52:44 -0000, Guilherme Salgado <email address hidden> wrote:
> > Change OMAP configs to use the correct serial tty depending on the
> > kernel version that is installed on the rootfs. This is a better fix
> > for bug 697824, but is still a temporary one which we hope to get rid
> > of soon.
>
> Can we gate this based on the hwpack version? Or will that happen as
> part of the new version support going in.

That's the plan; we'll use whatever serial_tty is in a v2 hwpack,
falling back to ttyS2 for v1 hwpacks.

[Loïc Minier (lool)]

When we move to hwpack v2, we can move back to hardcoding ttyS2 for hardware pack v1, and use the hwpack serial_tty field with hardware pack v2.

=== modified file 'linaro_media_create/boards.py'
--- linaro_media_create/boards.py	2011-01-26 18:03:50 +0000
+++ linaro_media_create/boards.py	2011-01-28 15:52:27 +0000
@@ -8,6 +8,7 @@
 import atexit
 import glob
 import os
+import re
 import tempfile
 
 from linaro_media_create import cmd_runner
@@ -30,6 +31,7 @@
     load_addr = None
     kernel_suffix = None
     boot_script = None
+    serial_tty = None
 
     @classmethod
     def get_sfdisk_cmd(cls):
@@ -61,7 +63,7 @@
             # XXX: I think this is not needed as we have board-specific
             # serial options for when is_live is true.
             if is_live:
-                serial_opts += ' serialtty=ttyS2'
+                serial_opts += ' serialtty=%s' % cls.serial_tty
 
         serial_opts += ' %s' % cls.extra_serial_opts
 
@@ -90,7 +92,8 @@
     def make_boot_files(cls, uboot_parts_dir, is_live, is_lowmem, consoles,
                         root_dir, rootfs_uuid, boot_dir, boot_script,
                         boot_device_or_file):
-        boot_cmd = cls._get_boot_cmd(is_live, is_lowmem, consoles, rootfs_uuid)
+        boot_cmd = cls._get_boot_cmd(
+            is_live, is_lowmem, consoles, rootfs_uuid)
         cls._make_boot_files(
             uboot_parts_dir, boot_cmd, root_dir, boot_dir, boot_script,
             boot_device_or_file)
@@ -106,11 +109,51 @@
         raise NotImplementedError()
 
 
+class classproperty(object):
+    """A descriptor that provides @property behavior on class methods."""
+    def __init__(self, getter):
+        self.getter = getter
+    def __get__(self, instance, cls):
+        return self.getter(cls)
+
+
 class OmapConfig(BoardConfig):
 
+    # XXX: Here we define these things as dynamic properties because our
+    # temporary hack to fix bug 697824 relies on changing the board's
+    # serial_tty at run time.
+    _extra_serial_opts = None
+    _live_serial_opts = None
+    _serial_tty = None
+
+    @classproperty
+    def live_serial_opts(cls):
+        return cls._live_serial_opts % cls.serial_tty
+
+    @classproperty
+    def extra_serial_opts(cls):
+        return cls._extra_serial_opts % cls.serial_tty
+
+    @classmethod
+    def set_appropriate_serial_tty(cls, chroot_dir):
+        """Set the appropriate serial_tty depending on the kernel used.
+
+        If the kernel found in the chroot dir is << 2.6.36 we use tyyS2, else
+        we use the default value (_serial_tty).
+        """
+        # XXX: This is also part of our temporary hack to fix bug 697824.
+        cls.serial_tty = cls._serial_tty
+        vmlinuz = _get_file_matching(
+            os.path.join(chroot_dir, 'boot', 'vmlinuz*'))
+        basename = os.path.basename(vmlinuz)
+        minor_version = re.match('.*2\.6\.([0-9]{2}).*', basename).group(1)
+        if int(minor_version) < 36:
+            cls.serial_tty = 'ttyS2'
+
     @classmethod
     def _make_boot_files(cls, uboot_parts_dir, boot_cmd, chroot_dir,
                          boot_dir, boot_script, boot_device_or_file):
+        cls.set_appropriate_serial_tty(chroot_dir)
         install_omap_boot_loader(chroot_dir, boot_dir)
         make_uImage(
             cls.load_addr, uboot_parts_dir, cls.kernel_suffix, boot_dir)
@@ -121,8 +164,9 @@
 
 class BeagleConfig(OmapConfig):
     uboot_flavor = 'omap3_beagle'
-    extra_serial_opts = 'console=tty0 console=ttyO2,115200n8'
-    live_serial_opts = 'serialtty=ttyO2'
+    _serial_tty = 'ttyO2'
+    _extra_serial_opts = 'console=tty0 console=%s,115200n8'
+    _live_serial_opts = 'serialtty=%s'
     kernel_addr = '0x80000000'
     initrd_addr = '0x81600000'
     load_addr = '0x80008000'
@@ -135,7 +179,8 @@
 
 class OveroConfig(OmapConfig):
     uboot_flavor = 'omap3_overo'
-    extra_serial_opts = 'console=tty0 console=ttyO2,115200n8'
+    _serial_tty = 'ttyO2'
+    _extra_serial_opts = 'console=tty0 console=%s,115200n8'
     kernel_addr = '0x80000000'
     initrd_addr = '0x81600000'
     load_addr = '0x80008000'
@@ -147,8 +192,9 @@
 
 class PandaConfig(OmapConfig):
     uboot_flavor = 'omap4_panda'
-    extra_serial_opts = 'console=tty0 console=ttyO2,115200n8'
-    live_serial_opts = 'serialtty=ttyO2'
+    _serial_tty = 'ttyO2'
+    _extra_serial_opts = 'console=tty0 console=%s,115200n8'
+    _live_serial_opts = 'serialtty=%s'
     kernel_addr = '0x80200000'
     initrd_addr = '0x81600000'
     load_addr = '0x80008000'
@@ -173,8 +219,9 @@
 
 
 class Ux500Config(BoardConfig):
-    extra_serial_opts = 'console=tty0 console=ttyAMA2,115200n8'
-    live_serial_opts = 'serialtty=ttyAMA2'
+    serial_tty = 'ttyAMA2'
+    extra_serial_opts = 'console=tty0 console=%s,115200n8' % serial_tty
+    live_serial_opts = 'serialtty=%s' % serial_tty
     kernel_addr = '0x00100000'
     initrd_addr = '0x08000000'
     load_addr = '0x00008000'
@@ -197,8 +244,9 @@
 
 
 class Mx51evkConfig(BoardConfig):
-    extra_serial_opts = 'console=tty0 console=ttymxc0,115200n8'
-    live_serial_opts = 'serialtty=ttymxc0'
+    serial_tty = 'ttymxc0'
+    extra_serial_opts = 'console=tty0 console=%s,115200n8' % serial_tty
+    live_serial_opts = 'serialtty=%s' % serial_tty
     kernel_addr = '0x90000000'
     initrd_addr = '0x90800000'
     load_addr = '0x90008000'
@@ -229,8 +277,9 @@
 
 class VexpressConfig(BoardConfig):
     uboot_flavor = 'ca9x4_ct_vxp'
-    extra_serial_opts = 'console=tty0 console=ttyAMA0,38400n8'
-    live_serial_opts = 'serialtty=ttyAMA0'
+    serial_tty = 'ttyAMA0'
+    extra_serial_opts = 'console=tty0 console=%s,38400n8' % serial_tty
+    live_serial_opts = 'serialtty=%s' % serial_tty
     kernel_addr = '0x60008000'
     initrd_addr = '0x81000000'
     load_addr = kernel_addr

=== modified file 'linaro_media_create/tests/test_media_create.py'
--- linaro_media_create/tests/test_media_create.py	2011-01-27 18:43:55 +0000
+++ linaro_media_create/tests/test_media_create.py	2011-01-28 15:52:27 +0000
@@ -166,60 +166,86 @@
         def mock_func_creator(name):
             return lambda *args, **kwargs: self.funcs_calls.append(name)
 
-        for name in dir(linaro_media_create.boards):
-            attr = getattr(linaro_media_create.boards, name)
+        for name in dir(boards):
+            attr = getattr(boards, name)
             if type(attr) == types.FunctionType:
                 self.useFixture(MockSomethingFixture(
-                    linaro_media_create.boards, name, mock_func_creator(name)))
+                    linaro_media_create.boards, name,
+                    mock_func_creator(name)))
+
+    def mock_set_appropriate_serial_tty(self, config):
+
+        def set_appropriate_serial_tty_mock(cls, chroot_dir):
+            cls.serial_tty = cls._serial_tty
+
+        self.useFixture(MockSomethingFixture(
+            config, 'set_appropriate_serial_tty',
+            classmethod(set_appropriate_serial_tty_mock)))
 
     def make_boot_files(self, config):
         config.make_boot_files('', False, False, [], '', '', '', '', '')
 
     def test_vexpress_steps(self):
-        config = linaro_media_create.boards.VexpressConfig
-        self.make_boot_files(config)
+        self.make_boot_files(boards.VexpressConfig)
         expected = ['make_uImage', 'make_uInitrd']
         self.assertEqual(expected, self.funcs_calls)
 
     def test_mx51evk_steps(self):
-        config = linaro_media_create.boards.Mx51evkConfig
-        self.make_boot_files(config)
+        self.make_boot_files(boards.Mx51evkConfig)
         expected = [
             'install_mx51evk_boot_loader', 'make_uImage', 'make_uInitrd',
             'make_boot_script']
         self.assertEqual(expected, self.funcs_calls)
 
     def test_ux500_steps(self):
-        config = linaro_media_create.boards.Ux500Config
-        self.make_boot_files(config)
+        self.make_boot_files(boards.Ux500Config)
         expected = ['make_uImage', 'make_uInitrd', 'make_boot_script']
         self.assertEqual(expected, self.funcs_calls)
 
     def test_panda_steps(self):
-        config = linaro_media_create.boards.PandaConfig
-        self.make_boot_files(config)
+        self.mock_set_appropriate_serial_tty(boards.PandaConfig)
+        self.make_boot_files(boards.PandaConfig)
         expected = [
             'install_omap_boot_loader', 'make_uImage', 'make_uInitrd',
             'make_boot_script', 'make_boot_ini']
         self.assertEqual(expected, self.funcs_calls)
 
     def test_beagle_steps(self):
-        config = linaro_media_create.boards.BeagleConfig
-        self.make_boot_files(config)
+        self.mock_set_appropriate_serial_tty(boards.BeagleConfig)
+        self.make_boot_files(boards.BeagleConfig)
         expected = [
             'install_omap_boot_loader', 'make_uImage', 'make_uInitrd',
             'make_boot_script', 'make_boot_ini']
         self.assertEqual(expected, self.funcs_calls)
 
     def test_overo_steps(self):
-        config = linaro_media_create.boards.OveroConfig
-        self.make_boot_files(config)
+        self.mock_set_appropriate_serial_tty(boards.OveroConfig)
+        self.make_boot_files(boards.OveroConfig)
         expected = [
             'install_omap_boot_loader', 'make_uImage', 'make_uInitrd',
             'make_boot_script', 'make_boot_ini']
         self.assertEqual(expected, self.funcs_calls)
 
 
+class TestFixForBug697824(TestCaseWithFixtures):
+
+    def test_set_appropriate_serial_tty_old_kernel(self):
+        tempdir = self.useFixture(CreateTempDirFixture()).tempdir
+        boot_dir = os.path.join(tempdir, 'boot')
+        os.makedirs(boot_dir)
+        open(os.path.join(boot_dir, 'vmlinuz-2.6.35-23-foo'), 'w').close()
+        boards.BeagleConfig.set_appropriate_serial_tty(tempdir)
+        self.assertEquals('ttyS2', boards.BeagleConfig.serial_tty)
+
+    def test_set_appropriate_serial_tty_new_kernel(self):
+        tempdir = self.useFixture(CreateTempDirFixture()).tempdir
+        boot_dir = os.path.join(tempdir, 'boot')
+        os.makedirs(boot_dir)
+        open(os.path.join(boot_dir, 'vmlinuz-2.6.36-13-foo'), 'w').close()
+        boards.BeagleConfig.set_appropriate_serial_tty(tempdir)
+        self.assertEquals('ttyO2', boards.BeagleConfig.serial_tty)
+
+
 class TestGetSfdiskCmd(TestCase):
 
     def test_default(self):
@@ -284,7 +310,12 @@
         self.assertEqual(expected, boot_cmd)
 
     def test_beagle(self):
-        boot_cmd = board_configs['beagle']._get_boot_cmd(
+        # XXX: To fix bug 697824 we have to change class attributes of our
+        # OMAP board configs, and some tests do that so to make sure they
+        # don't interfere with us we'll reset that before doing anything.
+        config = board_configs['beagle']
+        config.serial_tty = config._serial_tty
+        boot_cmd = config._get_boot_cmd(
             is_live=False, is_lowmem=False, consoles=None,
             rootfs_uuid="deadbeef")
         expected = (