Skip to content

Commit c459480

Browse files
author
Stephen Cheng
committed
CA-426556: register VM.set_NVRAM_EFI_variables_v2 on the main XAPI dispatcher
The UEFI certificate-expiry feature added a new RPC, VM.set_NVRAM_EFI_variables_v2, on the varstored-guard wire interface. In the original design we only considered the indirect call path (varstored daemon -> varstored-guard -> XAPI), so we reused the existing set_NVRAM_EFI_variables declaration in the XAPI datamodel and only registered V2 on varstored-guard. What we overlooked: both the varstored daemon and the varstored CLI tools (varstore-set, varstore-rm, ...) emit set_NVRAM_EFI_variables_v2 via the shared xapidb-lib code. Whether the call goes through guard or straight to XAPI depends on the socket= argument the binary is started with -- not on whether it is the daemon or a CLI tool. In typical deployments the daemon is given a varstored-guard per-VM socket and the CLI tools are run without that argument, so they land on XAPI directly, where V2 is not declared and the dispatcher returns "Device Error". Fix: declare set_NVRAM_EFI_variables_v2 in datamodel_vm.ml so it is also registered on the main XAPI dispatcher. Semantics are identical to V1; the only schema difference is that [update] is mandatory in V2. Signed-off-by: Stephen Cheng <stephen.cheng@citrix.com>
1 parent 0fb6909 commit c459480

5 files changed

Lines changed: 70 additions & 18 deletions

File tree

ocaml/idl/datamodel_vm.ml

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2349,6 +2349,27 @@ let set_HVM_boot_policy =
23492349
effect when it is next started"
23502350
~allowed_roles:_R_VM_ADMIN ()
23512351

2352+
(* Shared definitions for set_NVRAM_EFI_variables / _v2: the [update]
2353+
parameter has identical semantics in both, only its presence on the wire
2354+
differs (optional in V1, required in V2). *)
2355+
let _update_status_enum =
2356+
Enum
2357+
( "update_status"
2358+
, [
2359+
("yes", "Set secureboot_certificates_state to ok")
2360+
; ("no", "Leave secureboot_certificates_state unchanged")
2361+
; ( "unspecified"
2362+
, "Check certificates and update secureboot_certificates_state \
2363+
accordingly"
2364+
)
2365+
]
2366+
)
2367+
2368+
let _update_param_doc =
2369+
"If 'yes', set secureboot_certificates_state to ok. If 'no', keep the \
2370+
current secureboot_certificates_state unchanged. If 'unspecified', run \
2371+
certificate check to determine the state."
2372+
23522373
let set_NVRAM_EFI_variables =
23532374
call ~flags:[`Session] ~name:"set_NVRAM_EFI_variables"
23542375
~lifecycle:[(Published, rel_naples, "")]
@@ -2369,30 +2390,40 @@ let set_NVRAM_EFI_variables =
23692390
; param_default= None
23702391
}
23712392
; {
2372-
param_type=
2373-
Enum
2374-
( "update_status"
2375-
, [
2376-
("yes", "Set secureboot_certificates_state to ok")
2377-
; ("no", "Leave secureboot_certificates_state unchanged")
2378-
; ( "unspecified"
2379-
, "Check certificates and update \
2380-
secureboot_certificates_state accordingly"
2381-
)
2382-
]
2383-
)
2393+
param_type= _update_status_enum
23842394
; param_name= "update"
23852395
; param_doc=
2386-
"If 'yes', set secureboot_certificates_state to ok. If 'no', keep \
2387-
the current secureboot_certificates_state unchanged. If omitted \
2388-
(defaults to 'unspecified'), run certificate check to determine \
2389-
the state."
2390-
; param_release= numbered_release "26.7.0-next"
2396+
_update_param_doc ^ " If omitted, defaults to 'unspecified'."
2397+
; param_release= numbered_release "26.10.0-next"
23912398
; param_default= Some (VEnum "unspecified")
23922399
}
23932400
]
23942401
~hide_from_docs:true ~allowed_roles:_R_LOCAL_ROOT_ONLY ()
23952402

2403+
(* Both the varstored daemon and the varstored CLI tools (varstore-set,
2404+
varstore-rm, ...) emit the same [set_NVRAM_EFI_variables_v2] call via
2405+
the shared xapidb-lib code. Whether that call reaches XAPI directly
2406+
or goes through varstored-guard depends on the [socket=] argument the
2407+
binary is started with: if it points at a varstored-guard per-VM
2408+
socket the call is relayed through guard (where V2 is already
2409+
registered); if it is absent (or points at XAPI's own socket) the
2410+
call lands on XAPI's main dispatcher. In typical deployments the
2411+
daemon takes the guard path and the CLI tools take the direct path,
2412+
but either binary can take either path. V2 must therefore be
2413+
registered here so the direct path works. The semantics are
2414+
identical to V1; the only schema difference is that [update] is a
2415+
mandatory parameter in V2. *)
2416+
let set_NVRAM_EFI_variables_v2 =
2417+
call ~flags:[`Session] ~name:"set_NVRAM_EFI_variables_v2"
2418+
~lifecycle:[(Published, "26.10.0-next", "")]
2419+
~params:
2420+
[
2421+
(Ref _vm, "self", "The VM")
2422+
; (String, "value", "The EFI-variables value")
2423+
; (_update_status_enum, "update", _update_param_doc)
2424+
]
2425+
~hide_from_docs:true ~allowed_roles:_R_LOCAL_ROOT_ONLY ()
2426+
23962427
let restart_device_models =
23972428
call ~name:"restart_device_models" ~lifecycle:[]
23982429
~params:[(Ref _vm, "self", "The VM")]
@@ -2651,6 +2682,7 @@ let t =
26512682
; set_domain_type
26522683
; set_HVM_boot_policy
26532684
; set_NVRAM_EFI_variables
2685+
; set_NVRAM_EFI_variables_v2
26542686
; restart_device_models
26552687
; set_uefi_mode
26562688
; get_secureboot_readiness

ocaml/xapi-guard/lib/server_interface.ml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,8 @@ let make_server_varstored _persist ~cache path vm_uuid =
198198
let update =
199199
match update with "yes" -> `yes | "no" -> `no | _ -> `unspecified
200200
in
201-
with_xapi ~cache @@ VM.set_NVRAM_EFI_variables ~self ~value:nvram ~update
201+
with_xapi ~cache
202+
@@ VM.set_NVRAM_EFI_variables_v2 ~self ~value:nvram ~update
202203
)
203204
|> ret
204205
in

ocaml/xapi/message_forwarding.ml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3135,6 +3135,13 @@ functor
31353135
info "VM.set_NVRAM_EFI_variables: self = '%s'" (vm_uuid ~__context self) ;
31363136
Local.VM.set_NVRAM_EFI_variables ~__context ~self ~value ~update
31373137

3138+
let set_NVRAM_EFI_variables_v2 ~__context ~self ~value ~update =
3139+
(* called by varstored CLI tools (which connect to XAPI directly,
3140+
bypassing varstored-guard); same semantics as V1 *)
3141+
info "VM.set_NVRAM_EFI_variables_v2: self = '%s'"
3142+
(vm_uuid ~__context self) ;
3143+
Local.VM.set_NVRAM_EFI_variables_v2 ~__context ~self ~value ~update
3144+
31383145
let restart_device_models ~__context ~self =
31393146
info "VM.restart_device_models: self = '%s'" (vm_uuid ~__context self) ;
31403147
Local.VM.restart_device_models ~__context ~self

ocaml/xapi/xapi_vm.ml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1674,6 +1674,11 @@ let set_NVRAM_EFI_variables ~__context ~self ~value ~update =
16741674
in
16751675
Db.VM.set_secureboot_certificates_state ~__context ~self ~value:new_state
16761676

1677+
(* V2 has the same semantics as V1; it exists only to provide an RPC method
1678+
whose [update] parameter is mandatory on the wire (V1's [update] is
1679+
optional with default [`unspecified]). See datamodel_vm.ml. *)
1680+
let set_NVRAM_EFI_variables_v2 = set_NVRAM_EFI_variables
1681+
16771682
let restart_device_models ~__context ~self =
16781683
let power_state = Db.VM.get_power_state ~__context ~self in
16791684
if power_state <> `Running then

ocaml/xapi/xapi_vm.mli

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,13 @@ val set_NVRAM_EFI_variables :
431431
-> update:[`yes | `no | `unspecified]
432432
-> unit
433433

434+
val set_NVRAM_EFI_variables_v2 :
435+
__context:Context.t
436+
-> self:API.ref_VM
437+
-> value:string
438+
-> update:[`yes | `no | `unspecified]
439+
-> unit
440+
434441
val restart_device_models : __context:Context.t -> self:API.ref_VM -> unit
435442

436443
val set_uefi_mode :

0 commit comments

Comments
 (0)