Add VM.sysprep API call #6547

lindig · 2025-06-20T12:19:31Z

A new API call that works in collaboration with a guest agent on a Windows VM to pass a unattend.xml to Windows sysprep.

Receives the XML content as a parameter. The XE interface supports defining this from a file but an API client has to pass the content as a parameter.
The API call creates a local ISO SR; creates an ISO with unattended.xml and inserts the ISO into the VMs CD-ROM, then notifies the VM via xenstore.
Outside of scaffolding for the API, everything in vm_sysprep.ml.
Exceptions raised are handled in Xapi_vm.sysprep()

last-genius · 2025-06-20T12:45:07Z

Did you mean to assign Rob and Edwin as reviewers, not assignees?

last-genius · 2025-06-20T12:45:55Z

I can guess the big-picture intentions behind this, but as someone not terribly familiar with Windows setup, it could be worth spelling out why this is wanted and what this simplifies and improves

lindig · 2025-06-20T12:53:01Z

It's coming from a customer requirement: supporting a way to customise a Windows VM using Windows tools. The challenge is how to get a file to the VM such that a tool in the VM can read it. My preferred method would have been providing this via the network but this does not work when the VM has no network yet set up. So we end up with a somewhat convoluted approach to create an ISO that we provide via CD ROM to the VM. This is currently disabled via a flag.

edwintorok · 2025-06-23T10:50:50Z

ocaml/xapi-cli-server/cli_frontend.ml

+    , {
+        reqd= ["filename"]
+      ; optn= []
+      ; help= "Pass and execure sysprep configuration file"


edwintorok · 2025-06-23T10:53:32Z

ocaml/xapi/vm_sysprep.ml

+
+let finally = Xapi_stdext_pervasives.Pervasiveext.finally
+
+let genisoimage = "/usr/bin/genisoimage"


I think the usual way to refer to these is by declaring it in Xapi_globs.Resources, perhaps as an optional runtime dependency (if the file is missing we won't be able to sysprep, but we should still be able to perform other ops).

edwintorok · 2025-06-23T10:54:27Z

ocaml/xapi/vm_sysprep.ml

+let genisoimage = "/usr/bin/genisoimage"
+
+(** This will be shown to the user to explain a failure *)
+exception Failure of string


this can be confusing, because failwith also raises a builtin Failure exception, can we use a different name that doesn't clash with stdlib builtins?

I had that thought; can rename it.

edwintorok · 2025-06-23T11:03:30Z

ocaml/xapi/vm_sysprep.ml

+(** create a name with a random infix. We need random names for
+    temporay directories to avoid collition *)
+let temp_name prefix suffix =
+  let rnd = Random.State.bits (Lazy.force prng) land 0xFFFFFF in


Lazy doesn't play nicely with multithreaded code (in OCaml 4.14.2 it can actually segfault, although that is fixed on the 4.14 branch, and we should also really apply that patch to our internal compiler), also sharing a Random.State between threads is probably worse than using the default random state (which in OCaml 5 has some code to make it domain-local).

To avoid issues with lazy can you initialize this fully on startup like this:
let prng = Random.State.make_self_init ()

Then even if we have race conditions on this state, it'll only affect this prng, and even if we end up with same random number in 2 threads that just means we'll fail to generate the temp name and retry.

We have multiple PRNGs in XAPI already though, see e.g. xapi_session.ml where we use Mirage_crypto_rng.generate, although in this case we don't necessarily need cryptographic quality random numbers.

Add a new API call for VM sysprep and the corresponding XE implementation. This is mostly scaffolding. Signed-off-by: Christian Lindig <[email protected]>

We want to create a temporary directory that will be used to hold files for creating an ISO. There is no existing function that creates all necessary directories in a predicatble way. Signed-off-by: Christian Lindig <[email protected]>

Implement creating an ISO from a temporary directory Signed-off-by: Christian Lindig <[email protected]>

Signed-off-by: Christian Lindig <[email protected]>

Create a local SR unless it exists. Signed-off-by: Christian Lindig <[email protected]>

We need to locate the CD drive of the VM. Signed-off-by: Christian Lindig <[email protected]>

The VM is notified to perform a sysprep by writing to XenStore. The VM picks this up via its guest agent. Signed-off-by: Christian Lindig <[email protected]>

Signed-off-by: Christian Lindig <[email protected]>

Introduce on_startup(), called on xapi startup, to clean up the local SR to avoid accumulating ISO files. Signed-off-by: Christian Lindig <[email protected]>

For simplicity, add vm-sysprep-enabled = true/false to xapi.conf rather than using a full V6D feature flag. Signed-off-by: Christian Lindig <[email protected]>

Signed-off-by: Christian Lindig <[email protected]>

Add the paths to genisoimage so xapi_globs and list it as non-essential binary. Signed-off-by: Christian Lindig <[email protected]>

robhoes · 2025-06-26T13:45:58Z

ocaml/xapi/message_forwarding.ml

+          ~policy (fun () ->
+            forward_vm_op ~local_fn ~__context ~vm:self ~remote_fn
+        ) ;
+        Xapi_vm_lifecycle.update_allowed_operations ~__context ~self


with_vm_operation already does this.

robhoes · 2025-06-26T13:50:51Z

ocaml/idl/datamodel_lifecycle.ml

@@ -239,6 +239,8 @@ let prototyped_of_message = function
      Some "25.2.0"
  | "host", "set_numa_affinity_policy" ->
      Some "24.0.0"
+  | "VM", "sysprep" ->
+      Some "25.22.0"


This needs a refresh

robhoes · 2025-06-26T13:52:29Z

ocaml/xapi-consts/api_errors.ml

+
+(* VM.sysprep *)
+
+(* Using a single error during development, might want to expand this


Remove this now that the code is stable

robhoes · 2025-06-26T14:01:49Z

ocaml/xapi/vm_sysprep.ml

+      Db.SR.get_VDIs ~__context ~self:sr
+      |> List.iter @@ fun self ->
+         match Db.VDI.get_record ~__context ~self with
+         | API.{vDI_VBDs= []; vDI_location= _location; _} ->


What is vDI_location= _location doing? I don't see _location defined anywhere?

robhoes · 2025-06-26T14:02:57Z

ocaml/xapi/vm_sysprep.ml

+    )
+
+(** create a name with a random infix. We need random names for
+    temporay directories to avoid collition *)


"temporay"... "collition"... random mutations? :)

robhoes · 2025-06-26T14:33:00Z

ocaml/xapi/vm_sysprep.ml

+        let device_config = [("location", SR.dir); ("legacy_mode", "true")] in
+        Xapi_sr.create ~__context ~host ~name_label:label ~device_config
+          ~content_type:"iso" ~_type:"iso" ~name_description:"Sysprep ISOs"
+          ~shared:false ~sm_config:[] ~physical_size:(mib 512)


Why 512MiB if the XML file is limited to just 32KiB?

The SR holds multiple ISOs, each is about 300k; we remove them after each API call. I am not sure the 512MB is enforced by SM.

robhoes · 2025-06-26T14:37:20Z

ocaml/xapi/vm_sysprep.ml

+      match xs.Xs.read (control // "feature-sysprep") with
+      | "1" ->
+          debug "%s: VM %s supports sysprep" __FUNCTION__ vm_uuid
+      | _ ->
+          fail "VM %s does not support sysprep" vm_uuid
+      | exception _ ->
+          fail "VM %s does not support sysprep" vm_uuid
+  ) ;


This could eventually be replaced by allowed_operations checking (see check_op_for_feature in xapi_vm_lifecycle.ml).

I think this test is good because it checks against the driver directly.

It is, and so does the xapi_vm_lifecycle.ml check, which is based on xapi_guest_agent.ml putting all those control keys (including all feature- keys) into VM_guest_metrics.other (which is a bad name).

E.g. on a VM on my machine:

robhoes · 2025-06-26T14:39:56Z

ocaml/xapi/vm_sysprep.ml

+  let vdi = find_vdi ~__context ~label in
+  debug "%s: inserting Sysprep VDI for VM %s" __FUNCTION__ vm_uuid ;
+  Xapi_vbd.insert ~__context ~vdi ~vbd ;
+  Thread.delay 5.0 ;


It would be nice if the guest agent can watch for the device to appear rather than adding an arbitrary delay here...

I understand the desire; but we also need to be sure it has seen the ISO as otherwise we can't tell it to read from it. So this would require a back channel via xenstore that we watch

robhoes · 2025-06-26T14:41:16Z

ocaml/xapi/vm_sysprep.ml

+      Xapi_vbd.eject ~__context ~vbd ;
+      Sys.remove iso ;


Two duplicated lines could be factored out.

robhoes · 2025-06-26T14:49:36Z

ocaml/xapi/xapi.ml

+          ; ( "Remove local ISO SR"
+            , [Startup.OnThread]
+            , fun () -> Vm_sysprep.on_startup ~__context
+            )


This should be moved after the block below, where the master connection is established if running on a pool member, and dbsync has run.

lindig assigned robhoes and edwintorok Jun 20, 2025

lindig requested review from robhoes and edwintorok June 20, 2025 13:41

lindig unassigned robhoes and edwintorok Jun 20, 2025

lindig force-pushed the private/christianlin/vm-sysprep branch from 5c8c133 to 7ecd772 Compare June 23, 2025 10:06

edwintorok reviewed Jun 23, 2025

View reviewed changes

lindig force-pushed the private/christianlin/vm-sysprep branch 3 times, most recently from 2d29a74 to ffd8aed Compare June 24, 2025 12:55

lindig marked this pull request as ready for review June 24, 2025 13:12

lindig force-pushed the private/christianlin/vm-sysprep branch from ffd8aed to 782a6d6 Compare June 25, 2025 15:36

Christian Lindig added 13 commits June 25, 2025 16:37

CP-308455 VM.sysprep start with skeleton

5ba3caa

Add a new API call for VM sysprep and the corresponding XE implementation. This is mostly scaffolding. Signed-off-by: Christian Lindig <[email protected]>

CP-308455 VM.sysprep implement mkdtemp

50e0452

We want to create a temporary directory that will be used to hold files for creating an ISO. There is no existing function that creates all necessary directories in a predicatble way. Signed-off-by: Christian Lindig <[email protected]>

CP-308455 VM.sysprep make iso

98c3379

Implement creating an ISO from a temporary directory Signed-off-by: Christian Lindig <[email protected]>

CP-308455 VM.sysprep Add logging

9beea76

Signed-off-by: Christian Lindig <[email protected]>

CP-308455 VM.sysprep Add SR creation

1c15845

Create a local SR unless it exists. Signed-off-by: Christian Lindig <[email protected]>

CP-308455 VM.sysprep Find VBD for VM's CDR

51dd4d4

We need to locate the CD drive of the VM. Signed-off-by: Christian Lindig <[email protected]>

CP-308455 VM.sysprep Implement trigger

f881cb4

The VM is notified to perform a sysprep by writing to XenStore. The VM picks this up via its guest agent. Signed-off-by: Christian Lindig <[email protected]>

CP-308455 VM.sysprep log sysprep status

72ce7c3

Signed-off-by: Christian Lindig <[email protected]>

CP-308455 VM.sysprep watch execution, clean up

2f52406

Introduce on_startup(), called on xapi startup, to clean up the local SR to avoid accumulating ISO files. Signed-off-by: Christian Lindig <[email protected]>

CP-308455 VM.sysprep add feature flag

9dbb63c

For simplicity, add vm-sysprep-enabled = true/false to xapi.conf rather than using a full V6D feature flag. Signed-off-by: Christian Lindig <[email protected]>

CP-308455 VM.syspreo add some comments

6cd6816

Signed-off-by: Christian Lindig <[email protected]>

CP-308455 VM.sysprep unify error handling

7871fce

Signed-off-by: Christian Lindig <[email protected]>

CP-308455 VM.sysprep list genisoimage as resource

6e37536

Add the paths to genisoimage so xapi_globs and list it as non-essential binary. Signed-off-by: Christian Lindig <[email protected]>

lindig force-pushed the private/christianlin/vm-sysprep branch from 782a6d6 to 6e37536 Compare June 26, 2025 09:40

robhoes reviewed Jun 26, 2025

View reviewed changes


		let finally = Xapi_stdext_pervasives.Pervasiveext.finally

		let genisoimage = "/usr/bin/genisoimage"


		(* VM.sysprep *)

		(* Using a single error during development, might want to expand this

Add VM.sysprep API call #6547

Are you sure you want to change the base?

Add VM.sysprep API call #6547

Conversation

lindig commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

last-genius commented Jun 20, 2025

Uh oh!

last-genius commented Jun 20, 2025

Uh oh!

lindig commented Jun 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lindig commented Jun 20, 2025 •

edited

Loading