1113518 - [gonk-l] [nexus-5] MTP sometimes doesn't work on Nexus 5

Status: RESOLVED FIXED

(Firefox OS Graveyard :: MTP/UMS, defect)

Description

[Askeing Yen[:askeing][:fyen]]

MTP doesn't work.

Build Info:
Build ID               20141218151450
Gaia Revision          58734e8a48157f99d5b733412b600c2e04c954fe
Gaia Date              2014-12-18 05:21:04
Gecko Revision         n/a
Gecko Version          37.0a1
Device Name            hammerhead
Firmware(Release)      5.0
Firmware(Incremental)  eng.askeing.20141218.150855
Firmware Date          Thu Dec 18 15:10:01 CST 2014
Bootloader             HHZ12d

Comment 1

[Askeing Yen[:askeing][:fyen]]

MTP sometimes dosen't work on Nexus 5, it's not stable.

It seems like a bug, not a porting blocker issue.
Alphan will help to find the root cause.

Comment 2

[Alphan Chen [:alchen]]

From log, when error happened, the state of automounter keep in "UMS_CONFIGURING".
Since Nexus 5 is a fuse architecture, we cannot use UMS on Nexus 5.

Comment 3

[Alphan Chen [:alchen]]

Attachment: automounter.log

Here is the automounter and mtp log.

Comment 4

[Steven Yang [:styang]]

Hi Alphan,

Are you gonna take it? would also need ETA for better understanding the risk. thanks.

Comment 5

[Alphan Chen [:alchen]]

Yes, I will take this bug.

Comment 6

[Alphan Chen [:alchen]]

The following is the reason of why MTP cannot restart successfully.
When restarting MozMtpServer, "/dev/mtp_usb" cannot be opened. (the error code is "Device or resource busy")


12-18 15:21:40.937   193   423 I AutoMounter: Changing mode from 'Enable-MTP' to 'Disable'
12-18 15:21:40.937   193   423 I AutoMounter: Stopping MtpServer
12-18 15:21:40.937   193   423 I MozMtp  : ~MozMtpStorage: Storage destructed for Volume sdcard mStorageID 0x00010001
12-18 15:21:40.937   193   423 I MozMtp  : StorageUnavailable: Removing mStorageID 0x00010001 from MtpServer
12-18 15:21:40.937   193   423 I MozMtp  : StorageUnavailable: Removing mStorageID 0x00010001 from MozMtpDatabse
12-18 15:21:40.937   193   423 I AutoMounter: AutoMounter state changed from MTP_STARTED to IDLE
12-18 15:21:40.937   193   423 I AutoMounter: UpdateState: ums:A0C0E0 mtp:A1C1E0 mode:0 usb:1 tryToShare:0 state:IDLE
12-18 15:21:40.937   193   423 I AutoMounter: UpdateState: Volume sdcard is Mounted and inserted @ /storage/emulated/legacy gen 1 locked 0 sharing x
12-18 15:21:40.937   193   423 I AutoMounterSetting: Changing status from 'Enabled' to 'Disabled'

12-18 15:21:47.250   193   423 I AutoMounter: Changing mode from 'Disable' to 'Enable-MTP'
12-18 15:21:47.250   193   423 I AutoMounter: Starting MtpServer
12-18 15:21:47.250   193   423 E MozMtp  : Init: open of '/dev/mtp_usb' failed

Comment 7

[Alphan Chen [:alchen]]

/dev/mtp_usb is busy when server is still running.


class MtpServerRunnable : public nsRunnable
{
...
  nsresult Run()
  {
     ...
    MTP_LOG("MozMtpServer started");
    server->run();
    MTP_LOG("MozMtpServer finished");
     ...
  }
...
private:
  nsRefPtr<MozMtpServer> mMozMtpServer;
  ScopedClose mMtpUsbFd; // We want to hold this open while the server runs
}

Comment 8

[Alphan Chen [:alchen]]

Attachment: [automounter] Add a case for fuse and add some useful logs-1231

The patch contain the three parts:

1. Add one case for disabling MTP on fuse device.

2. Need to set sMozMtpServer as nullptr when "sMozMtpServer->Init()" failed.

3. Print the reason of opening "/dev/mtp_usb" failed.

Comment 9

[Alphan Chen [:alchen]]

Attachment: [automounter] Add a case for fuse and add some useful logs-0105

Update a new version.

Comment 10

[Dave Hylands [:dhylands]]

Comment on attachment 8543743 [details] [diff] [review]
[automounter] Add a case for fuse and add some useful logs-0105

Review of attachment 8543743 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good - just a couple minor nits to address.

::: dom/system/gonk/AutoMounter.cpp
@@ +105,5 @@
>  #define USB_FUNC_MTP    "mtp"
>  #define USB_FUNC_NONE   "none"
>  #define USB_FUNC_RNDIS  "rndis"
>  #define USB_FUNC_UMS    "mass_storage"
> +#define USB_FUNC_DFT    "default"

nit: Since we're introducing a "fake" USB Function, let's choose a name like ~~default~~ or something that isn't likely to ever be used in real life.

nit: Let's use USB_FUNC_DEFAULT rather than abbreviating to DFT

Comment 11

[Alphan Chen [:alchen]]

Attachment: [automounter] Add a case for fuse and add some useful logs. r=dhylands

Here is the try server result.
It looks fine.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bf08c0362bfc

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f3b742b23432

Comment 14

[Hermes Cheng (inactive after July 27, 2015)]

Could you please also land the patch to v2.2? Thanks.

Comment 15

[Alphan Chen [:alchen]]

Sure. I will do the test by myself first and request the approval for v2.2 later.

Comment 16

[Alphan Chen [:alchen]]

Comment on attachment 8555140 [details] [diff] [review]
[automounter] Add a case for fuse and add some useful logs. r=dhylands

[Approval Request Comment]
Bug caused by (feature/regressing bug #): It is a lack of MTP implementation.
User impact if declined: The MTP functionality may be broken when doing enable and disable frequently.
Testing completed: MTP testcases(http://goo.gl/pZIds0) are passed. 
Risk to taking this patch (and alternatives if risky): (low risk) It is a patch for the lack.
String or UUID changes made by this patch: none

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/6eb3d90b72fc