[Flame KK] Trying to apply a update without isOSUpdate=true fails with error code 43

RESOLVED FIXED in Firefox 34

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: amac, Assigned: amac)

Tracking

unspecified
2.1 S4 (12sep)
x86_64
Linux

Firefox Tracking Flags

(blocking-b2g:2.0+, firefox33 wontfix, firefox34 fixed, firefox35 fixed, b2g-v2.0 fixed, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

STR:
1. Build a user or userdebug flame KK build over the 180 build. On this build, set 
pref("app.update.url", "http://SOME_SERVER/APATH/update.xml");
2. Flash that build

3. Prepare a FOTA with:
./build.sh gecko-update-full
3. Create the update.xml as:

./tools/update-tools/build-update-xml.py -v 99.0 -V 99.0 -o update.xml -u http://SOME_SERVER/APATH/update.xml ${PATH_TO_MAR}

4. Copy the update.xml and .mar files to your SOME_SERVER on APATH 

5. Ensure that /system is in ro mode (or just reboot the phone) if you're not sure

6. Search for updates on the phone and download the update.

Expected:

The update is downloaded and the user is offered to apply it

Actual: The update is downloaded, but the user isn't offered the option to apply it. Instead, logcat shows: 

I/Gecko   (  681): *** AUS:SVC readStatusFile - status: failed: 43, path: /data/local/updates/0/update.status
I/Gecko   (  681): UpdatePrompt: Update error, state: failed, errorCode: 43
I/Gecko   (  681): UpdatePrompt: Setting gecko.updateStatus: Install Pending
I/Gecko   (  681): *** AUS:SVC UpdateManager:refreshUpdateStatus - Notifying observers that the update was staged. state: pending, status: failed: 43


And the /data/local/updates/0/update.log shows:

Performing a staged update
MOZ_UPDATER_PRIO=19/-1000/3/0
GonkAutoMounter: Error mounting /system as read-write: Permission denied

GonkAutoMounter: Could not remount /system as read-write.
Since 

adb shell mount -o rw,remount /system 

worked but the mount in automounter didn't I just checked out what could be missing. And turns out we have to set the device block file to RW before remounting (and actually only have to do that once per boot, which explains why if I remounted as rw and then as ro it worked).
Attachment #8488579 - Flags: review?(marshall)
Note that as per the previous comment, the STR is slighty wrong. If the mount permissions have been touched at all, then the device *must* be rebooted for the OTA to fail.
Comment on attachment 8488579 [details] [diff] [review]
Set the device block file as RW before remounting

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Partial OTAs cannot be applied.
Testing completed: Manually.
Risk to taking this patch (and alternatives if risky): I believe this is very low risk. In the worst case we won't be worse than now.
String or UUID changes made by this patch: None
Attachment #8488579 - Flags: approval-mozilla-b2g32?
Comment on attachment 8488579 [details] [diff] [review]
Set the device block file as RW before remounting

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Partial OTAs cannot be applied.
Testing completed: Manually.
Risk to taking this patch (and alternatives if risky): I believe this is very low risk. In the worst case we won't be worse than now.
String or UUID changes made by this patch: None
Attachment #8488579 - Flags: approval-mozilla-aurora?
[Blocking Requested - why for this release]: Without this patch we won't be able to do partial OTAs (the ones that don't require booting into recovery mode to apply).
blocking-b2g: --- → 2.0?
Comment on attachment 8488579 [details] [diff] [review]
Set the device block file as RW before remounting

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

Looks like a good approach, just a few concerns. I think it would also make sense to put the block device back in read-only mode in the |GonkAutoMounter| destructor, maybe as part of RemountSystem?

::: toolkit/mozapps/update/updater/automounter_gonk.cpp
@@ +207,5 @@
> +
> +/*
> + * Mark the given block device as read-write, using the BLKROSET ioctl.
> + */
> +static void fs_set_blk_rw(const char *blockdev) {

nit: use the upper camelcase function name style of the rest of this code? :) maybe, |SetBlockReadWrite|

@@ +216,5 @@
> +  if (fd < 0) {
> +    return;
> +  }
> +
> +  ioctl(fd, BLKROSET, &off);

can you check the return code of ioctl, and log if it fails?

it would also be good to assert that the block is now readwrite with another call to |ioctl(fd, BLKROGET, ..)|, but maybe that is pedantic?
Attachment #8488579 - Flags: review?(marshall) → review-
The only thing I didn't do was check the value with another ioctl again... Besides hoping the call to fail if it can't proceed, in any case the remount will fail if the device isn't RW, and that's controlled already.
Attachment #8488579 - Attachment is obsolete: true
Attachment #8488579 - Flags: approval-mozilla-b2g32?
Attachment #8488579 - Flags: approval-mozilla-aurora?
Attachment #8488720 - Flags: review?(marshall)
Comment on attachment 8488720 [details] [diff] [review]
V2. Includes previous review comments

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

Thanks for addressing this so quickly, it looks great!

::: toolkit/mozapps/update/updater/automounter_gonk.cpp
@@ +204,5 @@
>    return true;
>  }
>  
> +/*
> + * Mark the given block device as read-write, using the BLKROSET ioctl.

nit: just update this to reflect that the function can do read-only or read-write :)
Attachment #8488720 - Flags: review?(marshall) → review+
r=marshall
Attachment #8488720 - Attachment is obsolete: true
Attachment #8488735 - Flags: review+
Comment on attachment 8488735 [details] [diff] [review]
V3. Includes review comments

Set the device block file as RW before remounting
NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Partial OTAs cannot be applied.
Testing completed: Manually.
Risk to taking this patch (and alternatives if risky): I believe this is very low risk. In the worst case we won't be worse than now.
String or UUID changes made by this patch: None
Attachment #8488735 - Flags: approval-mozilla-b2g32?
Comment on attachment 8488735 [details] [diff] [review]
V3. Includes review comments

Set the device block file as RW before remounting

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Partial OTAs cannot be applied.
Testing completed: Manually.
Risk to taking this patch (and alternatives if risky): I believe this is very low risk. In the worst case we won't be worse than now.
String or UUID changes made by this patch: None
Attachment #8488735 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/f0d38c5711a4
Status: NEW → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Just a small clarification, for the blocking/approval. While testing the FOTAs (which are also failing on Flame ATM, see bug 1066723) I've just noticed that FOTAs can't be applied either without this patch (the process fails before rebooting into recovery).
(In reply to Antonio Manuel Amaya Calvo (:amac) from comment #13)
> Just a small clarification, for the blocking/approval. While testing the
> FOTAs (which are also failing on Flame ATM, see bug 1066723) I've just
> noticed that FOTAs can't be applied either without this patch (the process
> fails before rebooting into recovery).

Can you confirm this is a regression in 2.0 ?
I don't think this is a regression in 2.0 per se. It works in 2.0 with JB. I believe this is a regression introduced by switching to KK. But since 2.0 is going to be supported only in KK we have to fix it in 2.0 also. And maybe even in 1.4 although I haven't tested it there.
blocking-b2g: 2.0? → 2.0+
Attachment #8488735 - Flags: approval-mozilla-b2g32?
Attachment #8488735 - Flags: approval-mozilla-b2g32+
Attachment #8488735 - Flags: approval-mozilla-aurora?
Attachment #8488735 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.