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

signpost: GPT priority manipulation utility #28

Merged
merged 26 commits into from Jun 13, 2019
Merged

signpost: GPT priority manipulation utility #28

merged 26 commits into from Jun 13, 2019

Conversation

iliana
Copy link
Contributor

@iliana iliana commented Jun 3, 2019

Description of changes

This introduces signpost, Thar's utility for modifying GPT priority bits on the OS disk.

signpost is a small but important part of the overall Thar update story, and builds on the gptprio patchset to GRUB.

I think I have a small expectation that over the next couple of months, this will morph into a library that happens to have a low-level utility binary. But for now, this is what we will use.

Testing

Initial boot:

localhost login: root
bash-5.0# signpost status
OS disk: /dev/vda
Set A:   boot=/dev/vda2 root=/dev/vda3 hash=/dev/vda4 priority=1 tries_left=0 successful=true
Set B:   boot=/dev/vda6 root=/dev/vda7 hash=/dev/vda8 priority=0 tries_left=0 successful=false
Active:  Set A
Next:    Set A
bash-5.0# signpost clear-inactive
bash-5.0# dd if=/dev/vda2 of=/dev/vda6
40960+0 records in
40960+0 records out
20971520 bytes (21 MB, 20 MiB) copied, 0.337728 s, 62.1 MB/s
bash-5.0# dd if=/dev/vda3 of=/dev/vda7 conv=sparse
1843200+0 records in
1843200+0 records out
943718400 bytes (944 MB, 900 MiB) copied, 3.17942 s, 297 MB/s
bash-5.0# dd if=/dev/vda4 of=/dev/vda8
16384+0 records in
16384+0 records out
8388608 bytes (8.4 MB, 8.0 MiB) copied, 0.242723 s, 34.6 MB/s
bash-5.0# signpost upgrade-to-inactive
bash-5.0# signpost status
OS disk: /dev/vda
Set A:   boot=/dev/vda2 root=/dev/vda3 hash=/dev/vda4 priority=1 tries_left=0 successful=true
Set B:   boot=/dev/vda6 root=/dev/vda7 hash=/dev/vda8 priority=2 tries_left=1 successful=false
Active:  Set A
Next:    Set B
bash-5.0# reboot

Next boot:

localhost login: root
bash-5.0# signpost status
OS disk: /dev/vda
Set A:   boot=/dev/vda2 root=/dev/vda3 hash=/dev/vda4 priority=1 tries_left=0 successful=true
Set B:   boot=/dev/vda6 root=/dev/vda7 hash=/dev/vda8 priority=2 tries_left=0 successful=false
Active:  Set B
Next:    Set A
bash-5.0# signpost mark-successful-boot
bash-5.0# signpost status
OS disk: /dev/vda
Set A:   boot=/dev/vda2 root=/dev/vda3 hash=/dev/vda4 priority=1 tries_left=0 successful=true
Set B:   boot=/dev/vda6 root=/dev/vda7 hash=/dev/vda8 priority=2 tries_left=0 successful=true
Active:  Set B
Next:    Set B
bash-5.0# signpost rollback-to-inactive
[   46.962730] random: signpost: uninitialized urandom read (16 bytes read)
bash-5.0# signpost status
OS disk: /dev/vda
Set A:   boot=/dev/vda2 root=/dev/vda3 hash=/dev/vda4 priority=2 tries_left=0 successful=true
Set B:   boot=/dev/vda6 root=/dev/vda7 hash=/dev/vda8 priority=1 tries_left=0 successful=true
Active:  Set B
Next:    Set A
bash-5.0# reboot

Next boot:

localhost login: root
bash-5.0# signpost status
OS disk: /dev/vda
Set A:   boot=/dev/vda2 root=/dev/vda3 hash=/dev/vda4 priority=2 tries_left=0 successful=true
Set B:   boot=/dev/vda6 root=/dev/vda7 hash=/dev/vda8 priority=1 tries_left=0 successful=true
Active:  Set A
Next:    Set A

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@iliana iliana requested review from tjkirch and bcressey June 3, 2019 20:35
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
After switching to partition B and before marking boot successful,
`signpost status` displayed that next boot would use B, when GRUB
gptprio would select A.

Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
There should be two states when the inactive set is priority 0: a host
that's never updated, or a host in the process of writing a new update
to the inactive partitions. In either state rolling back to that
partition will break the host, so prevent that.

Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Copy link
Member

@jahkeup jahkeup left a comment

Choose a reason for hiding this comment

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

Lot's of cool stuff in here! I have some questions and comments that I had while reading, but I'm not very astute in rust just yet, so I ask partially in ignorance.

workspaces/signpost/README.md Outdated Show resolved Hide resolved
workspaces/signpost/README.md Outdated Show resolved Hide resolved
workspaces/signpost/src/error.rs Show resolved Hide resolved
workspaces/signpost/src/gptprio.rs Outdated Show resolved Hide resolved
workspaces/signpost/src/state/mod.rs Outdated Show resolved Hide resolved
workspaces/signpost/src/state/mod.rs Show resolved Hide resolved
workspaces/signpost/src/error.rs Show resolved Hide resolved
workspaces/signpost/src/error.rs Show resolved Hide resolved
workspaces/signpost/src/main.rs Show resolved Hide resolved
Copy link
Contributor

@tjkirch tjkirch left a comment

Choose a reason for hiding this comment

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

Nice, there's some very clean and manageable code here. 🍔

workspaces/signpost/src/main.rs Show resolved Hide resolved
workspaces/signpost/src/main.rs Show resolved Hide resolved
workspaces/signpost/README.md Outdated Show resolved Hide resolved
workspaces/signpost/README.md Outdated Show resolved Hide resolved
workspaces/signpost/src/error.rs Outdated Show resolved Hide resolved
workspaces/signpost/src/state/mod.rs Outdated Show resolved Hide resolved
workspaces/signpost/src/state/mod.rs Show resolved Hide resolved
workspaces/signpost/src/state/mod.rs Outdated Show resolved Hide resolved
workspaces/signpost/src/state/mod.rs Outdated Show resolved Hide resolved
workspaces/signpost/src/state/mod.rs Outdated Show resolved Hide resolved
workspaces/signpost/README.md Outdated Show resolved Hide resolved
workspaces/signpost/README.md Outdated Show resolved Hide resolved
workspaces/signpost/README.md Show resolved Hide resolved
workspaces/signpost/src/error.rs Show resolved Hide resolved
workspaces/signpost/src/state/mod.rs Outdated Show resolved Hide resolved
workspaces/signpost/src/state/mod.rs Outdated Show resolved Hide resolved
iliana added 14 commits June 5, 2019 21:43
This was generic from previous development and doesn't need to be
anymore.

Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
... in favor of a functioin whose intent is much clearer.

Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
When faced with the fact that this method name is awful, I struggled to
find a good reason that a consumer of signpost as a library would care
about what devices belonged to partition set A, as opposed to the active
or inactive partition set.

Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Copy link
Contributor

@tjkirch tjkirch left a comment

Choose a reason for hiding this comment

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

Random thought: Do you think it'd be hard to extend signpost to handle partition sets being on different disks? It should just be different pointers in grub, and here, things like searching for partitions on the single "os_disk"...

workspaces/signpost/src/state/block.rs Outdated Show resolved Hide resolved
workspaces/signpost/src/state/mod.rs Outdated Show resolved Hide resolved
workspaces/signpost/src/state/mod.rs Outdated Show resolved Hide resolved
workspaces/signpost/README.md Outdated Show resolved Hide resolved
workspaces/signpost/src/state/block.rs Outdated Show resolved Hide resolved
workspaces/signpost/src/state/mod.rs Outdated Show resolved Hide resolved
workspaces/signpost/src/gptprio.rs Show resolved Hide resolved
@iliana
Copy link
Contributor Author

iliana commented Jun 13, 2019

Do you think it'd be hard to extend signpost to handle partition sets being on different disks? It should just be different pointers in grub, and here, things like searching for partitions on the single "os_disk"...

I think it's more difficult than what I'd like to implement right now.

Signed-off-by: Tom Kirchner <tjk@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
This was written before we checked if GRUB would boot the inactive
partition with `will_boot`.

Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
Signed-off-by: Tom Kirchner <tjk@amazon.com>
Signed-off-by: iliana destroyer of worlds <iweller@amazon.com>
@tjkirch tjkirch merged commit 0eb348d into develop Jun 13, 2019
@tjkirch tjkirch deleted the signpost branch June 13, 2019 17:57
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.

None yet

5 participants