919982 - Instantiate additional ril-daemon/rilproxy services for jellybean QEmu

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Emulator, defect)

Description

[Vicamo Yang [:vicamo][:vyang]]

To enable multisim/dsds development, we're going to instantiate additional rild/rilproxy services.  We're still not sure whether we should have multsim/dsds support on B2G ICS.

Comment 1

[Vicamo Yang [:vicamo][:vyang]]

Some files in jellybean code base refer to "ril-daemon":

  1) external/sepolicy/property_contexts
    - Defines property context for "ctl.ril-daemon".  Will probably skip this for emulator.

  2) system/extras/tests/ext4/android_emmc_perf_tests.sh
    - Stop ril-daemon before testing emmc performance.  We probably won't be interested in emmc performance in emulator.

  3) system/core/rootdir/init.rc
    - Defines service "ril-daemon".  Additional ril-daemon services are now emulator-only, so we're not going to touch this file.

  4) system/core/init/property_service.c
    - Defines white list of uid/gid for access to property "ctl.ril-daemon".  Same as 1).

Comment 2

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Github pull request for b2g-manifest

Comment 3

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Github pull request for device_generic_goldfish

Comment 4

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Github pull request for gaia

Comment 5

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 809888 [details]
Github pull request for device_generic_goldfish

looks good to me, r=yurenju

Comment 6

[Michael Wu [:mwu]]

Can you use an android property instead of gecko pref to specify the number of ril daemons? We should avoid gecko preferences for device specific settings.

Comment 7

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Michael Wu [:mwu] from comment #6)
> Can you use an android property instead of gecko pref to specify the number
> of ril daemons? We should avoid gecko preferences for device specific
> settings.

Sure.  I'll revise PR for device_generic_goldfish and replace PR for gaia with a Gecko patch instead.

Yuren, then it won't touches Gaia.  Sorry for bothering you.

Comment 8

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 809888 [details]
Github pull request for device_generic_goldfish

Use Android property ro.ril.numclients instead.

Comment 9

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Gecko patch

Comment 10

[Michael Wu [:mwu]]

Is ro.ril.numclients something in use already, or are we inventing it? For properties that we introduce, we usually prefix it with ro.moz. to distinguish from properties introduced by vendors or Android. We have many ro.moz.ril.* properties already.

Comment 11

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Gecko patch : v2

Address comment 10, use "ro.moz.ril.numclients" instead.

Comment 12

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Michael Wu [:mwu] from comment #10)
> Is ro.ril.numclients something in use already, or are we inventing it? For
> properties that we introduce, we usually prefix it with ro.moz. to
> distinguish from properties introduced by vendors or Android. We have many
> ro.moz.ril.* properties already.

That's a new property.  I've updated the pull request and Gecko patch.  Thank you.

Comment 13

[Vicamo Yang [:vicamo][:vyang]]

Try: emulator only, with all tests.
https://tbpl.mozilla.org/?tree=Try&rev=2b9867acc532

Comment 14

[Michael Wu [:mwu]]

Hmm, we shouldn't use ADDITIONAL_DEFAULT_PROPERTIES like that. It's unconditionally added to all builds. This isn't a problem with our builds since this goldfish device dir only exists on emulator builds, but device directories are suppose to coexist with other directories. It also adds itself to the property list in boot.img rather than system.img, which is where it really belongs. (looks like ADDITIONAL_BUILD_PROPERTIES can be used for system.img)

I don't think we need another review cycle though. r=me with this property added to https://github.com/mozilla-b2g/platform_build/blob/b2g-4.3_r2.1/target/board/generic/device.mk#L20 instead of the device directory.

Comment 15

[Michael Wu [:mwu]]

Comment on attachment 809888 [details]
Github pull request for device_generic_goldfish

r=me with the property moved to the place mentioned before.

Comment 16

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Michael Wu [:mwu] from comment #14)
> I don't think we need another review cycle though. r=me with this property
> added to
> https://github.com/mozilla-b2g/platform_build/blob/b2g-4.3_r2.1/target/board/
> generic/device.mk#L20 instead of the device directory.

I don't think that's a good idea.  |board/generic/device.mk| is inherited by all products that inherit |product/full.mk|.[1]  And most all of our physical devices inherit |product/full.mk|:

  * https://github.com/mozilla-b2g/device-gp-keon/blob/master/full_keon.mk#L17
  * https://github.com/mozilla-b2g/android-device-crespo4g/blob/master/full_crespo4g.mk#L34
  * https://github.com/mozilla-b2g/device-wasabi/blob/master/full_wasabi.mk#L10
  * https://github.com/mozilla-b2g/device-leo/blob/master/full_leo.mk#L11
  * https://github.com/mozilla-b2g/device-helix/blob/master/full_helix.mk#L9
  ...

Correct me if I'm wrong.  Thank you.

[1]: https://github.com/mozilla-b2g/platform_build/blob/b2g-4.3_r2.1/target/product/full.mk#L23

Comment 17

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/b2g-inbound/rev/04697b7a7bc4

Comment 18

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/04697b7a7bc4

Comment 19

[Michael Wu [:mwu]]

Hmm. You're right. However, the problem isn't that board/generic/device.mk is the wrong place, but that we should be inheriting from full_base_telephony.mk rather than full.mk. (ro.adb.qemud should *not* be set on devices..)

Go ahead with ADDITIONAL_BUILD_PROPERTIES and we'll sort out this problem with devices inheriting full.mk later.

Comment 20

[Vicamo Yang [:vicamo][:vyang]]

ADDITIONAL_{DEFAULT,BUILD}_PROPERTIES, in contrast to PRODUCT_{DEFAULT_,}PROPERTY_OVERRIDES, are both for properties applied to all builds unconditionally.  The only difference between them is, DEFAULT properties are in |/default.prop| and BUILD properties in |/system/build.prop|.  To avoid the problem you mentioned in comment 14, we have to use PRODUCT_*PROPERTY_OVERRIDES, which leads us to abuse of |full.mk| mentioned in comment 16.

I personally feel ADDITIONAL_DEFAULT_PROPERTIES is more like it because we have all service descriptors in |/init.goldfish.rc|, which is installed in bootimg as well.  I'd like to align these settings in the same place.  However, I also agree that's not a big deal, so we can always address this later whenever it becomes necessary.

Comment 21

[Vicamo Yang [:vicamo][:vyang]]

Merged on GitHub:
https://github.com/vicamo/b2g_manifest/commit/a08325973fdc188264e0fdb8db0bb34f87453f72
https://github.com/vicamo/b2g_device_generic_goldfish/commit/49c1bdd8b15d7759d0b9f452f9654256c0123921