From a68be09a98d44840c9844cd75bdb6dd0c34bf298 Mon Sep 17 00:00:00 2001 From: Stephen Cheng Date: Tue, 31 Mar 2026 15:18:17 +0800 Subject: [PATCH 01/20] CP-311907: Add VM.secureboot_certificates_state field Add a new DynamicRO field to track Secure Boot certificate status per VM. The field indicates whether UEFI Secure Boot certificates need updating. - Define enum (ok, update_available, update_on_boot) and field in datamodel - Check certificate state via varstore-nvram-certcheck on import and DB upgrade for UEFI Secure Boot VMs - Skip control domains, default templates, and non-Secure Boot VMs in the DB upgrade rule Signed-off-by: Stephen Cheng --- ocaml/idl/datamodel_lifecycle.ml | 2 + ocaml/idl/datamodel_vm.ml | 25 +++++++++++ ocaml/tests/record_util/old_enum_all.ml | 3 ++ ocaml/tests/record_util/old_record_util.ml | 8 ++++ ocaml/tests/record_util/test_record_util.ml | 4 ++ ocaml/xapi-cli-server/records.ml | 6 +++ ocaml/xapi-guard/lib/server_interface.ml | 19 +++++++++ ocaml/xapi-guard/test/xapi_guard_test.ml | 4 ++ ocaml/xapi-idl/guard/varstored/interface.ml | 5 +++ ...et_secureboot_certificates_state.request.0 | 1 + ...t_secureboot_certificates_state.response.0 | 1 + ocaml/xapi/create_misc.ml | 3 +- ocaml/xapi/import.ml | 25 +++++++++++ ocaml/xapi/xapi_db_upgrade.ml | 42 +++++++++++++++++++ ocaml/xapi/xapi_globs.ml | 6 +++ ocaml/xapi/xapi_vm.ml | 3 +- ocaml/xapi/xapi_vm_clone.ml | 4 +- ocaml/xapi/xapi_vm_helpers.ml | 34 +++++++++++++++ 18 files changed, 192 insertions(+), 3 deletions(-) create mode 100644 ocaml/xapi-idl/lib_test/test_data/guard/varstored/requests/VM.get_secureboot_certificates_state.request.0 create mode 100644 ocaml/xapi-idl/lib_test/test_data/guard/varstored/responses/VM.get_secureboot_certificates_state.response.0 diff --git a/ocaml/idl/datamodel_lifecycle.ml b/ocaml/idl/datamodel_lifecycle.ml index 076c0a591dd..879dff0aa7f 100644 --- a/ocaml/idl/datamodel_lifecycle.ml +++ b/ocaml/idl/datamodel_lifecycle.ml @@ -143,6 +143,8 @@ let prototyped_of_field = function Some "26.2.0" | "VM_metrics", "numa_optimised" -> Some "26.2.0" + | "VM", "secureboot_certificates_state" -> + Some "26.7.0-next" | "VM", "groups" -> Some "24.19.1" | "VM", "pending_guidances_full" -> diff --git a/ocaml/idl/datamodel_vm.ml b/ocaml/idl/datamodel_vm.ml index 17178314acc..e0d83bc359f 100644 --- a/ocaml/idl/datamodel_vm.ml +++ b/ocaml/idl/datamodel_vm.ml @@ -2408,6 +2408,26 @@ let set_uefi_mode = ~result:(String, "Result from the varstore-sb-state call") ~doc:"Set the UEFI mode of a VM" ~allowed_roles:_R_POOL_ADMIN () +let vm_secureboot_certificates_state = + Enum + ( "vm_secureboot_certificates_state" + , [ + ( "ok" + , "The VM's certificates do not need to be updated (including the case \ + where Secure Boot does not apply to this VM, e.g. BIOS VM)." + ) + ; ( "update_available" + , "The Secure Boot certificates are due to expire or have already \ + expired." + ) + ; ( "update_on_boot" + , "XS will trigger an update of the certificates whenever the VM \ + boots. This includes VM.start, VM.reboot and a guest-triggered \ + reboot." + ) + ] + ) + let vm_secureboot_readiness = Enum ( "vm_secureboot_readiness" @@ -3184,6 +3204,11 @@ let t = doesn't need to" ; field ~qualifier:DynamicRO ~lifecycle:[] ~ty:(Set (Ref _vm_group)) "groups" "VM groups associated with the VM" + ; field ~qualifier:DynamicRO ~lifecycle:[] + ~ty:vm_secureboot_certificates_state + ~default_value:(Some (VEnum "ok")) "secureboot_certificates_state" + "The state of the Secure Boot certificates, showing whether an \ + update is required, already scheduled, or not needed." ] ) () diff --git a/ocaml/tests/record_util/old_enum_all.ml b/ocaml/tests/record_util/old_enum_all.ml index f58cdc7542f..70dc5d0274d 100644 --- a/ocaml/tests/record_util/old_enum_all.ml +++ b/ocaml/tests/record_util/old_enum_all.ml @@ -311,6 +311,9 @@ let all_vm_secureboot_readiness = ; `certs_incomplete ] +let all_vm_secureboot_certificates_state = + [`ok; `update_available; `update_on_boot] + let all_sr_health = [`healthy; `recovering; `unreachable; `unavailable] let all_event_operation = [`add; `del; `_mod] diff --git a/ocaml/tests/record_util/old_record_util.ml b/ocaml/tests/record_util/old_record_util.ml index 855a2b74b7e..aa84224ea78 100644 --- a/ocaml/tests/record_util/old_record_util.ml +++ b/ocaml/tests/record_util/old_record_util.ml @@ -186,6 +186,14 @@ let pool_guest_secureboot_readiness_to_string = function | `not_ready -> "not_ready" +let vm_secureboot_certificates_state_to_string = function + | `ok -> + "ok" + | `update_available -> + "update_available" + | `update_on_boot -> + "update_on_boot" + let pool_operation_to_string = function | `ha_enable -> "ha_enable" diff --git a/ocaml/tests/record_util/test_record_util.ml b/ocaml/tests/record_util/test_record_util.ml index 323ce4dae0e..d30aa0ddfc6 100644 --- a/ocaml/tests/record_util/test_record_util.ml +++ b/ocaml/tests/record_util/test_record_util.ml @@ -132,6 +132,10 @@ let tests = ( O.pool_guest_secureboot_readiness_to_string , N.pool_guest_secureboot_readiness_to_string ) + ; mk __LINE__ None all_vm_secureboot_certificates_state + ( O.vm_secureboot_certificates_state_to_string + , N.vm_secureboot_certificates_state_to_string + ) ; mk __LINE__ None all_pool_allowed_operations (O.pool_operation_to_string, N.pool_allowed_operations_to_string) ; mk __LINE__ None all_host_allowed_operations diff --git a/ocaml/xapi-cli-server/records.ml b/ocaml/xapi-cli-server/records.ml index 9e076ac23d7..5c391398b8a 100644 --- a/ocaml/xapi-cli-server/records.ml +++ b/ocaml/xapi-cli-server/records.ml @@ -2676,6 +2676,12 @@ let vm_record rpc session_id vm = Client.VM.set_groups ~rpc ~session_id ~self:vm ~value ) () + ; make_field ~name:"secureboot-certificates-state" + ~get:(fun () -> + Record_util.vm_secureboot_certificates_state_to_string + (x ()).API.vM_secureboot_certificates_state + ) + () ; make_field ~name:"snapshot-schedule" ~get:(fun () -> get_uuid_from_ref (x ()).API.vM_snapshot_schedule) ~set:(fun x -> diff --git a/ocaml/xapi-guard/lib/server_interface.ml b/ocaml/xapi-guard/lib/server_interface.ml index 6e81ce77f2d..f48da834448 100644 --- a/ocaml/xapi-guard/lib/server_interface.ml +++ b/ocaml/xapi-guard/lib/server_interface.ml @@ -192,6 +192,24 @@ let make_server_varstored _persist ~cache path vm_uuid = ) |> ret in + let get_secureboot_certificates_state _ _ = + (let* self = get_vm_ref () in + let* state = + with_xapi ~cache @@ VM.get_secureboot_certificates_state ~self + in + let state_str = + match state with + | `ok -> + "ok" + | `update_available -> + "update_available" + | `update_on_boot -> + "update_on_boot" + in + Lwt.return state_str + ) + |> ret + in let message_create _ _name priority _cls _uuid body = ret (let* (_ : _ Ref.t) = @@ -207,6 +225,7 @@ let make_server_varstored _persist ~cache path vm_uuid = let dummy_logout _ = ret @@ Lwt.return_unit in Server.get_NVRAM get_nvram ; Server.set_NVRAM set_nvram ; + Server.get_secureboot_certificates_state get_secureboot_certificates_state ; Server.message_create message_create ; Server.session_login dummy_login ; Server.session_logout dummy_logout ; diff --git a/ocaml/xapi-guard/test/xapi_guard_test.ml b/ocaml/xapi-guard/test/xapi_guard_test.ml index 2f048fe6660..bb5e783ed11 100644 --- a/ocaml/xapi-guard/test/xapi_guard_test.ml +++ b/ocaml/xapi-guard/test/xapi_guard_test.ml @@ -57,6 +57,10 @@ let xapi_rpc call = expect_vm vm_rpc ; nvram_contents := [("EFI-variables", API.string_of_rpc contents)] ; ret_ok "" + | "VM.get_secureboot_certificates_state", [session_id_rpc; vm_rpc] -> + expect_session_id session_id_rpc ; + expect_vm vm_rpc ; + ret_ok "ok" | _ -> Fmt.failwith "XAPI RPC call %s not expected in test" call.Rpc.name diff --git a/ocaml/xapi-idl/guard/varstored/interface.ml b/ocaml/xapi-idl/guard/varstored/interface.ml index e418dc4d094..c71e6efe875 100644 --- a/ocaml/xapi-idl/guard/varstored/interface.ml +++ b/ocaml/xapi-idl/guard/varstored/interface.ml @@ -80,6 +80,11 @@ module RPC_API (R : RPC) = struct ["Set the current VM's NVRAM contents"] (string_p @-> string_p @-> string_p @-> returning unit_p err) + let get_secureboot_certificates_state = + declare "VM.get_secureboot_certificates_state" + ["Get the current VM's Secure Boot certificates state"] + (string_p @-> string_p @-> returning string_p err) + let message_create = declare "message.create" ["Send an alert when booting a UEFI guest fails"] diff --git a/ocaml/xapi-idl/lib_test/test_data/guard/varstored/requests/VM.get_secureboot_certificates_state.request.0 b/ocaml/xapi-idl/lib_test/test_data/guard/varstored/requests/VM.get_secureboot_certificates_state.request.0 new file mode 100644 index 00000000000..dbdf9d24d47 --- /dev/null +++ b/ocaml/xapi-idl/lib_test/test_data/guard/varstored/requests/VM.get_secureboot_certificates_state.request.0 @@ -0,0 +1 @@ +VM.get_secureboot_certificates_statestringstring \ No newline at end of file diff --git a/ocaml/xapi-idl/lib_test/test_data/guard/varstored/responses/VM.get_secureboot_certificates_state.response.0 b/ocaml/xapi-idl/lib_test/test_data/guard/varstored/responses/VM.get_secureboot_certificates_state.response.0 new file mode 100644 index 00000000000..18ed8315ea9 --- /dev/null +++ b/ocaml/xapi-idl/lib_test/test_data/guard/varstored/responses/VM.get_secureboot_certificates_state.response.0 @@ -0,0 +1 @@ +StatusSuccessValuestring \ No newline at end of file diff --git a/ocaml/xapi/create_misc.ml b/ocaml/xapi/create_misc.ml index 3c2d680d328..e85c205d54f 100644 --- a/ocaml/xapi/create_misc.ml +++ b/ocaml/xapi/create_misc.ml @@ -317,7 +317,8 @@ and create_domain_zero_record ~__context ~domain_zero_ref (host_info : host_info ~has_vendor_device:false ~requires_reboot:false ~reference_label:"" ~domain_type:Xapi_globs.domain_zero_domain_type ~nVRAM:[] ~pending_guidances:[] ~recommended_guidances:[] - ~pending_guidances_recommended:[] ~pending_guidances_full:[] ; + ~pending_guidances_recommended:[] ~pending_guidances_full:[] + ~secureboot_certificates_state:`ok ; ensure_domain_zero_metrics_record ~__context ~domain_zero_ref host_info ; Db.Host.set_control_domain ~__context ~self:localhost ~value:domain_zero_ref ; Xapi_vm_helpers.update_memory_overhead ~__context ~vm:domain_zero_ref diff --git a/ocaml/xapi/import.ml b/ocaml/xapi/import.ml index c768ee966a7..40e126bb116 100644 --- a/ocaml/xapi/import.ml +++ b/ocaml/xapi/import.ml @@ -620,6 +620,14 @@ module VM : HandlerTools = struct else {vm_record with API.vM_has_vendor_device= false} in + (* If imported from an older pool without secureboot_certificates_state, + default to ok. The DB upgrade rule will correct it post-import. *) + let vm_record = + if vm_has_field ~x ~name:"secureboot_certificates_state" then + vm_record + else + {vm_record with API.vM_secureboot_certificates_state= `ok} + in let vm_record = { vm_record with @@ -776,6 +784,23 @@ module VM : HandlerTools = struct ) ; Db.VM.set_bios_strings ~__context ~self:vm ~value:vm_record.API.vM_bios_strings ; + (* If imported from an older pool, check and set certificate state. + Skip default templates as per design. *) + ( if + (not (vm_has_field ~x ~name:"secureboot_certificates_state")) + && not vm_record.API.vM_is_default_template + then + try + let state = + Xapi_vm_helpers.check_secureboot_certificates_state ~__context + ~self:vm + in + Db.VM.set_secureboot_certificates_state ~__context ~self:vm + ~value:state + with e -> + debug "Failed to check secureboot certificate state for VM %s: %s" + (Ref.string_of vm) (Printexc.to_string e) + ) ; debug "Created VM: %s (was %s)" (Ref.string_of vm) x.id ; (* Although someone could sneak in here and attempt to power on the VM, it doesn't really matter since no VBDs have been created yet. diff --git a/ocaml/xapi/xapi_db_upgrade.ml b/ocaml/xapi/xapi_db_upgrade.ml index aba1cf15444..0a41793123c 100644 --- a/ocaml/xapi/xapi_db_upgrade.ml +++ b/ocaml/xapi/xapi_db_upgrade.ml @@ -965,6 +965,47 @@ let upgrade_ca_fingerprints = ) } +let upgrade_secureboot_certificates_state = + { + description= "Set secureboot_certificates_state for existing VMs" + ; version= (fun _ -> true) + ; fn= + (fun ~__context -> + List.iter + (fun self -> + if + Db.VM.get_is_control_domain ~__context ~self + || Db.VM.get_is_default_template ~__context ~self + then + () + else + let boot_params = Db.VM.get_HVM_boot_params ~__context ~self in + let is_uefi = + List.assoc_opt "firmware" boot_params = Some "uefi" + in + let platformdata = Db.VM.get_platform ~__context ~self in + let is_secureboot = + Vm_platform.is_true ~key:"secureboot" ~platformdata + ~default:false + in + if is_uefi && is_secureboot then + try + let state = + Xapi_vm_helpers.check_secureboot_certificates_state + ~__context ~self + in + Db.VM.set_secureboot_certificates_state ~__context ~self + ~value:state + with e -> + D.warn + "Failed to check secureboot certificate state for VM %s: %s" + (Db.VM.get_uuid ~__context ~self) + (Printexc.to_string e) + ) + (Db.VM.get_all ~__context) + ) + } + let rules = [ upgrade_domain_type @@ -995,6 +1036,7 @@ let rules = ; empty_pool_uefi_certificates ; upgrade_update_guidance ; upgrade_ca_fingerprints + ; upgrade_secureboot_certificates_state ] (* Maybe upgrade most recent db *) diff --git a/ocaml/xapi/xapi_globs.ml b/ocaml/xapi/xapi_globs.ml index 684a3779a65..5fcf95803df 100644 --- a/ocaml/xapi/xapi_globs.ml +++ b/ocaml/xapi/xapi_globs.ml @@ -914,6 +914,8 @@ let varstore_ls = ref "/usr/bin/varstore-ls" let varstore_dir = ref "/var/lib/varstored" +let varstore_nvram_certcheck = ref "/usr/bin/varstore-nvram-certcheck" + let default_auth_dir = ref "/usr/share/varstored" let allow_custom_uefi_certs = ref false @@ -2119,6 +2121,10 @@ module Resources = struct ) ; ("varstore-ls", varstore_ls, "Executed to list the UEFI variables of a VM") ; ("varstore_dir", varstore_dir, "Path to local varstored directory") + ; ( "varstore-nvram-certcheck" + , varstore_nvram_certcheck + , "Executed to check VM NVRAM Secure Boot certificate status" + ) ; ( "nvidia-sriov-manage" , nvidia_sriov_manage_script , "Path to NVIDIA sriov-manage script" diff --git a/ocaml/xapi/xapi_vm.ml b/ocaml/xapi/xapi_vm.ml index dcb1e482089..b6979832265 100644 --- a/ocaml/xapi/xapi_vm.ml +++ b/ocaml/xapi/xapi_vm.ml @@ -688,7 +688,8 @@ let create ~__context ~name_label ~name_description ~power_state ~user_version ~suspend_SR ~version ~generation_id ~hardware_platform_version ~has_vendor_device ~requires_reboot:false ~reference_label ~domain_type ~pending_guidances:[] ~recommended_guidances:[] - ~pending_guidances_recommended:[] ~pending_guidances_full:[] ; + ~pending_guidances_recommended:[] ~pending_guidances_full:[] + ~secureboot_certificates_state:`ok ; Xapi_vm_lifecycle.update_allowed_operations ~__context ~self:vm_ref ; update_memory_overhead ~__context ~vm:vm_ref ; update_vm_virtual_hardware_platform_version ~__context ~vm:vm_ref ; diff --git a/ocaml/xapi/xapi_vm_clone.ml b/ocaml/xapi/xapi_vm_clone.ml index 4bc09fee111..25a30eac278 100644 --- a/ocaml/xapi/xapi_vm_clone.ml +++ b/ocaml/xapi/xapi_vm_clone.ml @@ -402,7 +402,9 @@ let copy_vm_record ?snapshot_info_record ~__context ~vm ~disk_op ~new_name ~requires_reboot:false ~reference_label:all.Db_actions.vM_reference_label ~domain_type:all.Db_actions.vM_domain_type ~nVRAM:all.Db_actions.vM_NVRAM ~pending_guidances:[] ~recommended_guidances:[] - ~pending_guidances_recommended:[] ~pending_guidances_full:[] ; + ~pending_guidances_recommended:[] ~pending_guidances_full:[] + ~secureboot_certificates_state: + all.Db_actions.vM_secureboot_certificates_state ; (* update the VM's parent field in case of snapshot. Note this must be done after "ref" has been created, so that its "children" field can be updated by the database layer *) ( match disk_op with diff --git a/ocaml/xapi/xapi_vm_helpers.ml b/ocaml/xapi/xapi_vm_helpers.ml index 748e2afb20e..8b163e29c91 100644 --- a/ocaml/xapi/xapi_vm_helpers.ml +++ b/ocaml/xapi/xapi_vm_helpers.ml @@ -1705,3 +1705,37 @@ let ensure_device_model_profile_present ~__context ~domain_type ~is_a_template else (* only add device-model to an HVM VM platform if it is not already there *) default :: platform + +let check_secureboot_certificates_state ~__context ~self = + let vm_uuid = Db.VM.get_uuid ~__context ~self in + let nvram = Db.VM.get_NVRAM ~__context ~self in + match List.assoc_opt "EFI-variables" nvram with + | None -> + D.info "VM %s has no EFI-variables in NVRAM, defaulting to ok" vm_uuid ; + `ok + | Some efi_vars -> ( + let tmp_path = Filename.temp_file ("nvram-" ^ vm_uuid ^ "-") ".dat" in + let result = + finally + (fun () -> + Xapi_stdext_unix.Unixext.write_string_to_file tmp_path efi_vars ; + let output, _ = + Forkhelpers.execute_command_get_output + !Xapi_globs.varstore_nvram_certcheck + [tmp_path] + in + String.trim output + ) + (fun () -> Xapi_stdext_unix.Unixext.unlink_safe tmp_path) + in + match result with + | "update_required" -> + `update_available + | "update_ok" -> + `ok + | other -> + D.warn + "varstore-nvram-certcheck returned unexpected output for VM %s: %s" + vm_uuid other ; + `ok + ) From 9eb495d1312e04585315d0b3db80d0aa5efddd29 Mon Sep 17 00:00:00 2001 From: Stephen Cheng Date: Thu, 2 Apr 2026 11:03:06 +0800 Subject: [PATCH 02/20] CP-311908: Add versioned VM.set_NVRAM_EFI_variables with update parameter Add a new versioned parameter 'update' to VM.set_NVRAM_EFI_variables, allowing varstored to indicate whether Secure Boot certificates were changed during an NVRAM write. This enables xapi to maintain the VM.secureboot_certificates_state field accurately. The 'update' parameter is an enum with three values: - 'yes': certificates were updated, set state to 'ok' - 'no': certificates unchanged, keep current state as-is - 'unspecified': (default for v1 callers) run certcheck to determine state Signed-off-by: Stephen Cheng --- ocaml/idl/datamodel_vm.ml | 37 ++++++++++++++++++- ocaml/tests/test_vm_check_operation_error.ml | 2 +- ocaml/xapi-guard/lib/server_interface.ml | 13 ++++++- ocaml/xapi-guard/test/xapi_guard_test.ml | 12 +++++- ocaml/xapi-idl/guard/varstored/interface.ml | 5 +++ .../VM.set_NVRAM_EFI_variables_v2.request.0 | 1 + .../VM.set_NVRAM_EFI_variables_v2.response.0 | 1 + ocaml/xapi/message_forwarding.ml | 4 +- ocaml/xapi/xapi_vm.ml | 24 +++++++++++- ocaml/xapi/xapi_vm.mli | 6 ++- 10 files changed, 96 insertions(+), 9 deletions(-) create mode 100644 ocaml/xapi-idl/lib_test/test_data/guard/varstored/requests/VM.set_NVRAM_EFI_variables_v2.request.0 create mode 100644 ocaml/xapi-idl/lib_test/test_data/guard/varstored/responses/VM.set_NVRAM_EFI_variables_v2.response.0 diff --git a/ocaml/idl/datamodel_vm.ml b/ocaml/idl/datamodel_vm.ml index e0d83bc359f..0cb894ede96 100644 --- a/ocaml/idl/datamodel_vm.ml +++ b/ocaml/idl/datamodel_vm.ml @@ -2352,7 +2352,42 @@ let set_HVM_boot_policy = let set_NVRAM_EFI_variables = call ~flags:[`Session] ~name:"set_NVRAM_EFI_variables" ~lifecycle:[(Published, rel_naples, "")] - ~params:[(Ref _vm, "self", "The VM"); (String, "value", "The value")] + ~versioned_params: + [ + { + param_type= Ref _vm + ; param_name= "self" + ; param_doc= "The VM" + ; param_release= naples_release + ; param_default= None + } + ; { + param_type= String + ; param_name= "value" + ; param_doc= "The EFI-variables value" + ; param_release= naples_release + ; param_default= None + } + ; { + param_type= + Enum + ( "update_status" + , [ + ("yes", "Certificates have been updated") + ; ("no", "Certificates have not been changed") + ; ("unspecified", "Caller did not provide the update argument") + ] + ) + ; param_name= "update" + ; param_doc= + "If 'yes', set secureboot_certificates_state to ok. If 'no', keep \ + the current secureboot_certificates_state unchanged. If omitted \ + (defaults to 'unspecified'), run certificate check to determine \ + the state." + ; param_release= numbered_release "26.7.0-next" + ; param_default= Some (VEnum "unspecified") + } + ] ~hide_from_docs:true ~allowed_roles:_R_LOCAL_ROOT_ONLY () let restart_device_models = diff --git a/ocaml/tests/test_vm_check_operation_error.ml b/ocaml/tests/test_vm_check_operation_error.ml index 40d37c4beb5..460ded911e2 100644 --- a/ocaml/tests/test_vm_check_operation_error.ml +++ b/ocaml/tests/test_vm_check_operation_error.ml @@ -52,7 +52,7 @@ let test_vm_set_nvram_running () = let new_vars = "CCCC" in let new_nvram = [("EFI-variables", new_vars)] in Api_server_common.Forwarder.VM.set_NVRAM_EFI_variables ~__context - ~self:vm_ref ~value:new_vars ; + ~self:vm_ref ~value:new_vars ~update:`no ; let read_nvram = Db.VM.get_NVRAM ~__context ~self:vm_ref in Alcotest.(check (list (pair string string))) "NVRAM updated" new_nvram read_nvram diff --git a/ocaml/xapi-guard/lib/server_interface.ml b/ocaml/xapi-guard/lib/server_interface.ml index f48da834448..574400e5351 100644 --- a/ocaml/xapi-guard/lib/server_interface.ml +++ b/ocaml/xapi-guard/lib/server_interface.ml @@ -188,7 +188,17 @@ let make_server_varstored _persist ~cache path vm_uuid = in let set_nvram _ _ nvram = (let* self = get_vm_ref () in - with_xapi ~cache @@ VM.set_NVRAM_EFI_variables ~self ~value:nvram + with_xapi ~cache + @@ VM.set_NVRAM_EFI_variables ~self ~value:nvram ~update:`unspecified + ) + |> ret + in + let set_nvram_v2 _ _ nvram update = + (let* self = get_vm_ref () in + let update = + match update with "yes" -> `yes | "no" -> `no | _ -> `unspecified + in + with_xapi ~cache @@ VM.set_NVRAM_EFI_variables ~self ~value:nvram ~update ) |> ret in @@ -225,6 +235,7 @@ let make_server_varstored _persist ~cache path vm_uuid = let dummy_logout _ = ret @@ Lwt.return_unit in Server.get_NVRAM get_nvram ; Server.set_NVRAM set_nvram ; + Server.set_NVRAM_v2 set_nvram_v2 ; Server.get_secureboot_certificates_state get_secureboot_certificates_state ; Server.message_create message_create ; Server.session_login dummy_login ; diff --git a/ocaml/xapi-guard/test/xapi_guard_test.ml b/ocaml/xapi-guard/test/xapi_guard_test.ml index bb5e783ed11..e945376f316 100644 --- a/ocaml/xapi-guard/test/xapi_guard_test.ml +++ b/ocaml/xapi-guard/test/xapi_guard_test.ml @@ -57,6 +57,12 @@ let xapi_rpc call = expect_vm vm_rpc ; nvram_contents := [("EFI-variables", API.string_of_rpc contents)] ; ret_ok "" + | "VM.set_NVRAM_EFI_variables_v2", [session_id_rpc; vm_rpc; contents; _update] + -> + expect_session_id session_id_rpc ; + expect_vm vm_rpc ; + nvram_contents := [("EFI-variables", API.string_of_rpc contents)] ; + ret_ok "" | "VM.get_secureboot_certificates_state", [session_id_rpc; vm_rpc] -> expect_session_id session_id_rpc ; expect_vm vm_rpc ; @@ -112,7 +118,10 @@ let test_change_nvram ~rpc ~session_id () = let* nvram0 = VM.get_NVRAM ~rpc ~session_id ~self in Alcotest.(check' dict) ~msg:"nvram initial" ~expected:[] ~actual:nvram0 ; let contents = "nvramnew" in - let* () = VM.set_NVRAM_EFI_variables ~rpc ~session_id ~self ~value:contents in + let* () = + VM.set_NVRAM_EFI_variables ~rpc ~session_id ~self ~value:contents + ~update:`no + in let* nvram1 = VM.get_NVRAM ~rpc ~session_id ~self in Alcotest.(check' dict) ~msg:"nvram changed" @@ -137,6 +146,7 @@ let test_bad_get_nvram ~rpc ~session_id () = let test_bad_set_nvram ~rpc ~session_id () = let* () = VM.set_NVRAM_EFI_variables ~rpc ~session_id ~self:vm_bad ~value:"bad" + ~update:`no in let* vm_ref = VM.get_by_uuid ~rpc ~session_id ~uuid:vm_uuid_str in let* nvram = VM.get_NVRAM ~rpc ~session_id ~self:vm_ref in diff --git a/ocaml/xapi-idl/guard/varstored/interface.ml b/ocaml/xapi-idl/guard/varstored/interface.ml index c71e6efe875..20b529b672f 100644 --- a/ocaml/xapi-idl/guard/varstored/interface.ml +++ b/ocaml/xapi-idl/guard/varstored/interface.ml @@ -80,6 +80,11 @@ module RPC_API (R : RPC) = struct ["Set the current VM's NVRAM contents"] (string_p @-> string_p @-> string_p @-> returning unit_p err) + let set_NVRAM_v2 = + declare "VM.set_NVRAM_EFI_variables_v2" + ["Set the current VM's NVRAM contents, with update flag"] + (string_p @-> string_p @-> string_p @-> string_p @-> returning unit_p err) + let get_secureboot_certificates_state = declare "VM.get_secureboot_certificates_state" ["Get the current VM's Secure Boot certificates state"] diff --git a/ocaml/xapi-idl/lib_test/test_data/guard/varstored/requests/VM.set_NVRAM_EFI_variables_v2.request.0 b/ocaml/xapi-idl/lib_test/test_data/guard/varstored/requests/VM.set_NVRAM_EFI_variables_v2.request.0 new file mode 100644 index 00000000000..8ba486eb48f --- /dev/null +++ b/ocaml/xapi-idl/lib_test/test_data/guard/varstored/requests/VM.set_NVRAM_EFI_variables_v2.request.0 @@ -0,0 +1 @@ +VM.set_NVRAM_EFI_variables_v2stringstringstringstring \ No newline at end of file diff --git a/ocaml/xapi-idl/lib_test/test_data/guard/varstored/responses/VM.set_NVRAM_EFI_variables_v2.response.0 b/ocaml/xapi-idl/lib_test/test_data/guard/varstored/responses/VM.set_NVRAM_EFI_variables_v2.response.0 new file mode 100644 index 00000000000..4fa6e28fc1f --- /dev/null +++ b/ocaml/xapi-idl/lib_test/test_data/guard/varstored/responses/VM.set_NVRAM_EFI_variables_v2.response.0 @@ -0,0 +1 @@ +StatusSuccessValue \ No newline at end of file diff --git a/ocaml/xapi/message_forwarding.ml b/ocaml/xapi/message_forwarding.ml index f02c37366a7..249c92e42b8 100644 --- a/ocaml/xapi/message_forwarding.ml +++ b/ocaml/xapi/message_forwarding.ml @@ -3130,10 +3130,10 @@ functor (vm_uuid ~__context self) value ; Local.VM.set_HVM_boot_policy ~__context ~self ~value - let set_NVRAM_EFI_variables ~__context ~self ~value = + let set_NVRAM_EFI_variables ~__context ~self ~value ~update = (* called by varstored, bypasses VM powerstate check *) info "VM.set_NVRAM_EFI_variables: self = '%s'" (vm_uuid ~__context self) ; - Local.VM.set_NVRAM_EFI_variables ~__context ~self ~value + Local.VM.set_NVRAM_EFI_variables ~__context ~self ~value ~update let restart_device_models ~__context ~self = info "VM.restart_device_models: self = '%s'" (vm_uuid ~__context self) ; diff --git a/ocaml/xapi/xapi_vm.ml b/ocaml/xapi/xapi_vm.ml index b6979832265..2069efe1cc1 100644 --- a/ocaml/xapi/xapi_vm.ml +++ b/ocaml/xapi/xapi_vm.ml @@ -1651,7 +1651,7 @@ let set_HVM_boot_policy ~__context ~self ~value = let nvram = Mutex.create () -let set_NVRAM_EFI_variables ~__context ~self ~value = +let set_NVRAM_EFI_variables ~__context ~self ~value ~update = with_lock nvram (fun () -> (* do not use remove_from_NVRAM: we do not want to * temporarily end up with an empty NVRAM in HA *) @@ -1659,7 +1659,27 @@ let set_NVRAM_EFI_variables ~__context ~self ~value = let nvram = Db.VM.get_NVRAM ~__context ~self in let value = (key, value) :: List.remove_assoc key nvram in Db.VM.set_NVRAM ~__context ~self ~value - ) + ) ; + (* Update secureboot_certificates_state after NVRAM is written. + If this fails, log and leave the state unchanged *) + try + match update with + | `yes -> + Db.VM.set_secureboot_certificates_state ~__context ~self ~value:`ok + | `no -> + () (* keep current state unchanged *) + | `unspecified -> + let new_state = + Xapi_vm_helpers.check_secureboot_certificates_state ~__context ~self + in + Db.VM.set_secureboot_certificates_state ~__context ~self + ~value:new_state + with e -> + let vm_uuid = Db.VM.get_uuid ~__context ~self in + debug + "set_NVRAM_EFI_variables: failed to update secureboot_certificates_state \ + for VM %s: %s" + vm_uuid (Printexc.to_string e) let restart_device_models ~__context ~self = let power_state = Db.VM.get_power_state ~__context ~self in diff --git a/ocaml/xapi/xapi_vm.mli b/ocaml/xapi/xapi_vm.mli index b3f07d38a9d..16b3816e79c 100644 --- a/ocaml/xapi/xapi_vm.mli +++ b/ocaml/xapi/xapi_vm.mli @@ -425,7 +425,11 @@ val set_HVM_boot_policy : __context:Context.t -> self:API.ref_VM -> value:string -> unit val set_NVRAM_EFI_variables : - __context:Context.t -> self:API.ref_VM -> value:string -> unit + __context:Context.t + -> self:API.ref_VM + -> value:string + -> update:[`yes | `no | `unspecified] + -> unit val restart_device_models : __context:Context.t -> self:API.ref_VM -> unit From 4960bc4e69c3303c6ef04a69ea3b93d35136939f Mon Sep 17 00:00:00 2001 From: Chunjie Zhu Date: Wed, 15 Apr 2026 17:25:55 +0800 Subject: [PATCH 03/20] VM.secureboot_certificates_state is lost during import Signed-off-by: Chunjie Zhu --- ocaml/xapi/import.ml | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ocaml/xapi/import.ml b/ocaml/xapi/import.ml index 40e126bb116..fc1edd902cc 100644 --- a/ocaml/xapi/import.ml +++ b/ocaml/xapi/import.ml @@ -784,12 +784,14 @@ module VM : HandlerTools = struct ) ; Db.VM.set_bios_strings ~__context ~self:vm ~value:vm_record.API.vM_bios_strings ; - (* If imported from an older pool, check and set certificate state. - Skip default templates as per design. *) - ( if - (not (vm_has_field ~x ~name:"secureboot_certificates_state")) - && not vm_record.API.vM_is_default_template - then + (* VM.create_from_record may not preserve this DynamicRO field. + If metadata contains it, explicitly restore it. + If imported from an older pool without this field, compute it from + imported NVRAM (skipping default templates as per design). *) + ( if vm_has_field ~x ~name:"secureboot_certificates_state" then + Db.VM.set_secureboot_certificates_state ~__context ~self:vm + ~value:vm_record.API.vM_secureboot_certificates_state + else if not vm_record.API.vM_is_default_template then try let state = Xapi_vm_helpers.check_secureboot_certificates_state ~__context From 9e0da609113ad3d329b950e14c506d3fdc485e63 Mon Sep 17 00:00:00 2001 From: Stephen Cheng Date: Thu, 16 Apr 2026 11:12:38 +0800 Subject: [PATCH 04/20] Fixup: Always recompute the certificates state on import Also fix other comments during review Signed-off-by: Stephen Cheng --- ocaml/idl/datamodel_common.ml | 2 +- ocaml/idl/datamodel_lifecycle.ml | 2 +- ocaml/idl/datamodel_vm.ml | 13 ++++--- ocaml/idl/schematest.ml | 2 +- ocaml/tests/record_util/old_enum_all.ml | 3 -- ocaml/tests/record_util/old_record_util.ml | 8 ----- ocaml/tests/record_util/test_record_util.ml | 4 --- ocaml/xapi/import.ml | 38 ++++++++------------- ocaml/xapi/xapi_db_upgrade.ml | 20 +++++------ ocaml/xapi/xapi_vm.ml | 33 +++++++----------- ocaml/xapi/xapi_vm_helpers.ml | 10 ++++-- 11 files changed, 55 insertions(+), 80 deletions(-) diff --git a/ocaml/idl/datamodel_common.ml b/ocaml/idl/datamodel_common.ml index 0fe08b11c39..25e689f4fa4 100644 --- a/ocaml/idl/datamodel_common.ml +++ b/ocaml/idl/datamodel_common.ml @@ -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 = 901 +let schema_minor_vsn = 902 (* Historical schema versions just in case this is useful later *) let rio_schema_major_vsn = 5 diff --git a/ocaml/idl/datamodel_lifecycle.ml b/ocaml/idl/datamodel_lifecycle.ml index 879dff0aa7f..830ee1c61d5 100644 --- a/ocaml/idl/datamodel_lifecycle.ml +++ b/ocaml/idl/datamodel_lifecycle.ml @@ -144,7 +144,7 @@ let prototyped_of_field = function | "VM_metrics", "numa_optimised" -> Some "26.2.0" | "VM", "secureboot_certificates_state" -> - Some "26.7.0-next" + Some "26.10.0" | "VM", "groups" -> Some "24.19.1" | "VM", "pending_guidances_full" -> diff --git a/ocaml/idl/datamodel_vm.ml b/ocaml/idl/datamodel_vm.ml index 0cb894ede96..a70c3552398 100644 --- a/ocaml/idl/datamodel_vm.ml +++ b/ocaml/idl/datamodel_vm.ml @@ -2373,9 +2373,12 @@ let set_NVRAM_EFI_variables = Enum ( "update_status" , [ - ("yes", "Certificates have been updated") - ; ("no", "Certificates have not been changed") - ; ("unspecified", "Caller did not provide the update argument") + ("yes", "Set secureboot_certificates_state to ok") + ; ("no", "Leave secureboot_certificates_state unchanged") + ; ( "unspecified" + , "Check certificates and update \ + secureboot_certificates_state accordingly" + ) ] ) ; param_name= "update" @@ -2456,7 +2459,7 @@ let vm_secureboot_certificates_state = expired." ) ; ( "update_on_boot" - , "XS will trigger an update of the certificates whenever the VM \ + , "An update of the certificates will be triggered whenever the VM \ boots. This includes VM.start, VM.reboot and a guest-triggered \ reboot." ) @@ -3243,7 +3246,7 @@ let t = ~ty:vm_secureboot_certificates_state ~default_value:(Some (VEnum "ok")) "secureboot_certificates_state" "The state of the Secure Boot certificates, showing whether an \ - update is required, already scheduled, or not needed." + update is available, already scheduled, or not needed." ] ) () diff --git a/ocaml/idl/schematest.ml b/ocaml/idl/schematest.ml index c963c8f116b..7ad0d15a25b 100644 --- a/ocaml/idl/schematest.ml +++ b/ocaml/idl/schematest.ml @@ -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 = "32bbba07579ca8844fa6162164530268" +let last_known_schema_hash = "1912326f952ea3de0155e0746751d91a" let current_schema_hash : string = let open Datamodel_types in diff --git a/ocaml/tests/record_util/old_enum_all.ml b/ocaml/tests/record_util/old_enum_all.ml index 70dc5d0274d..f58cdc7542f 100644 --- a/ocaml/tests/record_util/old_enum_all.ml +++ b/ocaml/tests/record_util/old_enum_all.ml @@ -311,9 +311,6 @@ let all_vm_secureboot_readiness = ; `certs_incomplete ] -let all_vm_secureboot_certificates_state = - [`ok; `update_available; `update_on_boot] - let all_sr_health = [`healthy; `recovering; `unreachable; `unavailable] let all_event_operation = [`add; `del; `_mod] diff --git a/ocaml/tests/record_util/old_record_util.ml b/ocaml/tests/record_util/old_record_util.ml index aa84224ea78..855a2b74b7e 100644 --- a/ocaml/tests/record_util/old_record_util.ml +++ b/ocaml/tests/record_util/old_record_util.ml @@ -186,14 +186,6 @@ let pool_guest_secureboot_readiness_to_string = function | `not_ready -> "not_ready" -let vm_secureboot_certificates_state_to_string = function - | `ok -> - "ok" - | `update_available -> - "update_available" - | `update_on_boot -> - "update_on_boot" - let pool_operation_to_string = function | `ha_enable -> "ha_enable" diff --git a/ocaml/tests/record_util/test_record_util.ml b/ocaml/tests/record_util/test_record_util.ml index d30aa0ddfc6..323ce4dae0e 100644 --- a/ocaml/tests/record_util/test_record_util.ml +++ b/ocaml/tests/record_util/test_record_util.ml @@ -132,10 +132,6 @@ let tests = ( O.pool_guest_secureboot_readiness_to_string , N.pool_guest_secureboot_readiness_to_string ) - ; mk __LINE__ None all_vm_secureboot_certificates_state - ( O.vm_secureboot_certificates_state_to_string - , N.vm_secureboot_certificates_state_to_string - ) ; mk __LINE__ None all_pool_allowed_operations (O.pool_operation_to_string, N.pool_allowed_operations_to_string) ; mk __LINE__ None all_host_allowed_operations diff --git a/ocaml/xapi/import.ml b/ocaml/xapi/import.ml index fc1edd902cc..612dd542dc0 100644 --- a/ocaml/xapi/import.ml +++ b/ocaml/xapi/import.ml @@ -620,13 +620,11 @@ module VM : HandlerTools = struct else {vm_record with API.vM_has_vendor_device= false} in - (* If imported from an older pool without secureboot_certificates_state, - default to ok. The DB upgrade rule will correct it post-import. *) + (* Always default secureboot_certificates_state to ok in the record + passed to create_from_record -- the actual state will be recomputed + below against the importing pool's certificates. *) let vm_record = - if vm_has_field ~x ~name:"secureboot_certificates_state" then - vm_record - else - {vm_record with API.vM_secureboot_certificates_state= `ok} + {vm_record with API.vM_secureboot_certificates_state= `ok} in let vm_record = { @@ -784,24 +782,18 @@ module VM : HandlerTools = struct ) ; Db.VM.set_bios_strings ~__context ~self:vm ~value:vm_record.API.vM_bios_strings ; - (* VM.create_from_record may not preserve this DynamicRO field. - If metadata contains it, explicitly restore it. - If imported from an older pool without this field, compute it from - imported NVRAM (skipping default templates as per design). *) - ( if vm_has_field ~x ~name:"secureboot_certificates_state" then - Db.VM.set_secureboot_certificates_state ~__context ~self:vm - ~value:vm_record.API.vM_secureboot_certificates_state - else if not vm_record.API.vM_is_default_template then - try - let state = - Xapi_vm_helpers.check_secureboot_certificates_state ~__context + (* Always recompute secureboot_certificates_state against the + importing pool's certificates, since the exporting pool's state + is not meaningful here. *) + ( if not vm_record.API.vM_is_default_template then + let state = + ( Xapi_vm_helpers.check_secureboot_certificates_state ~__context ~self:vm - in - Db.VM.set_secureboot_certificates_state ~__context ~self:vm - ~value:state - with e -> - debug "Failed to check secureboot certificate state for VM %s: %s" - (Ref.string_of vm) (Printexc.to_string e) + :> API.vm_secureboot_certificates_state + ) + in + Db.VM.set_secureboot_certificates_state ~__context ~self:vm + ~value:state ) ; debug "Created VM: %s (was %s)" (Ref.string_of vm) x.id ; (* Although someone could sneak in here and attempt to power on the VM, it diff --git a/ocaml/xapi/xapi_db_upgrade.ml b/ocaml/xapi/xapi_db_upgrade.ml index 0a41793123c..9c000a27950 100644 --- a/ocaml/xapi/xapi_db_upgrade.ml +++ b/ocaml/xapi/xapi_db_upgrade.ml @@ -968,7 +968,7 @@ let upgrade_ca_fingerprints = let upgrade_secureboot_certificates_state = { description= "Set secureboot_certificates_state for existing VMs" - ; version= (fun _ -> true) + ; version= (fun x -> x < (5, 902)) ; fn= (fun ~__context -> List.iter @@ -989,18 +989,14 @@ let upgrade_secureboot_certificates_state = ~default:false in if is_uefi && is_secureboot then - try - let state = - Xapi_vm_helpers.check_secureboot_certificates_state + let state = + ( Xapi_vm_helpers.check_secureboot_certificates_state ~__context ~self - in - Db.VM.set_secureboot_certificates_state ~__context ~self - ~value:state - with e -> - D.warn - "Failed to check secureboot certificate state for VM %s: %s" - (Db.VM.get_uuid ~__context ~self) - (Printexc.to_string e) + :> API.vm_secureboot_certificates_state + ) + in + Db.VM.set_secureboot_certificates_state ~__context ~self + ~value:state ) (Db.VM.get_all ~__context) ) diff --git a/ocaml/xapi/xapi_vm.ml b/ocaml/xapi/xapi_vm.ml index 2069efe1cc1..367c9e4b43b 100644 --- a/ocaml/xapi/xapi_vm.ml +++ b/ocaml/xapi/xapi_vm.ml @@ -1660,26 +1660,19 @@ let set_NVRAM_EFI_variables ~__context ~self ~value ~update = let value = (key, value) :: List.remove_assoc key nvram in Db.VM.set_NVRAM ~__context ~self ~value ) ; - (* Update secureboot_certificates_state after NVRAM is written. - If this fails, log and leave the state unchanged *) - try - match update with - | `yes -> - Db.VM.set_secureboot_certificates_state ~__context ~self ~value:`ok - | `no -> - () (* keep current state unchanged *) - | `unspecified -> - let new_state = - Xapi_vm_helpers.check_secureboot_certificates_state ~__context ~self - in - Db.VM.set_secureboot_certificates_state ~__context ~self - ~value:new_state - with e -> - let vm_uuid = Db.VM.get_uuid ~__context ~self in - debug - "set_NVRAM_EFI_variables: failed to update secureboot_certificates_state \ - for VM %s: %s" - vm_uuid (Printexc.to_string e) + (* Update secureboot_certificates_state after NVRAM is written *) + match update with + | `yes -> + Db.VM.set_secureboot_certificates_state ~__context ~self ~value:`ok + | `no -> + () (* keep current state unchanged *) + | `unspecified -> + let new_state = + ( Xapi_vm_helpers.check_secureboot_certificates_state ~__context ~self + :> API.vm_secureboot_certificates_state + ) + in + Db.VM.set_secureboot_certificates_state ~__context ~self ~value:new_state let restart_device_models ~__context ~self = let power_state = Db.VM.get_power_state ~__context ~self in diff --git a/ocaml/xapi/xapi_vm_helpers.ml b/ocaml/xapi/xapi_vm_helpers.ml index 8b163e29c91..8d39dc9b339 100644 --- a/ocaml/xapi/xapi_vm_helpers.ml +++ b/ocaml/xapi/xapi_vm_helpers.ml @@ -1706,7 +1706,8 @@ let ensure_device_model_profile_present ~__context ~domain_type ~is_a_template (* only add device-model to an HVM VM platform if it is not already there *) default :: platform -let check_secureboot_certificates_state ~__context ~self = +let check_secureboot_certificates_state ~__context ~self : + [`ok | `update_available] = let vm_uuid = Db.VM.get_uuid ~__context ~self in let nvram = Db.VM.get_NVRAM ~__context ~self in match List.assoc_opt "EFI-variables" nvram with @@ -1714,6 +1715,7 @@ let check_secureboot_certificates_state ~__context ~self = D.info "VM %s has no EFI-variables in NVRAM, defaulting to ok" vm_uuid ; `ok | Some efi_vars -> ( + try let tmp_path = Filename.temp_file ("nvram-" ^ vm_uuid ^ "-") ".dat" in let result = finally @@ -1738,4 +1740,8 @@ let check_secureboot_certificates_state ~__context ~self = "varstore-nvram-certcheck returned unexpected output for VM %s: %s" vm_uuid other ; `ok - ) + with e -> + D.warn "Failed to check secureboot certificate state for VM %s: %s" + vm_uuid (Printexc.to_string e) ; + `ok + ) From 87ca912257c3dd945f1dc2b49d17c2668154e16a Mon Sep 17 00:00:00 2001 From: Chunjie Zhu Date: Tue, 21 Apr 2026 14:08:27 +0800 Subject: [PATCH 05/20] CP-311905: VM.update_secureboot_certificates_on_boot interface Signed-off-by: Chunjie Zhu --- ocaml/idl/datamodel_vm.ml | 15 +++ ocaml/tests/test_vm.ml | 124 +++++++++++++++++++++++- ocaml/xapi-cli-server/cli_frontend.ml | 10 ++ ocaml/xapi-cli-server/cli_operations.ml | 7 ++ ocaml/xapi/message_forwarding.ml | 6 ++ ocaml/xapi/xapi_vm.ml | 30 ++++++ ocaml/xapi/xapi_vm.mli | 3 + 7 files changed, 194 insertions(+), 1 deletion(-) diff --git a/ocaml/idl/datamodel_vm.ml b/ocaml/idl/datamodel_vm.ml index a70c3552398..982184ab9c2 100644 --- a/ocaml/idl/datamodel_vm.ml +++ b/ocaml/idl/datamodel_vm.ml @@ -2466,6 +2466,20 @@ let vm_secureboot_certificates_state = ] ) +let update_secureboot_certificates_on_boot = + call ~name:"update_secureboot_certificates_on_boot" ~lifecycle:[] + ~params: + [ + (Ref _vm, "self", "The VM") + ; ( Bool + , "mark" + , "If true: mark certificates for update on next boot. If false: \ + remove the mark" + ) + ] + ~doc:"Mark or unmark secure boot certificate update on VM boot" + ~allowed_roles:_R_VM_ADMIN () + let vm_secureboot_readiness = Enum ( "vm_secureboot_readiness" @@ -2640,6 +2654,7 @@ let t = ; restart_device_models ; set_uefi_mode ; get_secureboot_readiness + ; update_secureboot_certificates_on_boot ; set_blocked_operations ; add_to_blocked_operations ; remove_from_blocked_operations diff --git a/ocaml/tests/test_vm.ml b/ocaml/tests/test_vm.ml index 4bceac90ee8..ea82f65d14e 100644 --- a/ocaml/tests/test_vm.ml +++ b/ocaml/tests/test_vm.ml @@ -191,4 +191,126 @@ module VMSetBiosStrings = Generic.MakeStateful (struct `QuickAndAutoDocumented tests end) -let tests = [("test_vm_set_bios_strings", VMSetBiosStrings.tests)] +module VMSecurebootCertificatesStateField = Generic.MakeStateful (struct + module Io = struct + type input_t = [`ok | `update_available | `update_on_boot] + + type output_t = [`ok | `update_available | `update_on_boot] + + let string_of_state = function + | `ok -> + "ok" + | `update_available -> + "update_available" + | `update_on_boot -> + "update_on_boot" + + let string_of_input_t = string_of_state + + let string_of_output_t = string_of_state + end + + module State = Test_state.XapiDb + + let name_label = "secureboot-certs-state" + + let load_input __context state = + let self = Test_common.make_vm ~__context ~name_label () in + Db.VM.set_secureboot_certificates_state ~__context ~self ~value:state + + let extract_output __context _ = + let self = + List.nth (Db.VM.get_by_name_label ~__context ~label:name_label) 0 + in + Db.VM.get_secureboot_certificates_state ~__context ~self + + let tests = + `QuickAndAutoDocumented + [ + (`ok, `ok) + ; (`update_available, `update_available) + ; (`update_on_boot, `update_on_boot) + ] +end) + +module VMUpdateSecurebootCertificatesOnBoot = Generic.MakeStateful (struct + module Io = struct + type input_t = [`ok | `update_available | `update_on_boot] * bool + + type output_t = + ([`ok | `update_available | `update_on_boot], string * string list) result + + let string_of_state = function + | `ok -> + "ok" + | `update_available -> + "update_available" + | `update_on_boot -> + "update_on_boot" + + let string_of_input_t (state, mark) = + Printf.sprintf "(%s, mark=%b)" (string_of_state state) mark + + let string_of_error (code, args) = + Printf.sprintf "%s(%s)" code (String.concat ", " args) + + let string_of_output_t = function + | Ok state -> + Printf.sprintf "Ok %s" (string_of_state state) + | Error err -> + Printf.sprintf "Error %s" (string_of_error err) + end + + module State = Test_state.XapiDb + + let name_label = "update-secureboot-certs-on-boot" + + let load_input __context (state, _) = + let self = Test_common.make_vm ~__context ~name_label () in + Db.VM.set_secureboot_certificates_state ~__context ~self ~value:state + + let extract_output __context (_, mark) = + let self = + List.nth (Db.VM.get_by_name_label ~__context ~label:name_label) 0 + in + try + Xapi_vm.update_secureboot_certificates_on_boot ~__context ~self ~mark ; + Ok (Db.VM.get_secureboot_certificates_state ~__context ~self) + with Api_errors.Server_error (code, args) -> Error (code, args) + + let tests = + `QuickAndAutoDocumented + [ + ((`update_available, true), Ok `update_on_boot) + ; ((`update_on_boot, false), Ok `update_available) + ; ( (`ok, true) + , Error + ( Api_errors.operation_not_allowed + , [ + "Cannot set update_on_boot: VM.secureboot_certificates_state \ + is not update_available" + ] + ) + ) + ; ( (`ok, false) + , Error + ( Api_errors.operation_not_allowed + , [ + "Cannot clear update_on_boot: VM.secureboot_certificates_state \ + is not update_on_boot" + ] + ) + ) + ] +end) + +let tests = + [ + ("test_vm_set_bios_strings", VMSetBiosStrings.tests) + ; ( "test_vm_secureboot_certificates_state_field" + , VMSecurebootCertificatesStateField.tests + ) + ; ( "test_vm_update_secureboot_certificates_on_boot" + , VMUpdateSecurebootCertificatesOnBoot.tests + ) + ] diff --git a/ocaml/xapi-cli-server/cli_frontend.ml b/ocaml/xapi-cli-server/cli_frontend.ml index 02122f70eaa..58994e54579 100644 --- a/ocaml/xapi-cli-server/cli_frontend.ml +++ b/ocaml/xapi-cli-server/cli_frontend.ml @@ -2760,6 +2760,16 @@ let rec cmdtable_data : (string * cmd_spec) list = ; flags= [] } ) + ; ( "vm-update-secureboot-certificates-on-boot" + , { + reqd= ["uuid"; "mark"] + ; optn= [] + ; help= "Mark/unmark secure boot certificate update for next VM boot." + ; implementation= + No_fd Cli_operations.vm_update_secureboot_certificates_on_boot + ; flags= [] + } + ) ; ( "vm-group-create" , { reqd= ["name-label"; "placement"] diff --git a/ocaml/xapi-cli-server/cli_operations.ml b/ocaml/xapi-cli-server/cli_operations.ml index a19c52d94eb..b601b259346 100644 --- a/ocaml/xapi-cli-server/cli_operations.ml +++ b/ocaml/xapi-cli-server/cli_operations.ml @@ -6256,6 +6256,13 @@ let vm_get_secureboot_readiness printer rpc session_id params = printer (Cli_printer.PMsg (Record_util.vm_secureboot_readiness_to_string result)) +let vm_update_secureboot_certificates_on_boot _printer rpc session_id params = + let uuid = List.assoc "uuid" params in + let mark = get_bool_param params "mark" in + let vm = Client.VM.get_by_uuid ~rpc ~session_id ~uuid in + Client.VM.update_secureboot_certificates_on_boot ~rpc ~session_id ~self:vm + ~mark + let cd_list printer rpc session_id params = let srs = Client.SR.get_all_records_where ~rpc ~session_id ~expr:"true" in let cd_srs = diff --git a/ocaml/xapi/message_forwarding.ml b/ocaml/xapi/message_forwarding.ml index 249c92e42b8..254f5aa0026 100644 --- a/ocaml/xapi/message_forwarding.ml +++ b/ocaml/xapi/message_forwarding.ml @@ -3152,6 +3152,12 @@ functor info "VM.get_secureboot_readiness: self = '%s'" (vm_uuid ~__context self) ; Local.VM.get_secureboot_readiness ~__context ~self + let update_secureboot_certificates_on_boot ~__context ~self ~mark = + info + "VM.update_secureboot_certificates_on_boot: self = '%s'; mark = '%b'" + (vm_uuid ~__context self) mark ; + Local.VM.update_secureboot_certificates_on_boot ~__context ~self ~mark + let set_blocked_operations ~__context ~self ~value = info "VM.set_blocked_operations: self = '%s'" (vm_uuid ~__context self) ; Local.VM.set_blocked_operations ~__context ~self ~value ; diff --git a/ocaml/xapi/xapi_vm.ml b/ocaml/xapi/xapi_vm.ml index 367c9e4b43b..d6782c8bc8c 100644 --- a/ocaml/xapi/xapi_vm.ml +++ b/ocaml/xapi/xapi_vm.ml @@ -1754,6 +1754,36 @@ let get_secureboot_readiness ~__context ~self = ) ) +let update_secureboot_certificates_on_boot ~__context ~self ~mark = + let current = Db.VM.get_secureboot_certificates_state ~__context ~self in + match (mark, current) with + | true, `update_available -> + Db.VM.set_secureboot_certificates_state ~__context ~self + ~value:`update_on_boot + | false, `update_on_boot -> + Db.VM.set_secureboot_certificates_state ~__context ~self + ~value:`update_available + | true, _ -> + raise + (Api_errors.Server_error + ( Api_errors.operation_not_allowed + , [ + "Cannot set update_on_boot: VM.secureboot_certificates_state is \ + not update_available" + ] + ) + ) + | false, _ -> + raise + (Api_errors.Server_error + ( Api_errors.operation_not_allowed + , [ + "Cannot clear update_on_boot: VM.secureboot_certificates_state \ + is not update_on_boot" + ] + ) + ) + let sysprep ~__context ~self ~unattend ~timeout = let uuid = Db.VM.get_uuid ~__context ~self in debug "%s %S (timeout %f)" __FUNCTION__ uuid timeout ; diff --git a/ocaml/xapi/xapi_vm.mli b/ocaml/xapi/xapi_vm.mli index 16b3816e79c..ccaf36ea16e 100644 --- a/ocaml/xapi/xapi_vm.mli +++ b/ocaml/xapi/xapi_vm.mli @@ -439,6 +439,9 @@ val set_uefi_mode : val get_secureboot_readiness : __context:Context.t -> self:API.ref_VM -> API.vm_secureboot_readiness +val update_secureboot_certificates_on_boot : + __context:Context.t -> self:API.ref_VM -> mark:bool -> unit + val set_blocked_operations : __context:Context.t -> self:API.ref_VM From df2db1c0d82bb4240c01bc921e1c5b9a78809f7f Mon Sep 17 00:00:00 2001 From: Chunjie Zhu Date: Wed, 22 Apr 2026 12:30:37 +0800 Subject: [PATCH 06/20] CP-311905: VM.update_secureboot_certificates_on_boot interface Signed-off-by: Chunjie Zhu --- ocaml/xapi-cli-server/cli_frontend.ml | 4 ++-- ocaml/xapi-cli-server/cli_operations.ml | 14 +++++++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/ocaml/xapi-cli-server/cli_frontend.ml b/ocaml/xapi-cli-server/cli_frontend.ml index 58994e54579..2357934ecdf 100644 --- a/ocaml/xapi-cli-server/cli_frontend.ml +++ b/ocaml/xapi-cli-server/cli_frontend.ml @@ -2762,12 +2762,12 @@ let rec cmdtable_data : (string * cmd_spec) list = ) ; ( "vm-update-secureboot-certificates-on-boot" , { - reqd= ["uuid"; "mark"] + reqd= ["mark"] ; optn= [] ; help= "Mark/unmark secure boot certificate update for next VM boot." ; implementation= No_fd Cli_operations.vm_update_secureboot_certificates_on_boot - ; flags= [] + ; flags= [Vm_selectors] } ) ; ( "vm-group-create" diff --git a/ocaml/xapi-cli-server/cli_operations.ml b/ocaml/xapi-cli-server/cli_operations.ml index b601b259346..87c484d08fc 100644 --- a/ocaml/xapi-cli-server/cli_operations.ml +++ b/ocaml/xapi-cli-server/cli_operations.ml @@ -6256,12 +6256,16 @@ let vm_get_secureboot_readiness printer rpc session_id params = printer (Cli_printer.PMsg (Record_util.vm_secureboot_readiness_to_string result)) -let vm_update_secureboot_certificates_on_boot _printer rpc session_id params = - let uuid = List.assoc "uuid" params in +let vm_update_secureboot_certificates_on_boot printer rpc session_id params = let mark = get_bool_param params "mark" in - let vm = Client.VM.get_by_uuid ~rpc ~session_id ~uuid in - Client.VM.update_secureboot_certificates_on_boot ~rpc ~session_id ~self:vm - ~mark + ignore + (do_vm_op printer rpc session_id + (fun vm -> + Client.VM.update_secureboot_certificates_on_boot ~rpc ~session_id + ~self:(vm.getref ()) ~mark + ) + params ["mark"] + ) let cd_list printer rpc session_id params = let srs = Client.SR.get_all_records_where ~rpc ~session_id ~expr:"true" in From f669faadae65ba0e59091df814dd183f9d692623 Mon Sep 17 00:00:00 2001 From: Chunjie Zhu Date: Wed, 22 Apr 2026 17:51:38 +0800 Subject: [PATCH 07/20] CP-311905: VM.update_secureboot_certificates_on_boot idempotent Signed-off-by: Chunjie Zhu --- ocaml/tests/test_vm.ml | 16 +++++++++------- ocaml/xapi/xapi_vm.ml | 26 +++++++++++++------------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/ocaml/tests/test_vm.ml b/ocaml/tests/test_vm.ml index ea82f65d14e..cd4c3ca4621 100644 --- a/ocaml/tests/test_vm.ml +++ b/ocaml/tests/test_vm.ml @@ -251,14 +251,11 @@ module VMUpdateSecurebootCertificatesOnBoot = Generic.MakeStateful (struct let string_of_input_t (state, mark) = Printf.sprintf "(%s, mark=%b)" (string_of_state state) mark - let string_of_error (code, args) = - Printf.sprintf "%s(%s)" code (String.concat ", " args) - let string_of_output_t = function | Ok state -> Printf.sprintf "Ok %s" (string_of_state state) - | Error err -> - Printf.sprintf "Error %s" (string_of_error err) + | Error (code, args) -> + Printf.sprintf "Error %s(%s)" code (String.concat ", " args) end module State = Test_state.XapiDb @@ -281,14 +278,19 @@ module VMUpdateSecurebootCertificatesOnBoot = Generic.MakeStateful (struct let tests = `QuickAndAutoDocumented [ + (* transitions *) ((`update_available, true), Ok `update_on_boot) ; ((`update_on_boot, false), Ok `update_available) + (* idempotent: already in desired state *) + ; ((`update_on_boot, true), Ok `update_on_boot) + ; ((`update_available, false), Ok `update_available) + (* irrelevant state: raises *) ; ( (`ok, true) , Error ( Api_errors.operation_not_allowed , [ "Cannot set update_on_boot: VM.secureboot_certificates_state \ - is not update_available" + is not in a valid state" ] ) ) @@ -297,7 +299,7 @@ module VMUpdateSecurebootCertificatesOnBoot = Generic.MakeStateful (struct ( Api_errors.operation_not_allowed , [ "Cannot clear update_on_boot: VM.secureboot_certificates_state \ - is not update_on_boot" + is not in a valid state" ] ) ) diff --git a/ocaml/xapi/xapi_vm.ml b/ocaml/xapi/xapi_vm.ml index d6782c8bc8c..6216105eba6 100644 --- a/ocaml/xapi/xapi_vm.ml +++ b/ocaml/xapi/xapi_vm.ml @@ -1763,23 +1763,23 @@ let update_secureboot_certificates_on_boot ~__context ~self ~mark = | false, `update_on_boot -> Db.VM.set_secureboot_certificates_state ~__context ~self ~value:`update_available - | true, _ -> + | true, `update_on_boot -> + () (* already marked for update on boot — idempotent *) + | false, `update_available -> + () (* already cleared — idempotent *) + | _, _ -> raise (Api_errors.Server_error ( Api_errors.operation_not_allowed , [ - "Cannot set update_on_boot: VM.secureboot_certificates_state is \ - not update_available" - ] - ) - ) - | false, _ -> - raise - (Api_errors.Server_error - ( Api_errors.operation_not_allowed - , [ - "Cannot clear update_on_boot: VM.secureboot_certificates_state \ - is not update_on_boot" + Printf.sprintf + "Cannot %s update_on_boot: VM.secureboot_certificates_state \ + is not in a valid state" + ( if mark then + "set" + else + "clear" + ) ] ) ) From e0f27de8b701bd26c08d2fb5a884090eef3527b1 Mon Sep 17 00:00:00 2001 From: Chunjie Zhu Date: Thu, 23 Apr 2026 14:30:12 +0800 Subject: [PATCH 08/20] CP-311905: fix make test failure Signed-off-by: Chunjie Zhu --- opam/tgroup.opam | 1 + 1 file changed, 1 insertion(+) diff --git a/opam/tgroup.opam b/opam/tgroup.opam index 6080d67b802..6a0a2535f3f 100644 --- a/opam/tgroup.opam +++ b/opam/tgroup.opam @@ -1,5 +1,6 @@ # This file is generated by dune, edit dune-project instead opam-version: "2.0" +synopsis: "Thread group management library" maintainer: ["Xapi project maintainers"] authors: ["xen-api@lists.xen.org"] license: "LGPL-2.1-only WITH OCaml-LGPL-linking-exception" From 4b53b2be0e70343d38d35aa6120805398d2f507b Mon Sep 17 00:00:00 2001 From: Chunjie Zhu Date: Sat, 9 May 2026 17:11:24 +0800 Subject: [PATCH 09/20] CA-427271: update non secureboot vm certificate state Signed-off-by: Chunjie Zhu --- ocaml/tests/test_xapi_db_upgrade.ml | 26 ++++++++++++++++++++++++++ ocaml/xapi/xapi_db_upgrade.ml | 7 +------ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/ocaml/tests/test_xapi_db_upgrade.ml b/ocaml/tests/test_xapi_db_upgrade.ml index 99f30140a41..fb95e13b4e9 100644 --- a/ocaml/tests/test_xapi_db_upgrade.ml +++ b/ocaml/tests/test_xapi_db_upgrade.ml @@ -88,9 +88,35 @@ let remove_restricted_pbd_keys () = ) other_keys +let upgrade_secureboot_certificates_state_for_uefi_vm_without_secureboot () = + let __context = T.make_test_database () in + let vm = + T.make_vm ~__context ~name_label:"uefi-no-secureboot" + ~hVM_boot_params:[("firmware", "uefi")] + ~platform:[("secureboot", "false")] + () + in + Db.VM.set_secureboot_certificates_state ~__context ~self:vm + ~value:`update_available ; + Alcotest.(check string) + "precondition: state starts as update_available" "update_available" + (Record_util.vm_secureboot_certificates_state_to_string + (Db.VM.get_secureboot_certificates_state ~__context ~self:vm) + ) ; + X.upgrade_secureboot_certificates_state.fn ~__context ; + Alcotest.(check string) + "UEFI VM without secureboot still gets checked" "ok" + (Record_util.vm_secureboot_certificates_state_to_string + (Db.VM.get_secureboot_certificates_state ~__context ~self:vm) + ) + let test = [ ("upgrade_bios", `Quick, upgrade_bios) ; ("update_snapshots", `Quick, update_snapshots) ; ("remove_restricted_pbd_keys", `Quick, remove_restricted_pbd_keys) + ; ( "upgrade_secureboot_certificates_state_for_uefi_vm_without_secureboot" + , `Quick + , upgrade_secureboot_certificates_state_for_uefi_vm_without_secureboot + ) ] diff --git a/ocaml/xapi/xapi_db_upgrade.ml b/ocaml/xapi/xapi_db_upgrade.ml index 9c000a27950..7a5e1fdd8f6 100644 --- a/ocaml/xapi/xapi_db_upgrade.ml +++ b/ocaml/xapi/xapi_db_upgrade.ml @@ -983,12 +983,7 @@ let upgrade_secureboot_certificates_state = let is_uefi = List.assoc_opt "firmware" boot_params = Some "uefi" in - let platformdata = Db.VM.get_platform ~__context ~self in - let is_secureboot = - Vm_platform.is_true ~key:"secureboot" ~platformdata - ~default:false - in - if is_uefi && is_secureboot then + if is_uefi then let state = ( Xapi_vm_helpers.check_secureboot_certificates_state ~__context ~self From 5d8d70592de77c3ea7ccac226b152c223783ed1b Mon Sep 17 00:00:00 2001 From: Changlei Li Date: Thu, 30 Apr 2026 16:51:22 +0800 Subject: [PATCH 10/20] CA-426408: Consider chunks in task expiry calculation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A test of VM with 239 VBDs (1 system disk + 238 iSCSI VBDs) fails because of `Timed out while waiting on task xxx`. In the fail task: ``` VM.start (task 61) → perform_atomics([..., Parallel("Devices.plug"), ...]) → perform_atomic(Parallel("Devices.plug")) ↓ Parallel:task=61.atoms=2.(Devices.plug (no qemu)) │ │ queue_atomics_and_wait(ops=[atom0, atom1]) │ expiration(atom0) = atomic_expires_after(Nested_parallel(239)) │ = Float.max(3600, ...) = 3600s ← BUG │ event_wait(task=79, timeout=3600s) │ ├─ task 79 (worker): Nested_parallel:task=79.atoms=239.(VBDs.activate_epoch_and_plug RW) │ │ │ │ queue_atomics_and_wait(239 ops, chunks of 10) │ ├─ chunk 0: 10 × Serial(VBD_set_active, VBD_epoch_begin, VBD_plug) │ ├─ chunk 1: 10 × ... │ ├─ ... │ └─ chunk 23: 9 × ... │ └─ total: 82 minutes │ └─ task 80 (worker): Serial(VIF_set_active, VIF_plug) ``` The task expiry calculation for parallel and nested_parallel doesn't consider the multi chunks. Just get the max expiry in the parallel task list. In fact the parallel task split to 24 chunks to run. Signed-off-by: Changlei Li --- ocaml/xenopsd/lib/xenops_server.ml | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/ocaml/xenopsd/lib/xenops_server.ml b/ocaml/xenopsd/lib/xenops_server.ml index 3db369d0d57..e1e2cae9109 100644 --- a/ocaml/xenopsd/lib/xenops_server.ml +++ b/ocaml/xenopsd/lib/xenops_server.ml @@ -303,11 +303,19 @@ let rec name_of_atomic = function | Best_effort atomic -> Printf.sprintf "Best_effort (%s)" (name_of_atomic atomic) +let max_parallel_atoms = 10 + let rec atomic_expires_after = function | Serial (_, _, ops) -> List.map atomic_expires_after ops |> List.fold_left ( +. ) 0. | Parallel (_, _, ops) | Nested_parallel (_, _, ops) -> - List.map atomic_expires_after ops |> List.fold_left Float.max 0. + let chunk_count = + (List.length ops + max_parallel_atoms - 1) / max_parallel_atoms + in + let per_chunk = + List.map atomic_expires_after ops |> List.fold_left Float.max 0. + in + per_chunk *. float_of_int chunk_count | _ -> (* 20 minutes, in seconds *) 1200. @@ -2010,8 +2018,8 @@ and parallel_atomic ~progress_callback ~description ~nested atoms t = let with_tracing = id_with_tracing parallel_id t in debug "begin_%s" parallel_id ; let task_list = - queue_atomics_and_wait ~progress_callback ~max_parallel_atoms:10 - with_tracing parallel_id atoms redirector + queue_atomics_and_wait ~progress_callback with_tracing parallel_id atoms + redirector in debug "end_%s" parallel_id ; (* make sure that we destroy all the parallel tasks that finished *) @@ -2067,8 +2075,7 @@ and queue_atomic_int ~progress_callback dbg id op redirector = Redirector.push redirector id item ; task -and queue_atomics_and_wait ~progress_callback ~max_parallel_atoms dbg id ops - redirector = +and queue_atomics_and_wait ~progress_callback dbg id ops redirector = let from = Updates.last_id dbg updates in Xenops_utils.chunks max_parallel_atoms ops |> List.mapi (fun chunk_idx ops -> From 01aeb977286fe472ea0dc7b78531af65716d6d87 Mon Sep 17 00:00:00 2001 From: Chunjie Zhu Date: Fri, 15 May 2026 13:31:41 +0800 Subject: [PATCH 11/20] update datamodel version and tgroup opam Signed-off-by: Chunjie Zhu --- dune-project | 1 + ocaml/idl/datamodel_common.ml | 2 +- ocaml/idl/datamodel_lifecycle.ml | 4 ++-- ocaml/xapi/xapi_db_upgrade.ml | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/dune-project b/dune-project index 548b8b5efad..7fb1ad9fc89 100644 --- a/dune-project +++ b/dune-project @@ -50,6 +50,7 @@ (package (name tgroup) + (synopsis "Thread group management library") (depends xapi-log xapi-stdext-unix)) (package diff --git a/ocaml/idl/datamodel_common.ml b/ocaml/idl/datamodel_common.ml index 25e689f4fa4..fd3ed22710a 100644 --- a/ocaml/idl/datamodel_common.ml +++ b/ocaml/idl/datamodel_common.ml @@ -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 = 902 +let schema_minor_vsn = 903 (* Historical schema versions just in case this is useful later *) let rio_schema_major_vsn = 5 diff --git a/ocaml/idl/datamodel_lifecycle.ml b/ocaml/idl/datamodel_lifecycle.ml index c2644dc6c34..4e90114d45c 100644 --- a/ocaml/idl/datamodel_lifecycle.ml +++ b/ocaml/idl/datamodel_lifecycle.ml @@ -146,7 +146,7 @@ let prototyped_of_field = function | "VM_metrics", "numa_optimised" -> Some "26.2.0" | "VM", "secureboot_certificates_state" -> - Some "26.12.0" + Some "26.13.0-next" | "VM", "groups" -> Some "24.19.1" | "VM", "pending_guidances_full" -> @@ -292,7 +292,7 @@ let prototyped_of_message = function | "VM", "sysprep" -> Some "25.24.0" | "VM", "update_secureboot_certificates_on_boot" -> - Some "26.12.0" + Some "26.13.0-next" | "VM", "get_secureboot_readiness" -> Some "24.17.0" | "VM", "set_uefi_mode" -> diff --git a/ocaml/xapi/xapi_db_upgrade.ml b/ocaml/xapi/xapi_db_upgrade.ml index 7a5e1fdd8f6..d1641dd6781 100644 --- a/ocaml/xapi/xapi_db_upgrade.ml +++ b/ocaml/xapi/xapi_db_upgrade.ml @@ -968,7 +968,7 @@ let upgrade_ca_fingerprints = let upgrade_secureboot_certificates_state = { description= "Set secureboot_certificates_state for existing VMs" - ; version= (fun x -> x < (5, 902)) + ; version= (fun x -> x < (5, 903)) ; fn= (fun ~__context -> List.iter From 7cda7c7991792e98b0a578f03f2c0b14ab79cffd Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Fri, 15 May 2026 13:31:47 +0100 Subject: [PATCH 12/20] CA-427441: Avoid using proxy for local repositories If you have a global proxy set in /etc/dnf/dnf.conf local repositories (pointing to http://127.0.0.1 ...) are not working. A proxy should not be used for such repositories. Signed-off-by: Frediano Ziglio --- ocaml/xapi/repository_helpers.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ocaml/xapi/repository_helpers.ml b/ocaml/xapi/repository_helpers.ml index 3bce3becf47..d3c789127a8 100644 --- a/ocaml/xapi/repository_helpers.ml +++ b/ocaml/xapi/repository_helpers.ml @@ -446,8 +446,8 @@ let with_local_repositories ~__context f = s in remove_repo_conf_file repo_name ; - write_yum_config ~proxy_config:[] ~source_url:None ~binary_url - ~repo_gpgcheck:false ~gpgkey_path ~repo_name ; + write_yum_config ~proxy_config:["proxy="] ~source_url:None + ~binary_url ~repo_gpgcheck:false ~gpgkey_path ~repo_name ; clean_yum_cache repo_name ; let Pkg_mgr.{cmd; params} = [ From c7741bc5f3f38bc32d97e7ed4980eb549cf5a0aa Mon Sep 17 00:00:00 2001 From: Changlei Li Date: Mon, 18 May 2026 09:32:02 +0800 Subject: [PATCH 13/20] Change to accumulate max timout of every chunk Signed-off-by: Changlei Li --- ocaml/xenopsd/lib/xenops_server.ml | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/ocaml/xenopsd/lib/xenops_server.ml b/ocaml/xenopsd/lib/xenops_server.ml index e1e2cae9109..92fd8d55c16 100644 --- a/ocaml/xenopsd/lib/xenops_server.ml +++ b/ocaml/xenopsd/lib/xenops_server.ml @@ -309,13 +309,11 @@ let rec atomic_expires_after = function | Serial (_, _, ops) -> List.map atomic_expires_after ops |> List.fold_left ( +. ) 0. | Parallel (_, _, ops) | Nested_parallel (_, _, ops) -> - let chunk_count = - (List.length ops + max_parallel_atoms - 1) / max_parallel_atoms - in - let per_chunk = - List.map atomic_expires_after ops |> List.fold_left Float.max 0. - in - per_chunk *. float_of_int chunk_count + let expires_of = List.map atomic_expires_after in + let max_of = List.fold_left Float.max 0. in + Xenops_utils.chunks max_parallel_atoms ops + |> List.map (fun ops -> ops |> expires_of |> max_of) + |> List.fold_left ( +. ) 0. | _ -> (* 20 minutes, in seconds *) 1200. @@ -2096,6 +2094,7 @@ and queue_atomics_and_wait ~progress_callback dbg id ops redirector = (fun (task, op) -> let task_id = Xenops_task.id_of_handle task in let expiration = atomic_expires_after op in + debug "Task %s expires_after=%f" task_id expiration ; let completion = event_wait updates task ~from ~timeout_start expiration (is_task task_id) task_ended From 4dde0894058a050de2649167bbed289c1563c4fa Mon Sep 17 00:00:00 2001 From: Stephen Cheng Date: Mon, 18 May 2026 12:52:44 +0800 Subject: [PATCH 14/20] CA-406020: parse dnf5 output in pool_update plugins dnf was upgraded to dnf5, which prints transaction errors in a new format. The existing parsers in pool_update.precheck and pool_update.apply were written against dnf4 and silently fell through to a generic "unknown error" for every failure mode, hiding the real reason from XAPI. Signed-off-by: Stephen Cheng --- python3/extensions/pool_update.apply | 31 ++-------- python3/extensions/pool_update.precheck | 79 ++++++++++++++----------- 2 files changed, 47 insertions(+), 63 deletions(-) diff --git a/python3/extensions/pool_update.apply b/python3/extensions/pool_update.apply index 96d00adcf32..9f353aaf5b2 100644 --- a/python3/extensions/pool_update.apply +++ b/python3/extensions/pool_update.apply @@ -4,7 +4,6 @@ import errno import logging import os -import re import shutil import subprocess import sys @@ -14,17 +13,6 @@ import fasteners import xcp.logger import XenAPI -TMP_DIR = "/tmp/" -UPDATE_ALREADY_APPLIED = "UPDATE_ALREADY_APPLIED" -UPDATE_APPLY_FAILED = "UPDATE_APPLY_FAILED" -OTHER_OPERATION_IN_PROGRESS = "OTHER_OPERATION_IN_PROGRESS" -UPDATE_PRECHECK_FAILED_UNKNOWN_ERROR = "UPDATE_PRECHECK_FAILED_UNKNOWN_ERROR" -CANNOT_FIND_UPDATE = "CANNOT_FIND_UPDATE" -INVALID_UPDATE = "INVALID_UPDATE" -ERROR_MESSAGE_DOWNLOAD_PACKAGE = "Error downloading packages:\n" -ERROR_MESSAGE_START = "Error: " -ERROR_MESSAGE_END = "You could try " - TMP_DIR = '/tmp/' UPDATE_ALREADY_APPLIED = 'UPDATE_ALREADY_APPLIED' UPDATE_APPLY_FAILED = 'UPDATE_APPLY_FAILED' @@ -32,12 +20,9 @@ OTHER_OPERATION_IN_PROGRESS = 'OTHER_OPERATION_IN_PROGRESS' UPDATE_PRECHECK_FAILED_UNKNOWN_ERROR = 'UPDATE_PRECHECK_FAILED_UNKNOWN_ERROR' CANNOT_FIND_UPDATE = 'CANNOT_FIND_UPDATE' INVALID_UPDATE = 'INVALID_UPDATE' -ERROR_MESSAGE_DOWNLOAD_PACKAGE = 'Error downloading packages:\n' -ERROR_MESSAGE_START = 'Error: ' -ERROR_MESSAGE_END = 'You could try ' -YUM_CMD = '/usr/bin/yum' +ERROR_MESSAGE_DOWNLOAD_PACKAGE = 'Failed to download packages' DNF_CMD = '/usr/bin/dnf' -PKG_MGR = DNF_CMD if os.path.exists(DNF_CMD) else YUM_CMD +PKG_MGR = DNF_CMD class EnvironmentFailure(Exception): """Failure due to running environment""" @@ -81,10 +66,8 @@ def execute_apply(update_package, yum_conf_file): if p.returncode != 0: raise EnvironmentFailure("Error cleaning yum cache") - # dnf reject to upgrade group if it is not installed, - # `dnf install` upgrade the group if it is already installed - sub_cmd = 'upgrade' if PKG_MGR == YUM_CMD else 'install' - cmd = [PKG_MGR, sub_cmd, '-y', '--noplugins', '-c', yum_conf_file, update_package] + # `dnf install` upgrades the group if it is already installed. + cmd = [PKG_MGR, 'install', '-y', '--noplugins', '-c', yum_conf_file, update_package] p = subprocess.Popen(cmd, shell=False, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, close_fds=True, env=yum_env, universal_newlines=True) output, _ = p.communicate() @@ -94,12 +77,6 @@ def execute_apply(update_package, yum_conf_file): if p.returncode != 0: if ERROR_MESSAGE_DOWNLOAD_PACKAGE in output: raise InvalidUpdate("Missing package(s) in the update.") - - m = re.search("(?<=" + ERROR_MESSAGE_START + ").+$", output, flags=re.DOTALL) - if m: - errmsg = m.group() - errmsg = re.sub(ERROR_MESSAGE_END + ".+", "", errmsg, flags=re.DOTALL) - raise ApplyFailure(errmsg) raise ApplyFailure(output) diff --git a/python3/extensions/pool_update.precheck b/python3/extensions/pool_update.precheck index 8e5f1858668..5d0cead6557 100755 --- a/python3/extensions/pool_update.precheck +++ b/python3/extensions/pool_update.precheck @@ -31,17 +31,19 @@ PATCH_PRECHECK_FAILED_ISO_MOUNTED = 'PATCH_PRECHECK_FAILED_ISO_MOUNTED' PATCH_PRECHECK_FAILED_VM_RUNNING = 'PATCH_PRECHECK_FAILED_VM_RUNNING' INVALID_UPDATE = 'INVALID_UPDATE' CANNOT_FIND_UPDATE = 'CANNOT_FIND_UPDATE' -ERROR_MESSAGE_START = 'Error: ' -ERROR_MESSAGE_END = 'You could try ' +ERROR_MESSAGE_FAILED_TO_RESOLVE = 'Failed to resolve the transaction:\n' +ERROR_MESSAGE_END = 'You can try to add to command line:' ERROR_MESSAGE_CONFLICTS_WITH = ' conflicts with ' -ERROR_MESSAGE_CONFLICTS = 'conflicts ' -ERROR_MESSAGE_PROCESSING_CONFLICT = '--> Processing Conflict:' -ERROR_MESSAGE_PREREQUISITE = 'Requires: ' -ERROR_MESSAGE_VERSION_REQUIRED = 'Requires: ' -ERROR_MESSAGE_VERSION_INSTALLED = 'Installed: ' -ERROR_MESSAGE_VERSION_UPDATED_BY = 'Updated By: ' -ERROR_MESSAGE_DOWNLOAD_PACKAGE = 'Error downloading packages:\n' -ERROR_MESSAGE_GPGKEY_NOT_IMPORTED = 'Gpg Keys not imported' +ERROR_MESSAGE_NOTHING_PROVIDES = 'nothing provides ' +ERROR_MESSAGE_REQUIRES = ' requires ' +ERROR_MESSAGE_DOWNLOAD_PACKAGE = 'Failed to download packages' +ERROR_MESSAGE_GPGKEY_NOT_IMPORTED_PATTERNS = ( + 'Signature verification failed.', + 'Public key is not installed.', + 'Public key import failed.', + 'The package is not signed.', + 'GPG signature verification error', +) ERROR_XML_START = '' ERRORCODE = 'errorcode' @@ -165,7 +167,15 @@ def execute_precheck(control_package, yum_conf_file, update_precheck_file): if ERROR_MESSAGE_DOWNLOAD_PACKAGE in output: raise InvalidUpdate('Missing package(s) in the update') - if ERROR_MESSAGE_GPGKEY_NOT_IMPORTED in output: + # GPG patterns must only be matched when dnf5 never reached the solver. + # Librepo prints transient messages like + # ">>> Librepo error: repomd.xml GPG signature verification error: ..." + # before auto-importing keys; those appear even on successful runs and + # also precede genuine resolution failures. If a "Failed to resolve" + # block is present, the real error is there, not in the GPG noise. + if (ERROR_MESSAGE_FAILED_TO_RESOLVE not in output + and any(pat in output + for pat in ERROR_MESSAGE_GPGKEY_NOT_IMPORTED_PATTERNS)): raise GpgkeyNotImported() if PATCH_PRECHECK_FAILED_ISO_MOUNTED in output: @@ -174,34 +184,31 @@ def execute_precheck(control_package, yum_conf_file, update_precheck_file): if PATCH_PRECHECK_FAILED_VM_RUNNING in output: raise VmRunning - m = re.search('(?<=' + ERROR_MESSAGE_START + ').+$', output, flags=re.DOTALL) + m = re.search(ERROR_MESSAGE_FAILED_TO_RESOLVE + '(.+)$', + output, flags=re.DOTALL) if m: - errmsg = m.group() + errmsg = m.group(1) errmsg = re.sub(ERROR_MESSAGE_END + '.+', '', errmsg, flags=re.DOTALL) - if (ERROR_MESSAGE_CONFLICTS_WITH in errmsg and - ERROR_MESSAGE_PROCESSING_CONFLICT in output): - regex = (ERROR_MESSAGE_PROCESSING_CONFLICT + '(.*)' - + ERROR_MESSAGE_CONFLICTS + '(.+?)\n') - conflict_tuples = re.findall(regex, output) - if conflict_tuples: - raise ConflictPresent(' '.join([tup[1] for tup in conflict_tuples])) - raise PrecheckFailure(errmsg) - if (ERROR_MESSAGE_VERSION_REQUIRED in errmsg and - (ERROR_MESSAGE_VERSION_INSTALLED in errmsg - or ERROR_MESSAGE_VERSION_UPDATED_BY in errmsg)): - regex = ERROR_MESSAGE_VERSION_REQUIRED + '(.+?)\n.+ {2,2}(.+)$' - match = re.search(regex, errmsg, flags=re.DOTALL) - if match: - required_version = match.group(1).rstrip() - installed_version = match.group(2).rstrip() - raise WrongServerVersion(required_version, installed_version) - raise PrecheckFailure(errmsg) - if ERROR_MESSAGE_PREREQUISITE in errmsg: - regex = ERROR_MESSAGE_PREREQUISITE + '(.+?)\n' - prerequisite_list = re.findall(regex, errmsg) - if prerequisite_list: - raise PrerequisiteMissing(' '.join(prerequisite_list)) + if ERROR_MESSAGE_CONFLICTS_WITH in errmsg: + conflicts = re.findall( + ERROR_MESSAGE_CONFLICTS_WITH + r'(\S+)', errmsg) + if conflicts: + raise ConflictPresent(' '.join(conflicts)) raise PrecheckFailure(errmsg) + wrong_version = re.search( + ERROR_MESSAGE_NOTHING_PROVIDES + + r'(\S+\s*[<>=]+\s*\S+) needed by (\S+)', errmsg) + if wrong_version: + required = wrong_version.group(1).strip() + installed = wrong_version.group(2).strip() + raise WrongServerVersion(required, installed) + prerequisites = re.findall( + ERROR_MESSAGE_NOTHING_PROVIDES + r'(.+?)(?: needed by | from |\n)', + errmsg) + prerequisites += re.findall( + ERROR_MESSAGE_REQUIRES + r'(.+?), but none of', errmsg) + if prerequisites: + raise PrerequisiteMissing(' '.join(prerequisites)) raise PrecheckFailure(errmsg) regex = ERROR_XML_START + '.+' + ERROR_XML_END From 4bf3f06cc7944a9401ff082adee14d21b4c48706 Mon Sep 17 00:00:00 2001 From: Changlei Li Date: Tue, 19 May 2026 10:44:16 +0800 Subject: [PATCH 15/20] CP-311530 Bump up api_version to 2.23 The api_version is actually rewriten by flag passed by `--xapi_api_version_minor`. Here bump up the value just affects the local build with make command. Signed-off-by: Changlei Li --- configure.ml | 2 +- ocaml/idl/api_version.ml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ml b/configure.ml index 91eee0f0ed7..607d3f58dae 100644 --- a/configure.ml +++ b/configure.ml @@ -58,7 +58,7 @@ let args = ; flag "yumpluginconfdir" ~doc:"DIR YUM plugins conf dir" ~default:"/etc/yum/pluginconf.d" ; flag "xapi_api_version_major" ~doc:"xapi api major version" ~default:"2" - ; flag "xapi_api_version_minor" ~doc:"xapi api minor version" ~default:"21" + ; flag "xapi_api_version_minor" ~doc:"xapi api minor version" ~default:"23" ] |> Arg.align diff --git a/ocaml/idl/api_version.ml b/ocaml/idl/api_version.ml index 23028c50796..b73173fbd1f 100644 --- a/ocaml/idl/api_version.ml +++ b/ocaml/idl/api_version.ml @@ -18,4 +18,4 @@ let api_version_major = 2L -let api_version_minor = 21L +let api_version_minor = 23L From 1dd5940ee454c5912dde43c22a72c794d8f45e50 Mon Sep 17 00:00:00 2001 From: Changlei Li Date: Tue, 19 May 2026 13:28:01 +0800 Subject: [PATCH 16/20] Add rel_orinoco Signed-off-by: Changlei Li --- ocaml/idl/datamodel_types.ml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/ocaml/idl/datamodel_types.ml b/ocaml/idl/datamodel_types.ml index 1be90528856..72b977b18b8 100644 --- a/ocaml/idl/datamodel_types.ml +++ b/ocaml/idl/datamodel_types.ml @@ -101,6 +101,8 @@ let rel_nile_preview = "nile-preview" let rel_nile = "nile" +let rel_orinoco = "orinoco" + type api_release = { code_name: string option ; version_major: int @@ -352,6 +354,13 @@ let release_order_full = ; branding= "XenServer 8" ; release_date= None } + ; { + code_name= Some rel_orinoco + ; version_major= 2 + ; version_minor= 23 + ; branding= "XenServer 9" + ; release_date= None + } ] (* When you add a new release, use the version number of the latest release, "Unreleased" for the branding, and Some "" for the release date, until the actual values are finalised. *) From 717068b8b903ec1f59155343e37912b9eb83d1be Mon Sep 17 00:00:00 2001 From: Changlei Li Date: Tue, 19 May 2026 17:04:09 +0800 Subject: [PATCH 17/20] Update schema_hash Signed-off-by: Changlei Li --- ocaml/idl/schematest.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ocaml/idl/schematest.ml b/ocaml/idl/schematest.ml index c4b13767caa..889a8637498 100644 --- a/ocaml/idl/schematest.ml +++ b/ocaml/idl/schematest.ml @@ -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 = "19eb067e98133189f1dc2c55f7ccec70" +let last_known_schema_hash = "fe75fa03d7ce97f9e51426bfb9b078fe" let current_schema_hash : string = let open Datamodel_types in From b2ca8b7cd10aba1bdde31fcd418561a97da04314 Mon Sep 17 00:00:00 2001 From: Stephen Cheng Date: Wed, 20 May 2026 13:37:35 +0800 Subject: [PATCH 18/20] CA-406020: Add Unit tests for the dnf5 output parser Signed-off-by: Stephen Cheng --- python3/extensions/pool_update.precheck | 122 +++++++------- python3/tests/test_pool_update_precheck.py | 181 +++++++++++++++++++++ 2 files changed, 248 insertions(+), 55 deletions(-) create mode 100644 python3/tests/test_pool_update_precheck.py diff --git a/python3/extensions/pool_update.precheck b/python3/extensions/pool_update.precheck index 5d0cead6557..792128f18be 100755 --- a/python3/extensions/pool_update.precheck +++ b/python3/extensions/pool_update.precheck @@ -134,6 +134,70 @@ def parse_precheck_failure(xmldoc): raise PrecheckError(code, *params) raise PrecheckFailure(xmldoc.toxml()) + +def classify_dnf_failure(output): + """Classify a failed `dnf install` output into a precheck exception. + + Always raises; the caller is responsible for the fallback when none + of the known patterns match (in which case this function simply + returns and the caller raises PrecheckFailure(output)). + """ + if ERROR_MESSAGE_DOWNLOAD_PACKAGE in output: + raise InvalidUpdate('Missing package(s) in the update') + + # GPG patterns must only be matched when dnf5 never reached the solver. + # Librepo prints transient messages like + # ">>> Librepo error: repomd.xml GPG signature verification error: ..." + # before auto-importing keys; those appear even on successful runs and + # also precede genuine resolution failures. If a "Failed to resolve" + # block is present, the real error is there, not in the GPG noise. + if (ERROR_MESSAGE_FAILED_TO_RESOLVE not in output + and any(pat in output + for pat in ERROR_MESSAGE_GPGKEY_NOT_IMPORTED_PATTERNS)): + raise GpgkeyNotImported() + + if PATCH_PRECHECK_FAILED_ISO_MOUNTED in output: + raise IsoMounted + + if PATCH_PRECHECK_FAILED_VM_RUNNING in output: + raise VmRunning + + m = re.search(ERROR_MESSAGE_FAILED_TO_RESOLVE + '(.+)$', + output, flags=re.DOTALL) + if m: + errmsg = m.group(1) + errmsg = re.sub(ERROR_MESSAGE_END + '.+', '', errmsg, flags=re.DOTALL) + if ERROR_MESSAGE_CONFLICTS_WITH in errmsg: + conflicts = re.findall( + ERROR_MESSAGE_CONFLICTS_WITH + r'(\S+)', errmsg) + if conflicts: + raise ConflictPresent(' '.join(conflicts)) + raise PrecheckFailure(errmsg) + wrong_version = re.search( + ERROR_MESSAGE_NOTHING_PROVIDES + + r'(\S+\s*[<>=]+\s*\S+) needed by (\S+)', errmsg) + if wrong_version: + required = wrong_version.group(1).strip() + installed = wrong_version.group(2).strip() + raise WrongServerVersion(required, installed) + prerequisites = re.findall( + ERROR_MESSAGE_NOTHING_PROVIDES + r'(.+?)(?: needed by | from |\n)', + errmsg) + prerequisites += re.findall( + ERROR_MESSAGE_REQUIRES + r'(.+?), but none of', errmsg) + if prerequisites: + raise PrerequisiteMissing(' '.join(prerequisites)) + raise PrecheckFailure(errmsg) + + regex = ERROR_XML_START + '.+' + ERROR_XML_END + m = re.search(regex, output, flags=re.DOTALL) + if m: + try: + xmldoc = xml.dom.minidom.parseString(m.group(0)) + except Exception as e: + raise PrecheckFailure(output) from e + parse_precheck_failure(xmldoc) + def execute_precheck(control_package, yum_conf_file, update_precheck_file): #pylint: disable=too-many-locals #pylint: disable=too-many-branches @@ -164,61 +228,9 @@ def execute_precheck(control_package, yum_conf_file, update_precheck_file): for line in output.split('\n'): xcp.logger.info(line) if p.returncode != 0: - if ERROR_MESSAGE_DOWNLOAD_PACKAGE in output: - raise InvalidUpdate('Missing package(s) in the update') - - # GPG patterns must only be matched when dnf5 never reached the solver. - # Librepo prints transient messages like - # ">>> Librepo error: repomd.xml GPG signature verification error: ..." - # before auto-importing keys; those appear even on successful runs and - # also precede genuine resolution failures. If a "Failed to resolve" - # block is present, the real error is there, not in the GPG noise. - if (ERROR_MESSAGE_FAILED_TO_RESOLVE not in output - and any(pat in output - for pat in ERROR_MESSAGE_GPGKEY_NOT_IMPORTED_PATTERNS)): - raise GpgkeyNotImported() - - if PATCH_PRECHECK_FAILED_ISO_MOUNTED in output: - raise IsoMounted - - if PATCH_PRECHECK_FAILED_VM_RUNNING in output: - raise VmRunning - - m = re.search(ERROR_MESSAGE_FAILED_TO_RESOLVE + '(.+)$', - output, flags=re.DOTALL) - if m: - errmsg = m.group(1) - errmsg = re.sub(ERROR_MESSAGE_END + '.+', '', errmsg, flags=re.DOTALL) - if ERROR_MESSAGE_CONFLICTS_WITH in errmsg: - conflicts = re.findall( - ERROR_MESSAGE_CONFLICTS_WITH + r'(\S+)', errmsg) - if conflicts: - raise ConflictPresent(' '.join(conflicts)) - raise PrecheckFailure(errmsg) - wrong_version = re.search( - ERROR_MESSAGE_NOTHING_PROVIDES - + r'(\S+\s*[<>=]+\s*\S+) needed by (\S+)', errmsg) - if wrong_version: - required = wrong_version.group(1).strip() - installed = wrong_version.group(2).strip() - raise WrongServerVersion(required, installed) - prerequisites = re.findall( - ERROR_MESSAGE_NOTHING_PROVIDES + r'(.+?)(?: needed by | from |\n)', - errmsg) - prerequisites += re.findall( - ERROR_MESSAGE_REQUIRES + r'(.+?), but none of', errmsg) - if prerequisites: - raise PrerequisiteMissing(' '.join(prerequisites)) - raise PrecheckFailure(errmsg) - - regex = ERROR_XML_START + '.+' + ERROR_XML_END - m = re.search(regex, output, flags=re.DOTALL) - if m: - try: - xmldoc = xml.dom.minidom.parseString(m.group(0)) - except Exception as e: - raise PrecheckFailure(output) from e - parse_precheck_failure(xmldoc) + classify_dnf_failure(output) + # classify_dnf_failure raises in every case it understands; if we get + # here it didn't match any pattern, so surface the raw dnf output. raise PrecheckFailure(output) if os.path.isfile(update_precheck_file): diff --git a/python3/tests/test_pool_update_precheck.py b/python3/tests/test_pool_update_precheck.py new file mode 100644 index 00000000000..72b9aaa3bbe --- /dev/null +++ b/python3/tests/test_pool_update_precheck.py @@ -0,0 +1,181 @@ +#!/usr/bin/env python3 +""" +Unit tests for the dnf5 output parser in pool_update.precheck. +""" + +import unittest + +from python3.tests.import_helper import import_file_as_module, mocked_modules + +with mocked_modules("XenAPI", "xcp", "xcp.logger"): + precheck = import_file_as_module("python3/extensions/pool_update.precheck") + + +# --------------------------------------------------------------------------- +# Captured dnf5 outputs (real, not synthesised) +# --------------------------------------------------------------------------- + +CONFLICT = """\ +Updating and loading repositories: +Repositories loaded. +Failed to resolve the transaction: +Problem: installed package xs-host-1.0-1.noarch conflicts with xs-host provided by xs-conflict-1.0-1.noarch from testrepo + - problem with installed package + - conflicting requests + - nothing provides xs-platform >= 2.0 needed by xs-host-2.0-1.noarch from testrepo +You can try to add to command line: + --skip-broken to skip uninstallable packages +""" + +WRONG_VERSION = """\ +Updating and loading repositories: + testrepo 100% | 75.3 KiB/s | 3.4 KiB | 00m00s +Repositories loaded. +Failed to resolve the transaction: +Package "xs-host-1.0-1.noarch" is already installed. +Problem: cannot install the best candidate for the job + - nothing provides xs-platform >= 2.0 needed by xs-host-2.0-1.noarch from testrepo +You can try to add to command line: + --no-best to not limit the transaction to the best candidates + --skip-broken to skip uninstallable packages +""" + +PREREQ_MISSING = """\ +Updating and loading repositories: +Repositories loaded. +Failed to resolve the transaction: +Problem: conflicting requests + - nothing provides missing-lib needed by xs-prereq-1.0-1.noarch from testrepo +You can try to add to command line: + --skip-broken to skip uninstallable packages +""" + +NONEXISTENT = """\ +Updating and loading repositories: +Repositories loaded. +Failed to resolve the transaction: +No match for argument: no-such-pkg-xyz +You can try to add to command line: + --skip-unavailable to skip unavailable packages +""" + +# Librepo emits "GPG signature verification error" on first access to a repo +# whose key is not yet trusted, before dnf5 auto-imports the key. The same +# line therefore also appears in front of unrelated solver failures, so the +# precheck plugin must only raise GpgkeyNotImported when no "Failed to +# resolve" block follows. The two fixtures below are condensed XenRT +# captures (jobs 31109885 / 31109895) used to exercise that gate. + +LIBREPO_GPG_THEN_PREREQ = """\ +Updating and loading repositories: +>>> Librepo error: repomd.xml GPG signature verification error: Bad GPG signature +Importing PGP key 0xDEADBEEF: + UserID : "XenServer Updates " +Key imported. +Repositories loaded. +Failed to resolve the transaction: +Problem: conflicting requests + - nothing provides update-test-hotfix-basic-1 needed by test-hotfix-depend-1-1.0-1.noarch from updates +You can try to add to command line: + --skip-broken to skip uninstallable packages +""" + +LIBREPO_GPG_THEN_WRONG_VERSION = """\ +Updating and loading repositories: +>>> Librepo error: repomd.xml GPG signature verification error: Bad GPG signature +Importing PGP key 0xDEADBEEF: +Key imported. +Repositories loaded. +Failed to resolve the transaction: +Problem: cannot install the best candidate for the job + - nothing provides platform-version = 0.73.2 needed by control-test-hotfix-basic-4-1.0-1.noarch from updates +You can try to add to command line: + --no-best to not limit the transaction to the best candidates +""" + + +class TestDnf5OutputParser(unittest.TestCase): + """Parser tests against captured dnf5 5.2.12 output.""" + + def test_conflict(self): + with self.assertRaises(precheck.ConflictPresent) as ctx: + precheck.classify_dnf_failure(CONFLICT) + # First "conflicts with" token in the captured output. + self.assertIn("xs-host", ctx.exception.args[0]) + + def test_wrong_server_version(self): + with self.assertRaises(precheck.WrongServerVersion) as ctx: + precheck.classify_dnf_failure(WRONG_VERSION) + self.assertEqual( + ctx.exception.args, + ("xs-platform >= 2.0", "xs-host-2.0-1.noarch"), + ) + + def test_prerequisite_missing(self): + with self.assertRaises(precheck.PrerequisiteMissing) as ctx: + precheck.classify_dnf_failure(PREREQ_MISSING) + self.assertEqual(ctx.exception.args, ("missing-lib",)) + + def test_nonexistent_package_falls_through(self): + # No conflict / nothing-provides / requires pattern: classifier + # raises PrecheckFailure with the cleaned errmsg. + with self.assertRaises(precheck.PrecheckFailure) as ctx: + precheck.classify_dnf_failure(NONEXISTENT) + self.assertIn("No match for argument: no-such-pkg-xyz", + ctx.exception.args[0]) + + def test_download_failure_short_circuits(self): + output = "Some preamble\nFailed to download packages\nmore noise\n" + with self.assertRaises(precheck.InvalidUpdate): + precheck.classify_dnf_failure(output) + + def test_gpgkey_patterns_short_circuit(self): + # Each GPG marker alone (no "Failed to resolve" block) must classify + # as GpgkeyNotImported. + for pattern in precheck.ERROR_MESSAGE_GPGKEY_NOT_IMPORTED_PATTERNS: + with self.subTest(pattern=pattern): + with self.assertRaises(precheck.GpgkeyNotImported): + precheck.classify_dnf_failure("preamble\n" + pattern + "\n") + + def test_librepo_gpg_noise_does_not_mask_prereq(self): + """Regression for XenRT 31109885 / 31109895. + + Librepo prints a transient GPG verification error before dnf5 + auto-imports the repo key; the run then fails at the solver stage + for an unrelated reason. The plugin must surface the real reason + (PrerequisiteMissing here) rather than GpgkeyNotImported. + """ + with self.assertRaises(precheck.PrerequisiteMissing) as ctx: + precheck.classify_dnf_failure(LIBREPO_GPG_THEN_PREREQ) + self.assertEqual( + ctx.exception.args, ("update-test-hotfix-basic-1",), + ) + + def test_librepo_gpg_noise_does_not_mask_wrong_version(self): + """Same regression, wrong-version variant.""" + with self.assertRaises(precheck.WrongServerVersion) as ctx: + precheck.classify_dnf_failure(LIBREPO_GPG_THEN_WRONG_VERSION) + self.assertEqual( + ctx.exception.args, + ("platform-version = 0.73.2", + "control-test-hotfix-basic-4-1.0-1.noarch"), + ) + + def test_iso_mounted(self): + output = "noise\n" + precheck.PATCH_PRECHECK_FAILED_ISO_MOUNTED + "\n" + with self.assertRaises(precheck.IsoMounted): + precheck.classify_dnf_failure(output) + + def test_vm_running(self): + output = "noise\n" + precheck.PATCH_PRECHECK_FAILED_VM_RUNNING + "\n" + with self.assertRaises(precheck.VmRunning): + precheck.classify_dnf_failure(output) + + def test_unrecognised_output_returns_silently(self): + # When the output matches none of the patterns the classifier just + # returns; execute_precheck then raises PrecheckFailure(output). + self.assertIsNone(precheck.classify_dnf_failure("totally unrelated\n")) + + +if __name__ == "__main__": + unittest.main() From 03271bfe8f35ffb0194b361477d8ba51f41bb391 Mon Sep 17 00:00:00 2001 From: Changlei Li Date: Wed, 20 May 2026 15:42:58 +0800 Subject: [PATCH 19/20] CA-427419 Add timeout in fetch_server_cert Signed-off-by: Changlei Li --- ocaml/libs/stunnel/dune | 38 +++++++++++++++++------------------ ocaml/libs/stunnel/stunnel.ml | 8 +++++--- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/ocaml/libs/stunnel/dune b/ocaml/libs/stunnel/dune index 776f4a8c7ed..db700ed807f 100644 --- a/ocaml/libs/stunnel/dune +++ b/ocaml/libs/stunnel/dune @@ -1,21 +1,19 @@ (library - (name stunnel) - (public_name stunnel) - (wrapped false) - (libraries - astring - forkexec - safe-resources - threads.posix - unix - uuid - xapi-consts - xapi-inventory - xapi-log - xapi-stdext-pervasives - xapi-stdext-std - xapi-stdext-threads - xapi-stdext-unix - ) -) - + (name stunnel) + (public_name stunnel) + (wrapped false) + (libraries + astring + forkexec + mtime + safe-resources + threads.posix + unix + uuid + xapi-consts + xapi-inventory + xapi-log + xapi-stdext-pervasives + xapi-stdext-std + xapi-stdext-threads + xapi-stdext-unix)) diff --git a/ocaml/libs/stunnel/stunnel.ml b/ocaml/libs/stunnel/stunnel.ml index 70e0b40f37a..6c3c2748e01 100644 --- a/ocaml/libs/stunnel/stunnel.ml +++ b/ocaml/libs/stunnel/stunnel.ml @@ -639,6 +639,7 @@ end This is useful for TOFU (Trust-On-First-Use) scenarios. *) let fetch_server_cert ~remote_host ~remote_port = try + let timeout = Mtime.Span.(30 * s) in let openssl = !Constants.openssl_path in (* First get the certificate with s_client *) let s_client_args = @@ -650,13 +651,14 @@ let fetch_server_cert ~remote_host ~remote_port = ] in let cert_output, _ = - Forkhelpers.execute_command_get_output_send_stdin openssl s_client_args "" + Forkhelpers.execute_command_get_output_send_stdin ~timeout openssl + s_client_args "" in (* Then parse it with x509 to get PEM format *) let x509_args = ["x509"; "-outform"; "PEM"] in let pem_output, _ = - Forkhelpers.execute_command_get_output_send_stdin openssl x509_args - cert_output + Forkhelpers.execute_command_get_output_send_stdin ~timeout openssl + x509_args cert_output in if String.length pem_output > 0 From fc15e978fc79be625c68a72e1cd7c8f8a8f8c249 Mon Sep 17 00:00:00 2001 From: Changlei Li Date: Wed, 20 May 2026 15:54:19 +0800 Subject: [PATCH 20/20] Update new field lifecycle Signed-off-by: Changlei Li --- ocaml/idl/datamodel_lifecycle.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ocaml/idl/datamodel_lifecycle.ml b/ocaml/idl/datamodel_lifecycle.ml index 4e90114d45c..ac61b896a5f 100644 --- a/ocaml/idl/datamodel_lifecycle.ml +++ b/ocaml/idl/datamodel_lifecycle.ml @@ -146,7 +146,7 @@ let prototyped_of_field = function | "VM_metrics", "numa_optimised" -> Some "26.2.0" | "VM", "secureboot_certificates_state" -> - Some "26.13.0-next" + Some "26.14.0" | "VM", "groups" -> Some "24.19.1" | "VM", "pending_guidances_full" -> @@ -292,7 +292,7 @@ let prototyped_of_message = function | "VM", "sysprep" -> Some "25.24.0" | "VM", "update_secureboot_certificates_on_boot" -> - Some "26.13.0-next" + Some "26.14.0" | "VM", "get_secureboot_readiness" -> Some "24.17.0" | "VM", "set_uefi_mode" ->