Merge lp:~liuyq0307/lava-dispatcher/update-mount-partitions into lp:lava-dispatcher

[Yongqin Liu]

 update the source for mount partitions for aosp build and JB4.2 build
Updated according to the review comment.

[Andy Doan (doanac)]

This approach is actually a nice improvement from what we have now. Some
small comments:

On 11/29/2012 05:01 AM, Yongqin Liu wrote:
> === modified file 'lava_dispatcher/config.py'
> --- lava_dispatcher/config.py 2012-11-16 00:47:20 +0000
> +++ lava_dispatcher/config.py 2012-11-29 11:00:29 +0000
> @@ -66,6 +66,7 @@
> sys_part_android_org = schema.IntOption()
> val = schema.StringOption()
> sdcard_mountpoint_path = schema.StringOption(default="/storage/sdcard0")
> + possible_partitions_files = schema.StringOption(default="init.rc")

Since this is an array, you should declare like:
   possible_partitions_files = schema.ListOption(default = [
       "init.partitions.rc", "fstab.partitions", "init.rc"])

> simulator_version_command = schema.StringOption()
> simulator_command = schema.StringOption()
>
> === modified file 'lava_dispatcher/default-config/lava-dispatcher/device-defaults.conf'
> --- lava_dispatcher/default-config/lava-dispatcher/device-defaults.conf 2012-11-15 14:51:05 +0000
> +++ lava_dispatcher/default-config/lava-dispatcher/device-defaults.conf 2012-11-29 11:00:29 +0000
> @@ -122,3 +122,8 @@
>
> # how long the disablesuspend script should take to complete
> #disablesuspend_timeout = 240
> +
> +# the possible partition files need to be modified for android
> +possible_partitions_files = init.partitions.rc,
> + fstab.partitions,
> + init.rc

Because of my change to config.py above the change to this file isn't
needed now.

> === modified file 'lava_dispatcher/device/master.py'
> --- lava_dispatcher/device/master.py 2012-11-21 22:07:45 +0000
> +++ lava_dispatcher/device/master.py 2012-11-29 11:00:29 +0000
> @@ -619,15 +619,24 @@
>
> # The mount partitions have moved from init.rc to init.partitions.rc
> # For backward compatible with early android build, we update both rc files
> - _update_uInitrd_partitions(session, 'init.rc')
> - _update_uInitrd_partitions(session, 'init.partitions.rc')
> + # For omapzoom and aosp and JB4.2 the operation for mounting partitions are
> + # in init.omap4pandaboard.rc and fstab.* files
> + possible_partitions_files = string_to_list(
> + session._client.config.possible_partitions_files)
> +

due to the config.py change I suggested, you don't need to
"string_to_list" call now.

> + for f in possible_partitions_files:
> + if session.is_file_exist(f):
> + _update_uInitrd_partitions(session, f)
> + # Will update the PS1 in init.rc in the following
> + # So will not cat the file here
> + if f != 'init.rc':
> + session.run("cat %s" % f, failok=True)
Why the conditional, seems like you should just print it out here and
then delete the call below that always tries to cat it?
>
> session.run(
> 'sed -i "/export PATH/a \ \ \ \ export PS1 \'%s\'" init.rc' %
> target.ANDROID_TESTER_PS1)
>
> session.run("cat init.rc")
> - session.run("cat init.partitions.rc", failok=True)
Why are you keeping the call to "cat init.rc"?

[Michael Hudson-Doyle (mwhudson)]

Yongqin Liu <email address hidden> writes:

> Yongqin Liu has proposed merging lp:~liuyq0307/lava-dispatcher/update-mount-partitions into lp:lava-dispatcher.
>
> Requested reviews:
> Linaro Validation Team (linaro-validation)
>
> For more details, see:
> https://code.launchpad.net/~liuyq0307/lava-dispatcher/update-mount-partitions/+merge/136902
>
> update the source for mount partitions for aosp build and JB4.2 build
> --
> https://code.launchpad.net/~liuyq0307/lava-dispatcher/update-mount-partitions/+merge/136902
> You are subscribed to branch lp:lava-dispatcher.
> === modified file 'lava_dispatcher/config.py'
> --- lava_dispatcher/config.py 2012-11-16 00:47:20 +0000
> +++ lava_dispatcher/config.py 2012-11-29 11:00:29 +0000
> @@ -66,6 +66,7 @@
> sys_part_android_org = schema.IntOption()
> val = schema.StringOption()
> sdcard_mountpoint_path = schema.StringOption(default="/storage/sdcard0")
> + possible_partitions_files = schema.StringOption(default="init.rc")

I'm a little confused, isn't this something that is more likely to vary
by build than by target board?

Cheers,
mwh

[Yongqin Liu (liuyq0307)]

> > + for f in possible_partitions_files:
> > + if session.is_file_exist(f):
> > + _update_uInitrd_partitions(session, f)
> > + # Will update the PS1 in init.rc in the following
> > + # So will not cat the file here
> > + if f != 'init.rc':
> > + session.run("cat %s" % f, failok=True)
> Why the conditional, seems like you should just print it out here and
> then delete the call below that always tries to cat it?
Like the comment just above the condition,
Here is a modification about the PS1 below, so keep the condition there.
Now I deleted the condition by moving the PS1 modification before this

> > session.run(
> > 'sed -i "/export PATH/a \ \ \ \ export PS1 \'%s\'" init.rc' %
> > target.ANDROID_TESTER_PS1)
> >
> > session.run("cat init.rc")
> > - session.run("cat init.partitions.rc", failok=True)
> Why are you keeping the call to "cat init.rc"?

[Yongqin Liu (liuyq0307)]

> schema.StringOption(default="/storage/sdcard0")
> > + possible_partitions_files = schema.StringOption(default="init.rc")
>
> I'm a little confused, isn't this something that is more likely to vary
> by build than by target board?
Yes, actually it varies by build, like ICS, JB, or others.
but will also varies by board because bhoj says that "we will be adding a cleanup to look at fstab.<ro.hardware> which would map to fstab.omap4pandaboard fstab.origen etc ."

and for us, we just need to list all the possible file there, and it will be OK I think.

[Andy Doan (doanac)]

i think its better than what we had in the first place

review: Approve

=== modified file 'lava_dispatcher/config.py'
--- lava_dispatcher/config.py	2012-11-16 00:47:20 +0000
+++ lava_dispatcher/config.py	2012-11-30 02:13:19 +0000
@@ -66,6 +66,9 @@
     sys_part_android_org = schema.IntOption()
     val = schema.StringOption()
     sdcard_mountpoint_path = schema.StringOption(default="/storage/sdcard0")
+    possible_partitions_files = schema.ListOption(default=["init.partitions.rc",
+                                                           "fstab.partitions",
+                                                           "init.rc"])
 
     simulator_version_command = schema.StringOption()
     simulator_command = schema.StringOption()

=== modified file 'lava_dispatcher/device/master.py'
--- lava_dispatcher/device/master.py	2012-11-21 22:07:45 +0000
+++ lava_dispatcher/device/master.py	2012-11-30 02:13:19 +0000
@@ -592,7 +592,7 @@
     data_part_lava = session._client.config.data_part_android
 
     session.run(
-        'sed -i "/mount ext4 \/dev\/block\/mmcblk0p%s/d" %s'
+        'sed -i "/\/dev\/block\/mmcblk0p%s/d" %s'
         % (cache_part_org, rc_filename), failok=True)
 
     session.run('sed -i "s/mmcblk0p%s/mmcblk0p%s/g" %s'
@@ -617,17 +617,20 @@
     session.run('mv uInitrd.data ramdisk.cpio.gz')
     session.run('gzip -d -f ramdisk.cpio.gz; cpio -i -F ramdisk.cpio')
 
+    session.run(
+        'sed -i "/export PATH/a \ \ \ \ export PS1 \'%s\'" init.rc' %
+        target.ANDROID_TESTER_PS1)
+
     # The mount partitions have moved from init.rc to init.partitions.rc
     # For backward compatible with early android build, we update both rc files
-    _update_uInitrd_partitions(session, 'init.rc')
-    _update_uInitrd_partitions(session, 'init.partitions.rc')
-
-    session.run(
-        'sed -i "/export PATH/a \ \ \ \ export PS1 \'%s\'" init.rc' %
-        target.ANDROID_TESTER_PS1)
-
-    session.run("cat init.rc")
-    session.run("cat init.partitions.rc", failok=True)
+    # For omapzoom and aosp and JB4.2 the operation for mounting partitions are
+    # in init.omap4pandaboard.rc and fstab.* files
+    possible_partitions_files = session._client.config.possible_partitions_files
+
+    for f in possible_partitions_files:
+        if session.is_file_exist(f):
+            _update_uInitrd_partitions(session, f)
+            session.run("cat %s" % f, failok=True)
 
     session.run('cpio -i -t -F ramdisk.cpio | cpio -o -H newc | \
             gzip > ramdisk_new.cpio.gz')