Skip to content

Commit 1b268dc

Browse files
authored
CA-413587: Checking feature for old FreeBSD driver (#6599)
The FreeBSD driver used by NetScaler supports all power actions. However, older versions of the FreeBSD driver do not explicitly advertise these support. As a result, xapi does not attempt to signal these power actions. This Free BSD driver issue has been fixed by this commit: [9dd5105](https://cgit.freebsd.org/src/commit/sys/dev/xen/control/control.c?id=9dd5105f22a2276cf099f0e10e318294fcc4f6a7) However, there may be many FreeBSD VMs running there which still have this issue. To address this as a workaround, all power actions should be permitted for FreeBSD guests. Additionally, virtual machines with an explicit `data/cant_suspend_reason` set aren't allowed to suspend, which would crash Windows and other UEFI VMs.
2 parents fd5cc88 + 6732c89 commit 1b268dc

File tree

1 file changed

+41
-28
lines changed

1 file changed

+41
-28
lines changed

ocaml/xapi/xapi_vm_lifecycle.ml

Lines changed: 41 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -173,45 +173,58 @@ let has_definitely_booted_pv ~vmmr =
173173
)
174174

175175
(** Return an error iff vmr is an HVM guest and lacks a needed feature.
176+
177+
* Note: The FreeBSD driver used by NetScaler supports all power actions.
178+
* However, older versions of the FreeBSD driver do not explicitly advertise
179+
* these support. As a result, xapi does not attempt to signal these power
180+
* actions. To address this as a workaround, all power actions should be
181+
* permitted for FreeBSD guests.
182+
183+
* Additionally, VMs with an explicit `data/cant_suspend_reason` set aren't
184+
* allowed to suspend, which would crash Windows and other UEFI VMs.
185+
176186
* The "strict" param should be true for determining the allowed_operations list
177187
* (which is advisory only) and false (more permissive) when we are potentially about
178188
* to perform an operation. This makes a difference for ops that require the guest to
179189
* react helpfully. *)
180190
let check_op_for_feature ~__context ~vmr:_ ~vmmr ~vmgmr ~power_state ~op ~ref
181191
~strict =
182-
if
192+
let implicit_support =
183193
power_state <> `Running
184194
(* PV guests offer support implicitly *)
185195
|| has_definitely_booted_pv ~vmmr
186-
then
187-
None
188-
else
189-
let some_err e = Some (e, [Ref.string_of ref]) in
190-
let lack_feature feature = not (has_feature ~vmgmr ~feature) in
191-
match op with
192-
| `clean_shutdown
193-
when strict
194-
&& lack_feature "feature-shutdown"
195-
&& lack_feature "feature-poweroff" ->
196-
some_err Api_errors.vm_lacks_feature
197-
| `clean_reboot
198-
when strict
199-
&& lack_feature "feature-shutdown"
200-
&& lack_feature "feature-reboot" ->
201-
some_err Api_errors.vm_lacks_feature
202-
| `changing_VCPUs_live when lack_feature "feature-vcpu-hotplug" ->
196+
|| Xapi_pv_driver_version.(has_pv_drivers (of_guest_metrics vmgmr))
197+
(* Full PV drivers imply all features *)
198+
in
199+
let some_err e = Some (e, [Ref.string_of ref]) in
200+
let lack_feature feature = not (has_feature ~vmgmr ~feature) in
201+
match op with
202+
| `suspend | `checkpoint | `pool_migrate | `migrate_send -> (
203+
match get_feature ~vmgmr ~feature:"data-cant-suspend-reason" with
204+
| Some reason ->
205+
Some (Api_errors.vm_non_suspendable, [Ref.string_of ref; reason])
206+
| None
207+
when (not implicit_support) && strict && lack_feature "feature-suspend" ->
203208
some_err Api_errors.vm_lacks_feature
204-
| `suspend | `checkpoint | `pool_migrate | `migrate_send -> (
205-
match get_feature ~vmgmr ~feature:"data-cant-suspend-reason" with
206-
| Some reason ->
207-
Some (Api_errors.vm_non_suspendable, [Ref.string_of ref; reason])
208-
| None when strict && lack_feature "feature-suspend" ->
209-
some_err Api_errors.vm_lacks_feature
210-
| None ->
211-
None
212-
)
213-
| _ ->
209+
| None ->
214210
None
211+
)
212+
| _ when implicit_support ->
213+
None
214+
| `clean_shutdown
215+
when strict
216+
&& lack_feature "feature-shutdown"
217+
&& lack_feature "feature-poweroff" ->
218+
some_err Api_errors.vm_lacks_feature
219+
| `clean_reboot
220+
when strict
221+
&& lack_feature "feature-shutdown"
222+
&& lack_feature "feature-reboot" ->
223+
some_err Api_errors.vm_lacks_feature
224+
| `changing_VCPUs_live when lack_feature "feature-vcpu-hotplug" ->
225+
some_err Api_errors.vm_lacks_feature
226+
| _ ->
227+
None
215228

216229
(* N.B. In the pattern matching above, "pat1 | pat2 | pat3" counts as
217230
* one pattern, and the whole thing can be guarded by a "when" clause. *)

0 commit comments

Comments
 (0)