Repository navigation
restore: write a plain image of the whole NOR with --raw - #264
Merged
Merged
Conversation
restore and upgrade only took ipctool's own containers, so a dump from a programmer or dd could not be put back without wrapping it by hand. Given a raw file, restore stopped with "Broken backup" only after it had freed resources and asked to proceed. --raw cuts the image along the camera's partitions and hands them to the same do_flash() path, so blocks are still read back as they are written and --skip-env still works. The file is mapped, not read, as an upgrade bundle is: a dump in /tmp would otherwise be in RAM twice. An image says nothing about itself, so fitting this flash exactly is what makes it safe to write. All of that is settled before anything is stopped and --force does not override it: NOR only, device numbers consecutive and in flash order, one erase size, sysfs offsets contiguous where the kernel has them, and the size equal to what the partitions cover. Device order matters because do_flash() writes by device number and places blocks by adding up sizes, and a partition enumeration could not open leaves a hole in that numbering. An "all" alias at mtd0 is refused for the same reason. The SHA1 is printed for comparing by hand, there being none to check. The checks are in raw_image.c so they can be tested on a host. Run on an emulated hi3516ev300 with an 8 MB NOR, comparing the flash file afterwards from the host: a full write matches the image, --skip-env keeps a valid env block, an "all" partition last is written through, and a short image, "all" first, a declined prompt and upgrade --raw leave the flash as it was.
PR Summary by QodoRestore whole-NOR images with restore --raw
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
Code Review by Qodo
1.
|
Sysfs offsets exist from 4.4 on, and on anything older --raw only compared sizes. Overlapping partitions that happened to add up to the image passed that, and each block's read-back would then have passed too. Refusing those kernels outright would have taken --raw away from every 3.x camera, so mtd_flash_offset() now asks the command line's mtdparts= and then the line the kernel logs per partition, which also covers tables built into the kernel. The format of that line is the same from 3.0 on. A partition is matched by name and size, and not at all if two places claim it. Partitions are enumerated through this lookup, so alias detection gets real offsets on those kernels too, in place of comparing sizes. With offsets to go on, restore --raw picks the partitions that cover the image exactly once and writes each straight to its device, instead of adding up sizes along device numbers. That also takes in what used to be refused: "all" as mtd0 and numbers out of flash order, as on Anjoy. Overlaps that are not whole, gaps, and a partition nothing can place are refused, the last with a hint to reboot while the log still has it. The mapping keeps pages in memory but does not freeze the file, so one changed after the prompt would have been written unnoticed and one truncated would have raised SIGBUS mid-flash. Size and SHA1 are now checked again once nothing else is running and nothing is erased yet; a copy would have put a dump in /tmp in RAM twice. Both follow the review of the previous commit. Run on an emulated hi3516ev300 with /sys unmounted, /proc/cmdline hidden and the log rotated away, to reach each source in turn, comparing the flash file from the host.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
restoreandupgradeonly take ipctool's own containers, so a dump from a programmer orddcould not be put back without wrapping it by hand.restore --raw <file>writes a plain image of the whole NOR flash.What it does
It cuts the image along the partitions that really lie on the flash and writes each to its device through
do_flash(), so blocks are still read back as they are written and--skip-envstill works. The file is mapped rather than read, as an upgrade bundle is, so a dump in/tmpis not in RAM twice.An image describes nothing and carries no checksums, so fitting this flash exactly is what makes it safe to write. These checks run before anything is stopped, and
--forceskips only the question, not them:all, a second name) is left outThe image's SHA1 and the partition map are printed before the prompt. Size and SHA1 are checked again after every other process is stopped and before anything is erased, because the mapping pins pages, not the file's contents. If either changed, the camera reboots with the flash untouched.
Where partition offsets come from
/sys/class/mtd/mtdN/offsetexists only from 4.4. Refusing older kernels would remove--rawfrom every 3.x camera, somtd_flash_offset()falls back to:mtdparts=on the kernel command line0x%012llx-0x%012llx : "name"line the kernel logs per partition, which also covers tables built into the kernel, and is the same format in the 3.0, 3.4 and 3.10 sources checkedA partition is matched by name and size, and not at all if two places claim it. If nothing can place a partition,
--rawrefuses and says to reboot and run it again while the log still has the lines.This changes more than
--raw: partitions are enumerated through this lookup, so on 3.x kernels alias detection forbackupand the report now uses real offsets rather than comparing sizes. The kernel log is also read on those kernels when the command line does not settle it.Testing
Run on an emulated hi3516ev300 (kernel 4.9, 8 MB NOR), comparing the flash file from the host afterwards. To reach each source, PID 1 left
/sysunmounted, bound an empty file over/proc/cmdline, and flooded/dev/kmsgto rotate the boot log away.--skip-envwith a valid envallfirst or lastallfirstallfirst, out of order--skip-envupgrade --rawraw_image_testcovers the planning andmtd_alias_testthe two parsers, including torn log lines and ambiguous names.Not tested: real hardware, booting from a written image (the images were random data), and a real 3.x kernel. The fallbacks ran on 4.9 with sysfs hidden. Their log parser is checked against the format string in the 3.x sources, not against a captured 3.x boot log.
Limits
restore, it does not unlock the flash, so write-protected regions could stop it partway through. The camera then reboots with a partly written flash.bootplusallwith nothing else is refused as not covering the image, even thoughallalone could carry it.Possibly related, not changed here
From reading the code, plain
restoremay misplace data on Anjoy boards:enum_mtd_info()lists their partitions in flash order, butmap_old_new_mtd()walkspart[]by device number. This is untested.--rawnow places by real offset, and its out-of-order case passes in the emulator.