update_kernel.sh can't determine $PARTITION_NUM_xxx on old rootfs |
||||||
Issue descriptionPARTITION_NUM_KERN_A and PARTITION_NUM_KERN_B were killed here: https://chromium-review.googlesource.com/c/439509/ after being "removed" from the update_kernel script here: https://chromium-review.googlesource.com/451543 But...not all references were removed. This results in us "learning" that the kernel partition is at, e.g., /dev/mmcblk0p, which is not a device partition at all. So we think we're writing the kernel partition, but we're not. We should fix up the update_kernel script to get this right, but it'd be nice if there was some validation for this too... I wonder if dd's 'conv=nocreat' is portable enough? I've found it very valuable in the past to present this kind of SNAFU. Or we could just do a simple bash '-e' check before writing. Also, how is it that no one noticed this for the last few months?
,
Jun 14 2017
$ cat /usr/sbin/write_gpt.sh | grep PARTITION_NUM shows nothing. I'm updating a pretty old image. If that's the only problem, then this is less important. But is it possible we can retain compatibility? Or at least add some error-checking, so it's more obvious the update isn't working.
,
Jun 14 2017
(Yes, I know 'cat ... | grep ...' is one 'cat' too many.) I see where the PARTITION_NUM stuff got into write_gpt.sh: https://chromium-review.googlesource.com/439508 So yes, I guess I need a relatively new image.
,
Jun 14 2017
I'll rely on your judgement for how important this is, or if we should just close it. I'm happy to cook up some kind of fix if necessary.
,
Jun 14 2017
Well, I think at a minimum it would be nice to improve the script so it knows when it's flashing to nowhere. That would have saved me some considerable trouble over the subtle problems of an updated set of modules, with no updated kernel partition. (And no errors; it's perfectly fine to create a new '/dev/mmcblk0p' file.) Full backward compatibility might be overkill. Notably, this is still marked 'Available'. I might tackle this myself later in the week if I get time. (Thanks for the pointers so far.)
,
Jun 14 2017
we should error out when we can't detect the partition, but i don't think we want to add backwards compat code. ToT is largely meant to be used with ToT-ish systems.
,
Jun 15 2017
Apparently 'dd ... conv=nocreat' isn't POSIX. Oh well. I've added some error checking here, which we'd want anyway: https://chromium-review.googlesource.com/537140
,
Jun 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/crosutils/+/9774b0db8b8d69b9bbc36d150a9c6fb1d723999f commit 9774b0db8b8d69b9bbc36d150a9c6fb1d723999f Author: Brian Norris <briannorris@chromium.org> Date: Sat Jun 17 08:12:42 2017 update_kernel: error out if kernel partition isn't found The kernel partition might not be found for a number of reasons: * the '--partition=' argument might be incorrect * the DUT might not be compatible with us (e.g., it's using an old image, with differences in /usr/sbin/write_gpt.sh) Currently, rather than noticing this and bailing out, we're happy to use 'dd' to create a new file instead. That can create unintuitive behaviors, where the DUT's kernel modules have been updated while the kernel partition is left unmodified. Let's detect this situation and error out, instead of leaving the user to figure out what happened. BUG= chromium:733028 TEST=update_kernel on old images (without $PARTITION_NUM... variables) and new ones Change-Id: I47f64ec7fb1175e0e25697a6b55b63366fcc4555 Signed-off-by: Brian Norris <briannorris@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/537140 Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/9774b0db8b8d69b9bbc36d150a9c6fb1d723999f/update_kernel.sh
,
Jun 19 2017
Didn't "fix" backward compatibility, but at least the script will error out and make this more obvious now.
,
Jan 22 2018
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by icoolidge@chromium.org
, Jun 14 2017The CL defines this function in remote_access.sh: # Discover partition numbers from the target. learn_partition_layout() { source <(remote_sh_raw cat /usr/sbin/write_gpt.sh) load_base_vars } Does that not work for your target? I'm finding it difficult to understand how that's possible