[update] Marker for VMMs stopped by an update by karencfv · Pull Request #11127 · oxidecomputer/omicron

karencfv · 2026-08-21T04:48:23Z

First PR for the work laid out in RFD 739 for instance restarts during an update.

We've not completely decided on all of the tasks for the machinery to restart instances, but I think the bit about marking VMMs as "stopped for update" is pretty uncontroversial, so I got started with this.

This is just adding the stopped_for_update_disposition_generation column to the vmm table; I will add tests for this field when there is actually something populating it.

The output for the omdb db vmm info command would look like this:

== VMM =========================================================================
                                  ID: 9c60a0e3-8f2b-4a1e-bd77-1f2e3a4b5c6d
                         instance ID: 6b6f2f4e-0d3c-4b8a-9e21-7c8d9e0f1a2b
                          created at: 2026-08-21 17:42:03.123456 UTC
                               state: stopped
      marked to stop for sled update: update disposition generation 4
                          updated at: 2026-08-21T17:42:05.987654Z (generation 5)
                    propolis address: fd00:1122:3344:101::1a:12400
                             sled ID: 2f5e8b1c-3a4d-4e6f-8091-a2b3c4d5e6f7
                         sled serial: BRM42220004
                        CPU platform: amd_milan

karencfv · 2026-08-21T04:53:56Z

+     *
+     * NULL when its state has not been modified by an update.
+     */
+    stopped_for_update_disposition_generation INT8,


I'm not adding a constraint here to make sure this field can only ever be present if state is Stopping, Stopped, or Failed on purpose. According to RFD 739, we will have a task that looks for the presence of this marker and then start the process of stopping the VMM.

Would like to confirm that this makes sense.

Yup, that's correct; the mark will be set when the instance is Running (or Starting or Rebooting), and will remain set when it actually has stppped.

hawkw

i had some annoying quibbles over naming, sorry. beyond that, this looks good!

hawkw · 2026-08-21T05:17:08Z

    const STATE: &'static str = "state";
    const FAILURE_REASON: &'static str = "  failure reason";
    const FAILURE_NOTE: &'static str = "  note";
+    const STOPPED_FOR_UPDATE: &'static str = "  update disposition";


this may be a bit nitpicky but i feel like i would want this to say something like "marked to stop for sled update" or similar. to an unfamiliar reader, i would want to communicate that:

this is about stopping the VMM

this is a mark saying that the VMM should be stopped (not an indication that it has stopped or that a stop has been requested yet)

the reason it should be stopped is to update the sled (not some hypothetical update of the VMM itself)

totally fair, tbh I spent quite a bit of time trying to think of what to put here and it all felt wrong. I changed it; it's a little long but readable I think

== VMM ========================================================================= ID: 9c60a0e3-8f2b-4a1e-bd77-1f2e3a4b5c6d instance ID: 6b6f2f4e-0d3c-4b8a-9e21-7c8d9e0f1a2b created at: 2026-08-21 17:42:03.123456 UTC state: stopped marked to stop for sled update: update disposition generation 4 updated at: 2026-08-21T17:42:05.987654Z (generation 5) propolis address: fd00:1122:3344:101::1a:12400 sled ID: 2f5e8b1c-3a4d-4e6f-8091-a2b3c4d5e6f7 sled serial: BRM42220004 CPU platform: amd_milan

hawkw · 2026-08-21T05:18:52Z

+     *
+     * NULL when its state has not been modified by an update.
+     */
+    stopped_for_update_disposition_generation INT8,


Yup, that's correct; the mark will be set when the instance is Running (or Starting or Rebooting), and will remain set when it actually has stppped.

hawkw · 2026-08-21T05:22:11Z

+     *
+     * NULL when its state has not been modified by an update.
+     */
+    stopped_for_update_disposition_generation INT8,


i'm not sure if i love the name "stopped_for_update_disposition_generation" because the use of "stopped" suggests that if this has set, the vmm has stopped, which may not be true. how about

Suggested change

stopped_for_update_disposition_generation INT8,

should_stop_for_update_generation INT8,

or something? i'd like it to be clear that this being set means "SHOULD stop" rather than "IS stopped".

i might also try to include the word "sled" so that it's obvious that it's the sled that is being updated, not the Propolis process, but the name of this column is already super long so i dunno if it's worth it.

i also thought about "stop_requested_...", but i think i like "should_stop_..." better, because we may not have actually sent a request to the sled agent to stop the VMM yet.

truthfully, I was pretty unhappy with that field as well. I don't want to leave out the "disposition" bit, because "update generation" could be interpreted as something else. should_stop_for_update_disposition_generation ended up being soooo long 🙃

So, I changed it to stop_for_update_disposition_generation. I don't think it implies anything has stopped yet. What do you think?

Yeah, I think "stop_for_update..." is definitely better than "stopped_for_update...". That plus some comments should hopefully make it clear enough.

karencfv

Thanks for taking a look @hawkw! I addressed your comments. Let me know what you think!

karencfv · 2026-08-24T07:31:30Z

    const STATE: &'static str = "state";
    const FAILURE_REASON: &'static str = "  failure reason";
    const FAILURE_NOTE: &'static str = "  note";
+    const STOPPED_FOR_UPDATE: &'static str = "  update disposition";


totally fair, tbh I spent quite a bit of time trying to think of what to put here and it all felt wrong. I changed it; it's a little long but readable I think

== VMM ========================================================================= ID: 9c60a0e3-8f2b-4a1e-bd77-1f2e3a4b5c6d instance ID: 6b6f2f4e-0d3c-4b8a-9e21-7c8d9e0f1a2b created at: 2026-08-21 17:42:03.123456 UTC state: stopped marked to stop for sled update: update disposition generation 4 updated at: 2026-08-21T17:42:05.987654Z (generation 5) propolis address: fd00:1122:3344:101::1a:12400 sled ID: 2f5e8b1c-3a4d-4e6f-8091-a2b3c4d5e6f7 sled serial: BRM42220004 CPU platform: amd_milan

karencfv · 2026-08-24T07:34:13Z

+     *
+     * NULL when its state has not been modified by an update.
+     */
+    stopped_for_update_disposition_generation INT8,


truthfully, I was pretty unhappy with that field as well. I don't want to leave out the "disposition" bit, because "update generation" could be interpreted as something else. should_stop_for_update_disposition_generation ended up being soooo long 🙃

So, I changed it to stop_for_update_disposition_generation. I don't think it implies anything has stopped yet. What do you think?

karencfv · 2026-08-24T07:36:38Z

btw, I'll merge with main once this is approved to avoid fixing merge conflicts more than once

hawkw

sorry to continue quibbling about the way we describe this, but I think the comment could still express how this works a bit more clearly. I do quite like the changes to the omdb output, and other than the comment, this looks good!

hawkw · 2026-08-24T22:17:31Z

+    /// The sled's `update_disposition` generation which triggered this VMM to
+    /// stop.


Sorry to keep splitting hairs about this, but this comment still feels a bit like it's phrased as though the VMM has definitely stopped if this is set. How would you feel about something more like

Suggested change

/// The sled's `update_disposition` generation which triggered this VMM to

/// stop.

/// The sled's `update_disposition` generation at which this VMM was marked

/// as needing to be stopped in order to update the sled.

///

/// This is set when the sled begins evacuating its VMs. It indicates two

/// things: that the VMM needs to be stopped, and that when it is stopped,

/// this was in order to update the sled. The sled may not have been stopped

/// yet.

and perhaps also something noting that the numeric value of the update disposition generation is included mostly for debugging purposes and that it's primarily just used as a flag based on whether it's present or not.

hawkw · 2026-08-24T22:18:18Z

    failure_reason omicron.public.vmm_failure_reason,
+    /*
+     * The sled's `update_disposition` generation which triggered this VMM to
+     * stop.


As mentioned in my other comment, I think this could be a bit clearer about the meaning of the field.

hawkw · 2026-08-24T22:18:30Z

+    if let Some(ud_generation) = stop_for_update_disposition_generation {
+        let u_g = u64::from(ud_generation.0);
+        println!(
+            "{indent}{STOP_FOR_UPDATE:>width$}: update disposition generation {u_g}"


this is great, thank you!

karencfv

sorry to continue quibbling about the way we describe this, but I think the comment could still express how this works a bit more clearly.

hahah no worries! It makes sense to take the time to get this right. I've made some changes and thanks for your feedback!

hawkw · 2026-08-25T00:21:00Z

        // |  leaving the first copy as an example for the next person.
        // v
        // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
+        KnownVersion::new(296, "vmm-stopped-for-update-disposition-generation"),


might wanna rename the migration, now that the column is stop_for_update...

oops! thanks

karencfv added 2 commits August 21, 2026 14:46

[update] Marker for VMMs stopped by an update

0f66d78

fix cmd output

9464c61

karencfv commented Aug 21, 2026

View reviewed changes

karencfv requested review from davepacheco and hawkw August 21, 2026 04:54

hawkw reviewed Aug 21, 2026

View reviewed changes

karencfv commented Aug 24, 2026

View reviewed changes

address comments

059e47e

hawkw reviewed Aug 24, 2026

View reviewed changes

make the comment better

f1260d8

karencfv commented Aug 24, 2026

View reviewed changes

hawkw approved these changes Aug 24, 2026

View reviewed changes

merge main

196136b

karencfv enabled auto-merge (squash) August 25, 2026 00:18

hawkw reviewed Aug 25, 2026

View reviewed changes

karencfv disabled auto-merge August 25, 2026 00:22

rename migration

b9f501e

karencfv enabled auto-merge (squash) August 25, 2026 00:29

karencfv merged commit 650a933 into oxidecomputer:main Aug 25, 2026
19 checks passed

karencfv deleted the stopped-for-update-marker branch August 25, 2026 03:12

This was referenced Aug 26, 2026

Tracking issue for RFD 739 (Instance restart during live update) #11169

Open

[reconfigurator] background task for marking VMMs to be stopped for update #11170

Open

	stopped_for_update_disposition_generation INT8,
	should_stop_for_update_generation INT8,

		/// The sled's `update_disposition` generation which triggered this VMM to
		/// stop.

-    /// The sled's `update_disposition` generation which triggered this VMM to
-    /// stop.
+    /// The sled's `update_disposition` generation at which this VMM was marked
+    /// as needing to be stopped in order to update the sled.
+    ///
+    /// This is set when the sled begins evacuating its VMs. It indicates two
+    /// things: that the VMM needs to be stopped, and that when it is stopped,
+    /// this was in order to update the sled. The sled may not have been stopped
+    /// yet.

Conversation

karencfv commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hawkw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karencfv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karencfv commented Aug 24, 2026

Uh oh!

hawkw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karencfv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

karencfv commented Aug 21, 2026 •

edited

Loading