Skip to content

CP-54026: option to control VM-internal shutdown behaviour under HA #6412

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.

Sign up for GitHub

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

Merged
robhoes merged 2 commits into from
Apr 10, 2025
Merged

CP-54026: option to control VM-internal shutdown behaviour under HA #6412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fbefb1c
CP-54026: option to control VM-internal shutdown behaviour under HA
robhoes Mar 19, 2025
9ecacc8
Update datamodel_lifecycle
robhoes Apr 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ocaml/idl/datamodel_common.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ open Datamodel_roles
to leave a gap for potential hotfixes needing to increment the schema version.*)
let schema_major_vsn = 5

let schema_minor_vsn = 787
let schema_minor_vsn = 788

(* Historical schema versions just in case this is useful later *)
let rio_schema_major_vsn = 5
Expand Down
4 changes: 3 additions & 1 deletion ocaml/idl/datamodel_lifecycle.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ let prototyped_of_field = function
| "host", "last_software_update" ->
Some "22.20.0"
| "VM_guest_metrics", "services" ->
Some "25.14.0-next"
Some "25.15.0"
| "VM_guest_metrics", "netbios_name" ->
Some "24.28.0"
| "VM", "groups" ->
Expand All @@ -123,6 +123,8 @@ let prototyped_of_field = function
Some "23.18.0"
| "VM", "actions__after_softreboot" ->
Some "23.1.0"
| "pool", "ha_reboot_vm_on_internal_shutdown" ->
Some "25.15.0-next"
| "pool", "license_server" ->
Some "25.6.0"
| "pool", "recommendations" ->
Expand Down
6 changes: 6 additions & 0 deletions ocaml/idl/datamodel_pool.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2191,6 +2191,12 @@ let t =
~ty:(Map (String, String))
~default_value:(Some (VMap [])) "license_server"
"Licensing data shared within the whole pool"
; field ~writer_roles:_R_POOL_OP ~qualifier:RW ~lifecycle:[] ~ty:Bool
~default_value:(Some (VBool true))
"ha_reboot_vm_on_internal_shutdown"
"Indicates whether an HA-protected VM that is shut down from \
inside (not through the API) should be automatically rebooted \
when HA is enabled"
]
)
()
2 changes: 1 addition & 1 deletion ocaml/idl/schematest.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ let hash x = Digest.string x |> Digest.to_hex
(* BEWARE: if this changes, check that schema has been bumped accordingly in
ocaml/idl/datamodel_common.ml, usually schema_minor_vsn *)

let last_known_schema_hash = "7756b4bea0be3985c1c8f6708f04d442"
let last_known_schema_hash = "e10b420b0863116ee188eea9e63b1349"

let current_schema_hash : string =
let open Datamodel_types in
Expand Down
6 changes: 4 additions & 2 deletions ocaml/tests/common/test_common.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,8 @@ let make_pool ~__context ~master ?(name_label = "") ?(name_description = "")
?(telemetry_next_collection = API.Date.epoch)
?(last_update_sync = API.Date.epoch) ?(update_sync_frequency = `daily)
?(update_sync_day = 0L) ?(update_sync_enabled = false)
?(recommendations = []) ?(license_server = []) () =
?(recommendations = []) ?(license_server = [])
?(ha_reboot_vm_on_internal_shutdown = true) () =
let pool_ref = Ref.make () in
Db.Pool.create ~__context ~ref:pool_ref ~uuid:(make_uuid ()) ~name_label
~name_description ~master ~default_SR ~suspend_image_SR ~crash_dump_SR
Expand All @@ -320,7 +321,8 @@ let make_pool ~__context ~master ?(name_label = "") ?(name_description = "")
~local_auth_max_threads:8L ~ext_auth_max_threads:8L
~ext_auth_cache_enabled:false ~ext_auth_cache_size:50L
~ext_auth_cache_expiry:300L ~update_sync_frequency ~update_sync_day
~update_sync_enabled ~recommendations ~license_server ;
~update_sync_enabled ~recommendations ~license_server
~ha_reboot_vm_on_internal_shutdown ;
pool_ref

let default_sm_features =
Expand Down
9 changes: 9 additions & 0 deletions ocaml/xapi-cli-server/records.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1244,6 +1244,15 @@ let pool_record rpc session_id pool =
; make_field ~name:"ha-overcommitted"
~get:(fun () -> string_of_bool (x ()).API.pool_ha_overcommitted)
()
; make_field ~name:"ha-reboot-vm-on-internal-shutdown"
~get:(fun () ->
string_of_bool (x ()).API.pool_ha_reboot_vm_on_internal_shutdown
)
~set:(fun x ->
Client.Pool.set_ha_reboot_vm_on_internal_shutdown ~rpc ~session_id
~self:pool ~value:(bool_of_string x)
)
()
; make_field ~name:"blobs"
~get:(fun () -> get_uuid_map_from_ref_map (x ()).API.pool_blobs)
()
Expand Down
2 changes: 1 addition & 1 deletion ocaml/xapi/dbsync_master.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ let create_pool_record ~__context =
~update_sync_day:0L ~update_sync_enabled:false ~local_auth_max_threads:8L
~ext_auth_max_threads:1L ~ext_auth_cache_enabled:false
~ext_auth_cache_size:50L ~ext_auth_cache_expiry:300L ~recommendations:[]
~license_server:[]
~license_server:[] ~ha_reboot_vm_on_internal_shutdown:true

let set_master_ip ~__context =
let ip =
Expand Down
14 changes: 14 additions & 0 deletions ocaml/xapi/xapi_xenops.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2033,6 +2033,20 @@ let update_vm ~__context id =
"Will update VM.allowed_operations because power_state has \
changed." ;
should_update_allowed_operations := true ;
(* Update ha_always_run before the power_state (if needed), to avoid racing
Copy link
Contributor

@Vincent-lau Vincent-lau Apr 8, 2025

Choose a reason for hiding this comment

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

so the ha monitor thread checks the field ha_always_run to determine whether it needs to restart a protected VM. Is it possible to add this logic into Helpers.vm_should_always_run (although this one does not have context) or xapi_ha_vm_failover.all_protected_vms?

Copy link
Contributor

@lindig lindig Apr 8, 2025

Choose a reason for hiding this comment

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

Should the documentation of the field in the IDL reference ha_always_run as a dependency?

Copy link
Member Author

@robhoes robhoes Apr 8, 2025

Choose a reason for hiding this comment

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

@lindig VM.ha_always_run is really an internal implementation detail now and API clients shouldn't really look at it anymore. Initially, this field was the way to control protected VMs using the API, but it has been deprecated for a long time.

Copy link
Member Author

@robhoes robhoes Apr 8, 2025

Choose a reason for hiding this comment

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

@Vincent-lau I think it will be hard to encode this in some sort of new state without catching the shutdown event here.

with the HA monitor thread. *)
let pool = Helpers.get_pool ~__context in
if
power_state = `Halted
&& not
(Db.Pool.get_ha_reboot_vm_on_internal_shutdown ~__context
~self:pool
)
then (
Db.VM.set_ha_always_run ~__context ~self ~value:false ;
debug "Setting ha_always_run on vm=%s as false after shutdown"
(Ref.string_of self)
) ;
debug "xenopsd event: Updating VM %s power_state <- %s" id
(Record_util.vm_power_state_to_string power_state) ;
(* This will mark VBDs, VIFs as detached and clear resident_on
Expand Down
Loading