xapi: evacuate non-HA-protected VMs when no host failures are tolerated

[olivierlambert]

Draft for discussion. Stacked on #7145 and currently shows both commits;
the first commit belongs to #7145. I would like maintainer input on the HA
semantics before this is considered for merge.

Why

Even with the clearer error from #7145, evacuating a host under HA still fails
outright as soon as a non-HA-protected VM is resident on it. That is a real
obstacle for small pools: with two hosts, ha_host_failures_to_tolerate is
necessarily 0, yet administrators routinely need to put a host into maintenance.
Today that fails even though the destination has plenty of free memory.

Excluding non-protected VMs from the evacuation plan only makes sense when HA is
actually reserving capacity for failover. The reason they are excluded is that
the HA planner accounts only for protected VMs, and placing extra VMs on the
remaining hosts could break the failover plan. When ha_host_failures_to_tolerate
is 0, no capacity is reserved for failover at all, so there is no plan to break:
migrating non-protected VMs is then exactly as safe as in a pool without HA.

How

Only partition VMs into protected and unprotected when HA reserves capacity
(ha_enabled && ha_host_failures_to_tolerate > 0). Otherwise treat all user VMs
as migratable and let the existing memory binpack place them, exactly as in a
pool without HA. When capacity is reserved, behaviour is unchanged and
unprotected VMs are still reported with HOST_EVACUATE_VM_NOT_HA_PROTECTED
(from #7145).

Only ocaml/xapi/xapi_host.ml is touched (in compute_evacuation_plan_no_wlb).

Open questions (why this is a draft)

• Is ha_host_failures_to_tolerate = 0 the right condition, or should it key off
  ha_overcommitted / ha_plan_exists_for instead?
• The original comment said the post-evacuation state should look to the HA
  planner the same as a host failure followed by VM restart. With ftt = 0 there
  is no failover reservation, but is there any other place that relies on
  non-protected VMs being excluded from the evacuation plan?
• Would you prefer this behind a feature flag in xapi_globs.ml (defaulted off)
  for wider testing first, as suggested in CONTRIBUTING?

Testing

Built and tested on CI (build and test, unit tests, format, CodeChecker and
ShellCheck all green on a fork branch). No automated test is added yet for the
compute_evacuation_plan_no_wlb paths; the existing test_ha_vm_failover.ml
covers the binpack layer but not this function. Happy to add coverage once the
approach is agreed.

Depends on #7145.

[minglumlu]

I think although the pool doesn't reserve any capacity for failover when the ha_host_failures_to_tolerate is 0, it still can try to restart protected VMs with best effort. In other words, ha_host_failures_to_tolerate = 0 is not equivalent to HA disabled.

The HA guarantee would change due to host.evacuate. This is just like "If the user makes their own poor placement decisions via explicit VM.start/VM.start_on then the plan may evaporate. This is no different to (eg) the user migrating a VM and breaking the plan." In this sense, I think there is no difference between ha_host_failures_to_tolerate = 0 and ha_host_failures_to_tolerate > 0. The problem is that feasible HA plan may "evaporate" after allowing migration of non-protected VMs.

[gthvn1]

Disclaimer: I am not an HA expert and I am still learning the subject, just thinking at loud here...

The evacuation code currently treats best-effort and "" VMs the same way, but I think they are different.
best-effort VMs are still restarted by HA after a failure, so I think that there is no reason to block their migration during evacuation regardless of the value of ha_host_failures_to_tolerate. Maybe we can only block "" VMs (which HA completely ignores) and allow best-effort VMs to migrate freely, since the reservation for restart VMs would not be affected. So maybe we could have something like:

• restart: HA restart it (part of the reservation plan),
• best-effort: HA restart it if there is some space (so it is consider as "protected" and won't fail with ha_host_failures_to_tolerate = 0)
• "": not restarted, unprotected

[olivierlambert]

@minglumlu thanks, fair challenge, and you're right on the first point: ftt=0 is not the same as HA disabled. The daemon is still armed and protected VMs are still restarted after a failure, on a best effort basis. My commit message overstated this by calling it "as safe as a pool without HA", I've reworded it.

On the second point, I think the VM.start/migrate analogy actually supports keying on ftt once you follow it into the guard. VM.start and VM.migrate don't freely break the plan: they go through assert_new_vm_preserves_ha_plan / assert_vm_placement_preserves_ha_plan, which plan for ha_plan_exists_for failures and raise HA_OPERATION_WOULD_BREAK_FAILOVER_PLAN if the change would make a currently feasible reserved plan evaporate. The "plan may evaporate" comment in compute_restart_plan is about the residual cases the guard can't catch (start_on placement, the restart transient), not a blanket tolerance.

So the two regimes really differ. With ftt > 0 we have ha_plan_exists_for > 0, and the guard actively blocks any VM.start/migrate that would erode the reserved plan. Evacuation has to stay strict there too, otherwise it becomes an unguarded backdoor around that same guard. That's where the exclusion (and the "looks like a host failure + restart" rule) comes from.

With ftt = 0 we have ha_plan_exists_for = 0. There is no reserved plan, and the guard only checks that protected VMs can run right now with zero reserve, which the evacuation already preserves since it migrates them. Filling the remaining hosts with non-protected VMs can only degrade best effort restart after a future failure, and that exact degradation is already allowed today: VM.start of a non-protected VM is not blocked at ftt=0. Evacuation is doing nothing the pool doesn't already permit.

So the dividing line isn't "HA on vs off", it's whether there is a reserved plan the pool actively defends. At ftt=0 there isn't.

About ha_overcommitted / ha_plan_exists_for: I kept the condition on ha_host_failures_to_tolerate because it reflects the admin's stated intent. Overcommit is a transient degraded state and I'd rather not permanently relax evacuation the moment a plan happens to evaporate. Happy to switch to ha_plan_exists_for if you think that's the more correct key.

Would you be more comfortable with this behind a xapi_globs flag (default off) for a first release? I'm fine either way.

[olivierlambert]

@gthvn1 the three tier model you describe is exactly how the restart side works: restart VMs are part of the reserved plan, best-effort VMs are restarted after them if there is room, and "" VMs are ignored. For placement though, the planner only knows two classes, and best-effort falls on the unprotected side. The plan is computed from all_protected_vms (restart only) against the live free memory of each host, so a best-effort VM migrated onto a remaining host is not accounted for by the plan: it just shows up as less free memory the next time the plan is recomputed, exactly like a "" VM would.

That is why VM.start and VM.migrate of a best-effort VM are guarded too: check_vm_preserves_ha_plan in message_forwarding.ml sends every non-restart VM through assert_vm_placement_preserves_ha_plan, which raises HA_OPERATION_WOULD_BREAK_FAILOVER_PLAN if placing it would make the reserved plan evaporate. So letting evacuation move best-effort VMs unconditionally at ftt > 0 would bypass a check that a plain VM.migrate of the same VM has to pass.

What could work as a follow-up is including best-effort (and "") VMs in the evacuation at ftt > 0 whenever their placement passes that same guard, and only reporting the error when it does not. That would unblock most evacuations on pools with spare capacity while keeping the plan safe. It is a bigger change though, since the binpack in compute_evacuation_plan would have to be validated against the failover plan, so I would rather keep this PR to the ftt = 0 case, where there is no reserved plan and all three tiers migrate anyway.