Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add partition preservation #321

Merged
merged 1 commit into from Aug 1, 2020
Merged

Add partition preservation #321

merged 1 commit into from Aug 1, 2020

Conversation

glennswest
Copy link
Contributor

@glennswest glennswest commented Jul 27, 2020

Add --save-partlabel <glob> and --save-partindex <range> options, which save partition table metadata for existing data partitions and then restore those partitions after writing the image. --save-partlabel takes a glob pattern matching the partition label, and --save-partindex takes a partition number or range of partition numbers (possibly single-ended). For example, to save all partitions with indexes greater than 4, specify --save-partindex 5-. The specified partitions need not exist. If no --save-* options are specified, no partitions are saved.

Multiples of each option can be specified, and/or multiple filters can be specified within a single option argument by separating them with commas. There are corresponding kargs coreos.inst.save_partlabel and coreos.inst.save_partindex, which cannot be repeated but which accept comma-separated patterns.

Upon restore, try to reuse the original partition number. If it's not available, renumber the partition to one more than the highest number used so far. For simplicity, never backfill entries earlier in the partition table, even if the corresponding slot is unused.

If a saved partition overlaps with the image contents, fail. This can't be detected in advance, so detect the overrun during fetch and stop before the saved partition is clobbered. Also fail if the install image has a partition extending past the end of the image that overlaps with a saved partition.

On any failure, after clearing the partition table, restore any saved partitions. In addition, augment partition-table clearing to clear the backup GPT, since some tools may otherwise hallucinate partitions that were overwritten during the install.

All of this assumes GPT partitioning both in the image and on disk, so if --save-* options are specified for a DASD target, fail.

Fixes #190.

src/blockdev.rs Outdated Show resolved Hide resolved
@ashcrow
Copy link
Member

ashcrow commented Jul 27, 2020

Looks like Travis CI failures on Rust < 1.45

src/blockdev.rs Outdated Show resolved Hide resolved
src/cmdline.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@bgilbert bgilbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High-level first look. This looks like a good starting point, thank you! Please run rustfmt over your changes and fix clippy lints.

Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
src/cmdline.rs Outdated Show resolved Hide resolved
src/blockdev.rs Outdated Show resolved Hide resolved
src/blockdev.rs Outdated Show resolved Hide resolved
src/blockdev.rs Outdated Show resolved Hide resolved
src/install.rs Outdated Show resolved Hide resolved
src/install.rs Outdated Show resolved Hide resolved
src/cmdline.rs Outdated Show resolved Hide resolved
@bgilbert bgilbert marked this pull request as draft July 30, 2020 17:54
@bgilbert bgilbert changed the title Add high-partition preservation during bare metal installs Add partition preservation Jul 30, 2020
@bgilbert bgilbert marked this pull request as ready for review July 31, 2020 07:36
@bgilbert
Copy link
Contributor

Ready for review!

Copy link
Contributor

@lucab lucab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly nits and cosmetic comments, which we can also tackle later.

The only real suspicious thing I saw is the logic that clears the GPT backup header.

src/download.rs Outdated Show resolved Hide resolved
src/blockdev.rs Show resolved Hide resolved
src/blockdev.rs Outdated Show resolved Hide resolved
src/blockdev.rs Outdated Show resolved Hide resolved
src/cmdline.rs Outdated Show resolved Hide resolved
src/install.rs Outdated Show resolved Hide resolved
scripts/coreos-installer-service Outdated Show resolved Hide resolved
src/cmdline.rs Show resolved Hide resolved
src/cmdline.rs Show resolved Hide resolved
src/download.rs Outdated Show resolved Hide resolved
src/download.rs Outdated Show resolved Hide resolved
src/blockdev.rs Outdated Show resolved Hide resolved
src/blockdev.rs Outdated Show resolved Hide resolved
src/blockdev.rs Show resolved Hide resolved
src/blockdev.rs Outdated Show resolved Hide resolved
src/blockdev.rs Outdated Show resolved Hide resolved
@bgilbert
Copy link
Contributor

Updated!

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one bikeshed that jumped out at me, but LGTM as is too!

README.md Outdated Show resolved Hide resolved
Add --save-partlabel <glob> and --save-partindex <range> options, which
save partition table metadata for existing data partitions and then
restore those partitions after writing the image.  --save-partlabel
takes a glob pattern matching the partition label, and --save-partindex
takes a partition number or range of partition numbers (possibly
single-ended).  For example, to save all partitions with indexes greater
than 4, specify "--save-partindex 5-".  The specified partitions need not
exist.  If no --save-* options are specified, no partitions are saved.

Multiples of each option can be specified, and/or multiple filters can
be specified within a single option argument by separating them with
commas.  There are corresponding kargs coreos.inst.save_partlabel and
coreos.inst.save_partindex, which cannot be repeated but which accept
comma-separated patterns.

Upon restore, try to reuse the original partition number.  If it's not
available, renumber the partition to one more than the highest number
used so far.  For simplicity, never backfill entries earlier in the
partition table, even if the corresponding slot is unused.

If a saved partition overlaps with the image contents, fail.  This can't
be detected in advance, so detect the overrun during fetch and stop
before the saved partition is clobbered.  Also fail if the install image
has a partition extending past the end of the image that overlaps with a
saved partition.

On any failure, after clearing the partition table, restore any saved
partitions.  In addition, augment partition-table clearing to clear the
backup GPT, since some tools may otherwise hallucinate partitions that
were overwritten during the install.

All of this assumes GPT partitioning both in the image and on disk, so
if --save-* options are specified for a DASD target, fail.

Fixes coreos#190.

Co-authored-by: Glenn West <gwest@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider supporting a safety check to avoid overwriting data partitions
5 participants