Code review comment for lp:~liuyq0307/lava-dispatcher/update-mount-partitions

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"?