Issue metadata
Sign in to add a comment
|
Autoupdate from and to build details are shown the same in update_engine log file |
||||||||||||||||||||||
Issue descriptionGoogle Chrome 64.0.3270.0 (Official Build) dev (64-bit) Revision 0 Platform 10134.0.0 (Official Build) dev-channel samus What steps will reproduce the problem? (1) Trigger AU of device from 10134.0.0 ==> 10135.0.0 (2) After download of updates and post installation, check 'From' and 'To' build details in update_engine log file What happens ? Both from and to build details are the same Attached update_engine log file.
,
Nov 17 2017
Can you describe how did you performed the update? Is this on canary builds or manual update? Also if possible, can you rollback the device and see what is the /etc/lsb-release?
,
Nov 18 2017
Re c#2: Can you describe how did you performed the update? Triggered autoupdate by giving 'autest' command from crosh terminal. Is this on canary builds or manual update? This is manual update triggered using autest command Also if possible, can you rollback the device and see what is the /etc/lsb-release? After download of updates, I did not reboot the device yet. So it is still on build 10134.0.0. /etc/lsb-release file has CHROMEOS_RELEASE_TRACK=dev-channel
,
Nov 21 2017
Ok. I looked into this problem, it seems a problem with postinst. We probably need to mount /etc in addition to other points in chromeos-postinst. I'm running some tests to confirm this. If not, I have to investigate more to find the root cause. Adding vapier@ as he recently did some of the postinst changes.
,
Nov 21 2017
postinst chroots into the new root, so there is no /etc to bind mount. the bind mounts we list there are writable/pseudo mount points.
,
Nov 21 2017
Doesn't bind mount work on simple folders too if /etc is not a writeable/pseudo mount point?
I just tried exactly what postinst does and I'm able to bind mount /etc.
mount -n --bind "/etc" "./etc"
and then I chroot in there and /etc was the mounted one.
But even that is not the issue here. The issue is that we need /etc/lsb-release from both new and old partitions, but when postinst mounts those folders and chroot to the new image, we don't have access to /etc/lsb-release of the old image anymore. From chromeos_postinst.cc:466 we have:
// If we can read in the lsb-release we are updating FROM, log it.
string lsb_contents;
if (ReadFileToString("/etc/lsb-release", &lsb_contents)) {
printf("\nFROM (rootfs):\n%s", lsb_contents.c_str());
}
...
// If we can read the lsb-release we are updating TO, log it
if (ReadFileToString(install_config.root.mount() + "/etc/lsb-release",
&lsb_contents)) {
printf("\nTO:\n%s\n", lsb_contents.c_str());
}
which needs both lsb-release files. We have to find a way to transfer the lsb-release to the new chroot. Is there any easy way to do it? We could for example copy the lsb-release into that tmp folder which we will chroot into and then read it in chromeos_postinst.cc:468:
if (ReadFileToString("/etc/lsb-release", &lsb_contents)) { ->
if (ReadFileToString("/lsb-release", &lsb_contents)) {
Does that sound reasonable or I'm doing something wrong?
,
Nov 22 2017
Sorry, simple copy doesn't work since we mount the partition as read only. I'll think about it more. Any other ideas are welcomed.
,
Nov 22 2017
yes, we could bind mount the old rootfs's /etc over the new rootfs's /etc ... my point is that's wrong and not what we want. we already have access to the /etc that matches the new rootfs simply by virtue of chrooting into the new rootfs. i think postinst making any assumptions about the state of / is bad. any paths it operates on should only be ones explicitly passed into it on the cli. it makes it a lot easier to keep track of and avoid future bugs like this slipping past. if UE wants specific details, couldn't it just log them itself ? let the postinst script log the /etc/lsb-release in the path it was told to run in only, and have UE itself log the existing /etc/lsb-release file. the update code also implicitly looks in /mnt/stateful_partition/etc/lsb-release btw.
,
Nov 22 2017
Ok. That sounds like a winner. I'll upload some patches to fix it. Thanks Mike.
,
Nov 22 2017
I looked deeper into the code and it seems the postinst needs the source version in order to fix an old FS corruption problem which is for versions less than "0.10.156.2" https://chromium.git.corp.google.com/chromiumos/platform2/+/619471e97b6197290448c268f9d51974a9ac193f/installer/chromeos_postinst.cc#251 Do we even support devices with this old version anymore? If not, can I get rid of that code? If yes, would passing the source version as a parameter to postinst be fine? I don't really prefer this because adding an input parameter to postinst will have no effect on the devices with UE that don't pass the parameter (older ones). This will potentially cause the postinst to fail. So this approach is probably no brainer. I think at the end we have to have the source lsb-release somewhere for the postinst to read from. As vapie@ mentioned, the UE reads from /mnt/stateful_partition/etc/lsb-release which is quite true, but I don't have enough background on this file. I cannot find an instance of it on my devices. Does anyone know if it has the exact same content as /etc/lsb-release? I don't think that is the case since there are places in UE that reads key HWID_OVERRIDE from it which doesn't belong in the system lsb-release. Can we just copy the source lsb-release somewhere in the stateful partition and read it from there in postinst? and possibly delete it afterwards? cc'ing dgarrett@ since he has worked on this in the past too.
,
Nov 23 2017
there shouldn't be a need to support R10 versions anymore. Lumpy is the oldest device we support currently and it was released on ~R17. lets prune any such code.
,
Nov 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/94e8bd508e9f6948c77654cb7003d7c8dc8a3324 commit 94e8bd508e9f6948c77654cb7003d7c8dc8a3324 Author: Amin Hassani <ahassani@google.com> Date: Tue Nov 28 23:44:24 2017 installer: Remove an old FS corruption patch for version 10 and lower We do not needs support for this patching anymore. Besides, this patching needs the source image version and we are in the process of depricating passing lsb-release of the source image to the postinst. So it is a good place to prune this code. BUG= chromium:786225 TEST=unittest pass; precq pass; Change-Id: I82726850d8d5eab3e4933723c009a7396fb64669 Reviewed-on: https://chromium-review.googlesource.com/791455 Commit-Ready: Amin Hassani <ahassani@chromium.org> Tested-by: Amin Hassani <ahassani@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/94e8bd508e9f6948c77654cb7003d7c8dc8a3324/installer/inst_util_unittest.cc [modify] https://crrev.com/94e8bd508e9f6948c77654cb7003d7c8dc8a3324/installer/inst_util.cc [modify] https://crrev.com/94e8bd508e9f6948c77654cb7003d7c8dc8a3324/installer/inst_util.h [modify] https://crrev.com/94e8bd508e9f6948c77654cb7003d7c8dc8a3324/installer/chromeos_postinst.cc
,
Nov 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/cc8b451b70b61a762418c92ab0a3def6e74c26e7 commit cc8b451b70b61a762418c92ab0a3def6e74c26e7 Author: Amin Hassani <ahassani@google.com> Date: Wed Nov 29 06:27:25 2017 installer: Stop printing source lsb-release in postinst postinst should not try to print the source lsb-release because it does not have access to it. We will show the source lsb-release in the update_engine itself. BUG= chromium:786225 TEST=cros flash and update_engine.log does not show the source lsb-release. Change-Id: Ic548b911b7a7ae6544352e6a2ef0808e13fb1e30 Reviewed-on: https://chromium-review.googlesource.com/791456 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Tested-by: Amin Hassani <ahassani@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/cc8b451b70b61a762418c92ab0a3def6e74c26e7/installer/chromeos_postinst.cc
,
Nov 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/741eca3304a35ef55eac49cefe2ce6b5756ed401 commit 741eca3304a35ef55eac49cefe2ce6b5756ed401 Author: Amin Hassani <ahassani@google.com> Date: Wed Nov 29 23:11:56 2017 installer: print uname when doing postinst It is a good idea to know which kernel version is running the postinst. So we print the uname before doing the main job of the postinst for future debugging purposes. BUG= chromium:786225 TEST=cros flash and update_engine.log shows the uname. Change-Id: I66ed1a1337fac0404d922e9c66fcd963ec2248c4 Reviewed-on: https://chromium-review.googlesource.com/794060 Commit-Ready: Amin Hassani <ahassani@chromium.org> Tested-by: Amin Hassani <ahassani@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/741eca3304a35ef55eac49cefe2ce6b5756ed401/installer/inst_util_unittest.cc [modify] https://crrev.com/741eca3304a35ef55eac49cefe2ce6b5756ed401/installer/inst_util.cc [modify] https://crrev.com/741eca3304a35ef55eac49cefe2ce6b5756ed401/installer/inst_util.h [modify] https://crrev.com/741eca3304a35ef55eac49cefe2ce6b5756ed401/installer/chromeos_postinst.cc
,
Dec 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/aosp/platform/system/update_engine/+/afd8cea0060751f969f7d4d20b4c4100ff37626d commit afd8cea0060751f969f7d4d20b4c4100ff37626d Author: Amin Hassani <ahassani@google.com> Date: Tue Dec 19 04:33:04 2017 update_engine: Log the rootfs and stateful partition's lsb-release postinst cannot not do this anymore, so we need to log it in the UE itself. BUG= chromium:786225 TEST=cros_flash two times and made sure they are logged. Change-Id: I2fbc593f55e6425127de283c78e4170460bac0d9 Reviewed-on: https://chromium-review.googlesource.com/791515 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Tested-by: Amin Hassani <ahassani@chromium.org> Reviewed-by: Ben Chan <benchan@chromium.org> Reviewed-by: Alex Deymo <deymo@google.com> [modify] https://crrev.com/afd8cea0060751f969f7d4d20b4c4100ff37626d/image_properties_chromeos.cc [modify] https://crrev.com/afd8cea0060751f969f7d4d20b4c4100ff37626d/image_properties_android.cc [modify] https://crrev.com/afd8cea0060751f969f7d4d20b4c4100ff37626d/image_properties.h [modify] https://crrev.com/afd8cea0060751f969f7d4d20b4c4100ff37626d/update_attempter.cc
,
Jan 2 2018
This should be fixed by now. I just checked a canary build and it shows different lsb-release. Feel free to verify it. I'm marking this as fixed.
,
Jan 2 2018
Verified lsb-release file on AU from 10257.0.0 => 10272.0.0 |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by abod...@chromium.org
, Nov 17 2017