From 7b5352f111a50b680e0de77aeba095ebc883be66 Mon Sep 17 00:00:00 2001 From: Pau Ruiz Safont Date: Tue, 10 Feb 2026 10:13:41 +0100 Subject: [PATCH 1/3] xapi-cli-server: stop using SR records to filter ISO-backed PBDs on cross-pool migration SR records have variant fields that add new variants when storage features are added, as is the case with current_operations. This means that fetching records from remote pools cause deserialization exceptions when their records contain newer, unknown variants. xe uses SR records to minimize the amount of remote calls needed for selecting the SR with the highest amount of free space during cross-pool migrations. Change xe to fetch references of only non-ISO SRs, and filter PBD records with them. This means that now PBD records are fetched, but these don't contain any variants, and have been unchanged since early 2008. This has the side effect of reducing the amount of calls from 2 per host-attached PBDS to 1 per non-iso, host-attached PBD with the additional cost of a single call to fetch the non-iso SRs Signed-off-by: Pau Ruiz Safont (cherry picked from commit 5a61aaefe97490d6473edf3c6e916b44b7f8d9bc) --- ocaml/xapi-cli-server/cli_operations.ml | 80 ++++++++++++++----------- 1 file changed, 45 insertions(+), 35 deletions(-) diff --git a/ocaml/xapi-cli-server/cli_operations.ml b/ocaml/xapi-cli-server/cli_operations.ml index a19c52d94eb..9341d844867 100644 --- a/ocaml/xapi-cli-server/cli_operations.ml +++ b/ocaml/xapi-cli-server/cli_operations.ml @@ -4839,51 +4839,61 @@ let vm_migrate printer rpc session_id params = (read_map_params "vgpu" params) in let preferred_sr = - (* The preferred SR is determined to be as the SR that the destine host has a PDB attached to it, - and among the choices of that the shared is preferred first(as it is recommended to have shared storage - in pool to host VMs), and then the one with the maximum available space *) + (* The preferred SR is determined to be as the SR that the + destination host has a PDB attached to it, and among the choices + of that the shared is preferred first (as it is recommended to + have shared storage in pool to host VMs), and then the one with + the maximum available space *) try - let expr = - Printf.sprintf - {|(field "host"="%s") and (field "currently_attached"="true")|} - (Ref.string_of host) + let host_attached_pbds = + let expr = + Printf.sprintf + {|(field "host"="%s") and (field "currently_attached"="true")|} + (Ref.string_of host) + in + remote Client.PBD.get_all_records_where ~expr + in + let non_iso_srs = + let expr = {|not (field "sr_content_type"="iso")|} in + remote Client.SR.get_all_where ~expr in let srs = - remote Client.PBD.get_all_where ~expr - |> List.map (fun pbd -> - let sr = remote Client.PBD.get_SR ~self:pbd in - (sr, remote Client.SR.get_record ~self:sr) + host_attached_pbds + |> List.filter_map (fun (_, pbd_rec) -> + let sr = pbd_rec.API.pBD_SR in + if List.mem sr non_iso_srs then + Some (sr, remote Client.SR.get_record ~self:sr) + else + None ) in - (* In the following loop, the current SR:sr' will be compared with previous checked ones, - first if it is an ISO type, then pass this one for selection, then the only shared one from this and - previous one will be valued, and if not that case (both shared or none shared), choose the one with - more space available *) + (* In the following loop, the current SR:sr' will be compared with + previous checked ones, first if it is an ISO type, then pass + this one for selection, then the only shared one from this and + previous one will be valued, and if not that case (both shared + or none shared), choose the one with more space available *) let sr, _ = List.fold_left (fun (sr, free_space) ((_, sr_rec') as sr') -> - if sr_rec'.API.sR_content_type = "iso" then - (sr, free_space) - else - let free_space' = - Int64.sub sr_rec'.API.sR_physical_size - sr_rec'.API.sR_physical_utilisation - in - match sr with - | None -> + let free_space' = + Int64.sub sr_rec'.API.sR_physical_size + sr_rec'.API.sR_physical_utilisation + in + match sr with + | None -> + (Some sr', free_space') + | Some ((_, sr_rec) as sr) -> ( + match (sr_rec.API.sR_shared, sr_rec'.API.sR_shared) with + | true, false -> + (Some sr, free_space) + | false, true -> (Some sr', free_space') - | Some ((_, sr_rec) as sr) -> ( - match (sr_rec.API.sR_shared, sr_rec'.API.sR_shared) with - | true, false -> - (Some sr, free_space) - | false, true -> + | _ -> + if free_space' > free_space then (Some sr', free_space') - | _ -> - if free_space' > free_space then - (Some sr', free_space') - else - (Some sr, free_space) - ) + else + (Some sr, free_space) + ) ) (None, Int64.zero) srs in From 96c570c7c5ffd6317b7dde3e403ee50091ede22f Mon Sep 17 00:00:00 2001 From: Pau Ruiz Safont Date: Tue, 10 Feb 2026 10:54:19 +0100 Subject: [PATCH 2/3] xapi-cli-server: stop using SR records for cross pool migrations SR records have variant fields that add new variants when storage features are added, as is the case with current_operations. This means that fetching records from remote pool cause deserialization exceptions when their records contain newer, unknown variants when using OCaml-based clients. Use expression-based queries to separate shared from local SRs without records, and add 2 queries per SR to gather the free space. Now the amount of calls have changed from 1 per host-attached PBD to 2, but the amount of calls can be limited to shared SRs, if there are any. Signed-off-by: Pau Ruiz Safont (cherry picked from commit c1de0d7c36c32e02efcec28f1a42226ef7c18bf0) --- ocaml/xapi-cli-server/cli_operations.ml | 65 ++++++++++++------------- 1 file changed, 31 insertions(+), 34 deletions(-) diff --git a/ocaml/xapi-cli-server/cli_operations.ml b/ocaml/xapi-cli-server/cli_operations.ml index 9341d844867..671bfed2f69 100644 --- a/ocaml/xapi-cli-server/cli_operations.ml +++ b/ocaml/xapi-cli-server/cli_operations.ml @@ -4853,51 +4853,48 @@ let vm_migrate printer rpc session_id params = in remote Client.PBD.get_all_records_where ~expr in - let non_iso_srs = - let expr = {|not (field "sr_content_type"="iso")|} in + let shared_non_iso_srs () = + let expr = + {|(not (field "content_type"="iso")) and (field "shared"="true")|} + in + remote Client.SR.get_all_where ~expr + in + let local_non_iso_srs () = + let expr = + {|(not (field "content_type"="iso")) and (field "shared"="false")|} + in remote Client.SR.get_all_where ~expr in - let srs = + let get_free_space_of non_iso_srs = host_attached_pbds |> List.filter_map (fun (_, pbd_rec) -> let sr = pbd_rec.API.pBD_SR in if List.mem sr non_iso_srs then - Some (sr, remote Client.SR.get_record ~self:sr) + let size = remote Client.SR.get_physical_size ~self:sr in + let used = + remote Client.SR.get_physical_utilisation ~self:sr + in + Some (sr, Int64.sub size used) else None ) in - (* In the following loop, the current SR:sr' will be compared with - previous checked ones, first if it is an ISO type, then pass - this one for selection, then the only shared one from this and - previous one will be valued, and if not that case (both shared - or none shared), choose the one with more space available *) - let sr, _ = - List.fold_left - (fun (sr, free_space) ((_, sr_rec') as sr') -> - let free_space' = - Int64.sub sr_rec'.API.sR_physical_size - sr_rec'.API.sR_physical_utilisation - in - match sr with - | None -> - (Some sr', free_space') - | Some ((_, sr_rec) as sr) -> ( - match (sr_rec.API.sR_shared, sr_rec'.API.sR_shared) with - | true, false -> - (Some sr, free_space) - | false, true -> - (Some sr', free_space') - | _ -> - if free_space' > free_space then - (Some sr', free_space') - else - (Some sr, free_space) - ) - ) - (None, Int64.zero) srs + let find_most_free_space srs = + match + List.fast_sort + (fun (_, a) (_, b) -> Int64.compare b a) + (get_free_space_of srs) + with + | (sr, _) :: _ -> + Some sr + | [] -> + None in - match sr with Some (sr_ref, _) -> Some sr_ref | _ -> None + match find_most_free_space (shared_non_iso_srs ()) with + | Some sr -> + Some sr + | None -> + find_most_free_space (local_non_iso_srs ()) with _ -> None in let vdi_map = From dfb2d4e72137e73172797c34649785cdbcfee375 Mon Sep 17 00:00:00 2001 From: Pau Ruiz Safont Date: Thu, 12 Mar 2026 14:07:53 +0000 Subject: [PATCH 3/3] xapi-cli-server: print reason for failing to select preferred SR Otherwise there's no feedback on the reason why this failed Signed-off-by: Pau Ruiz Safont (cherry picked from commit a93a9846510ff54f0f44b04c134a0994e176d6b5) --- ocaml/xapi-cli-server/cli_operations.ml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ocaml/xapi-cli-server/cli_operations.ml b/ocaml/xapi-cli-server/cli_operations.ml index 671bfed2f69..0c69df1eda0 100644 --- a/ocaml/xapi-cli-server/cli_operations.ml +++ b/ocaml/xapi-cli-server/cli_operations.ml @@ -4895,7 +4895,17 @@ let vm_migrate printer rpc session_id params = Some sr | None -> find_most_free_space (local_non_iso_srs ()) - with _ -> None + with exn -> + printer + (Cli_printer.PMsg + (Printf.sprintf + "Couldn't compute preferred SR, continuing with the \ + user-provided VDI mapping. The reason is: %s" + (Printexc.to_string exn) + ) + ) ; + + None in let vdi_map = match preferred_sr with