1007053 - Implement "canBeFormatted", "canBeMounted" and "canBeShared" API for device storage

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Alan Huang]

According to bug 943825 comment 9, we need these APIs to expose more volume configurations. Then, Settings App can decide how to deal with it.

Comment 1

[Dave Hylands [:dhylands]]

Bug 1006328 implements isShareable, which implies canBeMounted and canBeFormatted.

Comment 3

[Dave Hylands [:dhylands]]

Alan - I have a patch that implements isShareable from bug 1006328. Is it ok for me to take ownership of this?

Comment 4

[Dave Hylands [:dhylands]]

adb (running on the host) talks to the adb daemon running on the phone.

The adb daemon comes from the base image.

Something weird is happening if just running adb causes CPU load to increase, and all we do is reflash b2g.

Also, make sure that you're running adb 1.0.31 on the host. Some things in adb changed in jb (which the flame is based on), and using an older adb on the host may also be a factor here (not sure - just something to check on).

Comment 5

[Dave Hylands [:dhylands]]

I built 1.4 and flashed with the 10F base image.

I see this:

  PID   TID PR CPU% S     VSS     RSS PCY UID      Thread          Proc
  308   484  0  43% R 168112K  71464K     root     Gecko_IOThread  /system/b2g/b2g
  308   308  0  32% S 168112K  71464K     root     b2g             /system/b2g/b2g
  918   918  1   1% R   1268K    600K     root     top             top
   96    96  0   0% S      0K      0K     root     ksmd            

just doing an adb shell top -m 10 -t

I don't need to do a logcat.

The logcat output seems incredibly short, so there is definitely something wrong.

I did a ./flash.sh of 1.4 and saw the above. I did a ./flash.sh of master and the b2g process only takes about 1 or 2% CPU and logcat shows reams of output.

Comment 6

[Dave Hylands [:dhylands]]

Sorry my last 2 comments were totally in the wrong bug

Comment 7

[Dave Hylands [:dhylands]]

Attachment: Pt 1 - Make isFake be propagated properly to child processes

While trying to implement this, I noticed that the isFake attribute in the Volume wasn't being propagated to the child processes.

Comment 8

[Dave Hylands [:dhylands]]

Attachment: Pt 2 - Add canBeShared, canBeMounted and canBeFormatted attributes to devicestororage webidl

Comment 9

[Dave Hylands [:dhylands]]

Attachment: Pt 3 - Implement canBeMounted/Formatted/Shared

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8422008 [details] [diff] [review]
Pt 2 - Add canBeShared, canBeMounted and canBeFormatted attributes to devicestororage webidl

r=me

Comment 11

[Dave Hylands [:dhylands]]

Attachment: Pt 4 - gaia - Change to ds-test to show canBeShared attribute

Marking as r+ since this is a trivial change, and it only impacts a test program.

Comment 12

[Dave Hylands [:dhylands]]

https://hg.mozilla.org/integration/b2g-inbound/rev/44a39f5676c9
https://hg.mozilla.org/integration/b2g-inbound/rev/e6e413f8e09c
https://hg.mozilla.org/integration/b2g-inbound/rev/bfe626cd4f61

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/44a39f5676c9
https://hg.mozilla.org/mozilla-central/rev/e6e413f8e09c
https://hg.mozilla.org/mozilla-central/rev/bfe626cd4f61

Comment 14

[Dave Hylands [:dhylands]]

https://github.com/mozilla-b2g/gaia/commit/454f07816b9a683d15b39ebad21a21af58bdcfde

Comment 15

[[e-mail], [e-mail]]

Hi Dave, I'm working on bug 943825. And I try to use the three attributes since API is landed and supported already. The attributes provide some limitation for specific devices(Nexus 4/5). But I cannot reach guidance from MDN(https://developer.mozilla.org/en-US/docs/Web/API/DeviceStorage). Just Q&A Gecko devs from the code base. Shell we add some document for the new attributes? That would be more convenient for developments. Thanks.

Comment 16

[Dave Hylands [:dhylands]]

Yeah - I should have flagged this as dev_doc_needed. Now that I think of it, there is probably some others that need this too.

Comment 17

[Dave Hylands [:dhylands]]

canBeFormatted/Shared/Mounted should return false on Nexus 4/5.

Comment 18

[[e-mail], [e-mail]]

Hi Dave, I have a question about the definition of 'canBeMounted' attribute. There is an internal storage in device 'flame'. And, the 'canBeMounted' attribute will return true. But, this internal storage is not removable form the hardware. Could we use the attribute to identify a removable/unremovable storage? Per spec. of SDCard management, we won't provide mount/unmount functionality for an unremovable storage. Do we have some way to identify the 'removable' attribute? Thanks.

Comment 19

[Dave Hylands [:dhylands]]

Internal storage on the flame is formattable, but not removable.

Unfortunately, we don't have a way right now to determine if a media is removable or not, and I haven't figured out a way to ddo it programatically.

I think we would need to add something into /system/etc/volume.cfg to indicate if a given volume is removable or not.

For the internal storage, you still need to unmount before you can format don't you?

Comment 20

[[e-mail], [e-mail]]

So, shell we provide 'canBeRemovable' or 'canBePlugged' attribute for decision functionality mount/unmount for internal storage? Otherwise, we still need some 'heuristic' judgement to make sure internal/external(https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/media_storage.js#L345-L353). Then, we always disable mount/unmount for the internal storage. This is a little be tricky and nontrivial for implementation. I'm wondering some of special case we don't take care.

Since platform have supported format() API via bug 841660, I don't need to unmount it before format. The API will help to unmount storage before format. This is a friendly design for Gaia developer.:)

Comment 21

[[e-mail], [e-mail]]

BTW, some of the devices, which removable external storage is under the battery. It means a user have to shut down the device. Then, he/she removes the SDCard from the SD-Slot after unplugged the battery. I would like to suggest to considerate this case in UX. +Jenny for discussion.

This kinds of hardware limitation might be hard to expose to Gaia/Gecko side? Dave, do you have any idea for this section?

Comment 22

[Dave Hylands [:dhylands]]

What's the use case for knowing that the media is removable or not?

If I were going to create a new API, I would use isRemovable (not canBeRemovable). (isRemovable describes an attribute, canBe describes the ability to perform an action - a subtle distinction)

Whether you consider an sdcard under the battery as removable or not really depends on the use case.

If its just to expose mount/unmount, then I would consider an sdcard under the battery as non-removable (i.e. can't be removed while the phone is powered on).

The other option is to not add anything new to the API and have gaia use canBeMounted to determine if the storage area should expose mount/unmount.

Currently, canBeFormatted/Mounted/Shared all have the same value, but we could add an entry to /system/etc/volume.cfg that causes canBeMounted to return false for sdcards which can't be removed while the phone is powered on.

Comment 23

[[e-mail], [e-mail]]

(In reply to Dave Hylands [:dhylands] from comment #22)
> What's the use case for knowing that the media is removable or not?
The use case is defined in SD Card spec., page 9, 'Note' in right corner.(https://bugzilla.mozilla.org/show_bug.cgi?id=921105#c22)

Note:
Mount SD Card/Unmount SD Card/Format SD Card may not show if the device or storage doesn’t support these functions.

Per discussion with UX designer, they will not provide mount/unmount functionality for non-removable media.
> 
> If I were going to create a new API, I would use isRemovable (not
> canBeRemovable). (isRemovable describes an attribute, canBe describes the
> ability to perform an action - a subtle distinction)
'isRemovable' is pretty clear for me.:)

> 
> Whether you consider an sdcard under the battery as removable or not really
> depends on the use case.
> 
> If its just to expose mount/unmount, then I would consider an sdcard under
> the battery as non-removable (i.e. can't be removed while the phone is
> powered on).
> 
> The other option is to not add anything new to the API and have gaia use
> canBeMounted to determine if the storage area should expose mount/unmount.
> 
> Currently, canBeFormatted/Mounted/Shared all have the same value, but we
> could add an entry to /system/etc/volume.cfg that causes canBeMounted to
> return false for sdcards which can't be removed while the phone is powered
> on.

Both of above options are okay for me. But, if we configure 'canBeMounted' attribute for 'isRemovable' use case, the storage will not be able to mount/unmount one day.(Might the user story of settings::media storage be reorganised one day, we could need 'canBeMounted' attribute. Because the attribute is corresponding to storage mount/unmount ability, not on use case.) Base on my view, I would like the new attribute 'isRemovable'. This is more accurate for me.

Comment 24

[Dave Hylands [:dhylands]]

I'm fine with adding isRemovable.

File a new bug, cc me, and add the dev-doc-needed keyword to the bug.

Comment 25

[[e-mail], [e-mail]]

Thanks for your suggestion. Let's track via bug 1033952.