From 82fb7f851790d03661e76671bb062f953dd6abb3 Mon Sep 17 00:00:00 2001 From: Rob Hoes Date: Thu, 25 May 2023 13:46:39 +0000 Subject: [PATCH 1/2] CP-42064: Move NbdClient module from Xapi_vbd to Attach_helpers Here it can be used by functions in the Storage_migrate module. Signed-off-by: Rob Hoes --- ocaml/xapi/attach_helpers.ml | 41 ++++++++++++++++++++++++++++++++ ocaml/xapi/xapi_vbd.ml | 45 ++++-------------------------------- 2 files changed, 45 insertions(+), 41 deletions(-) diff --git a/ocaml/xapi/attach_helpers.ml b/ocaml/xapi/attach_helpers.ml index 3797009b173..ed3e32bada5 100644 --- a/ocaml/xapi/attach_helpers.ml +++ b/ocaml/xapi/attach_helpers.ml @@ -132,3 +132,44 @@ let with_vbds rpc session_id __context vm vdis mode f = !vbds ) ) + +module NbdClient = struct + let print_fork_error f = + try f () + with Forkhelpers.Spawn_internal_error (stderr, stdout, status) as e -> ( + match status with + | Unix.WEXITED n -> + error "Forkhelpers.Spawn_internal_error(%s, %s, WEXITED %d)" stderr + stdout n ; + raise e + | Unix.WSIGNALED n -> + error "Forkhelpers.Spawn_internal_error(%s, %s, WSIGNALED %d)" stderr + stdout n ; + raise e + | Unix.WSTOPPED n -> + error "Forkhelpers.Spawn_internal_error(%s, %s, WSTOPPED %d)" stderr + stdout n ; + raise e + ) + + let run_command cmd args = + debug "running %s %s" cmd (String.concat " " args) ; + let stdout, _ = + print_fork_error (fun () -> + Forkhelpers.execute_command_get_output cmd args + ) + in + stdout + + let start_nbd_client ~unix_socket_path ~export_name = + run_command + !Xapi_globs.nbd_client_manager_script + ["connect"; "--path"; unix_socket_path; "--exportname"; export_name] + |> String.trim + + let stop_nbd_client ~nbd_device = + run_command + !Xapi_globs.nbd_client_manager_script + ["disconnect"; "--device"; nbd_device] + |> ignore +end diff --git a/ocaml/xapi/xapi_vbd.ml b/ocaml/xapi/xapi_vbd.ml index af05176d10c..1da2516d809 100644 --- a/ocaml/xapi/xapi_vbd.ml +++ b/ocaml/xapi/xapi_vbd.ml @@ -26,45 +26,6 @@ let update_allowed_operations ~__context ~self : unit = let assert_attachable ~__context ~self : unit = assert_attachable ~__context ~self -let print_fork_error f = - try f () - with Forkhelpers.Spawn_internal_error (stderr, stdout, status) as e -> ( - match status with - | Unix.WEXITED n -> - error "Forkhelpers.Spawn_internal_error(%s, %s, WEXITED %d)" stderr - stdout n ; - raise e - | Unix.WSIGNALED n -> - error "Forkhelpers.Spawn_internal_error(%s, %s, WSIGNALED %d)" stderr - stdout n ; - raise e - | Unix.WSTOPPED n -> - error "Forkhelpers.Spawn_internal_error(%s, %s, WSTOPPED %d)" stderr - stdout n ; - raise e - ) - -let run_command cmd args = - debug "running %s %s" cmd (String.concat " " args) ; - let stdout, _ = - print_fork_error (fun () -> Forkhelpers.execute_command_get_output cmd args) - in - stdout - -module NbdClient = struct - let start_nbd_client ~unix_socket_path ~export_name = - run_command - !Xapi_globs.nbd_client_manager_script - ["connect"; "--path"; unix_socket_path; "--exportname"; export_name] - |> String.trim - - let stop_nbd_client ~nbd_device = - run_command - !Xapi_globs.nbd_client_manager_script - ["disconnect"; "--device"; nbd_device] - |> ignore -end - let set_mode ~__context ~self ~value = let vm = Db.VBD.get_VM ~__context ~self in Xapi_vm_lifecycle.assert_initial_power_state_is ~__context ~self:vm @@ -96,7 +57,8 @@ let plug ~__context ~self = let unix_socket_path, export_name = Storage_interface.parse_nbd_uri nbd in - NbdClient.start_nbd_client ~unix_socket_path ~export_name + Attach_helpers.NbdClient.start_nbd_client ~unix_socket_path + ~export_name | [], [], [] -> raise (Storage_interface.Storage_error @@ -145,7 +107,8 @@ let unplug ~__context ~self = let device = Db.VBD.get_device ~__context ~self in let nbd_device_prefix = "nbd" in let is_nbd = Astring.String.is_prefix ~affix:nbd_device_prefix device in - if is_nbd then NbdClient.stop_nbd_client ~nbd_device:("/dev/" ^ device) ; + if is_nbd then + Attach_helpers.NbdClient.stop_nbd_client ~nbd_device:("/dev/" ^ device) ; Storage_access.deactivate_and_detach ~__context ~vbd:self ~domid ; Db.VBD.set_currently_attached ~__context ~self ~value:false ) else From 5d02ca1ca389f1ecd5a86bc32abf9e61c71e3127 Mon Sep 17 00:00:00 2001 From: Rob Hoes Date: Wed, 24 May 2023 16:36:04 +0000 Subject: [PATCH 2/2] CP-42064: Fix storage migration for NBD-backed storage The storage-migration code did not handle the case where a VDI is backed by NBD rather than a blktap2 device. So far, NBD-backed VDIs have only been used for GFS2 SRs, which do not support storage migration yet. This patch adds the missing pieces. Signed-off-by: Rob Hoes --- ocaml/tapctl/tapctl.ml | 2 ++ ocaml/tapctl/tapctl.mli | 2 ++ ocaml/xapi/storage_migrate.ml | 56 ++++++++++++++++++++++++++--------- 3 files changed, 46 insertions(+), 14 deletions(-) diff --git a/ocaml/tapctl/tapctl.ml b/ocaml/tapctl/tapctl.ml index 4052de87386..b2148fcf55b 100644 --- a/ocaml/tapctl/tapctl.ml +++ b/ocaml/tapctl/tapctl.ml @@ -73,6 +73,8 @@ type driver = Vhd | Aio let string_of_driver = function Vhd -> "vhd" | Aio -> "aio" +let tapdev_of ~pid ~minor = {minor; tapdisk_pid= pid} + (* DUMMY MODE FUNCTIONS *) let get_minor tapdev = tapdev.minor diff --git a/ocaml/tapctl/tapctl.mli b/ocaml/tapctl/tapctl.mli index 4508ff66f56..6d32728c7b2 100644 --- a/ocaml/tapctl/tapctl.mli +++ b/ocaml/tapctl/tapctl.mli @@ -33,6 +33,8 @@ val tapdev_of_rpc : Rpc.t -> tapdev val rpc_of_tapdev : tapdev -> Rpc.t +val tapdev_of : pid:int -> minor:int -> tapdev + val get_minor : tapdev -> int val get_tapdisk_pid : tapdev -> int diff --git a/ocaml/xapi/storage_migrate.ml b/ocaml/xapi/storage_migrate.ml index 594635a50b0..b87b06ab609 100644 --- a/ocaml/xapi/storage_migrate.ml +++ b/ocaml/xapi/storage_migrate.ml @@ -323,12 +323,12 @@ module Local = StorageAPI (Idl.Exn.GenClient (struct end)) let tapdisk_of_attach_info (backend : Storage_interface.backend) = - let xendisks, _, _, _ = + let _, blockdevices, _, nbds = Storage_interface.implementations_of_backend backend in - match xendisks with - | xendisk :: _ -> ( - let path = xendisk.Storage_interface.params in + match (blockdevices, nbds) with + | blockdevice :: _, _ -> ( + let path = blockdevice.Storage_interface.path in try match Tapctl.of_device (Tapctl.create ()) path with | tapdev, _, _ -> @@ -344,8 +344,19 @@ let tapdisk_of_attach_info (backend : Storage_interface.backend) = debug "Device %s has an unknown driver" path ; None ) - | [] -> - debug "No XenDisk implementation in backend: %s" + | _, nbd :: _ -> ( + try + let path, _ = Storage_interface.parse_nbd_uri nbd in + let filename = Unix.realpath path |> Filename.basename in + Scanf.sscanf filename "nbd%d.%d" (fun pid minor -> + Some (Tapctl.tapdev_of ~pid ~minor) + ) + with _ -> + debug "No tapdisk found for NBD backend: %s" nbd.Storage_interface.uri ; + None + ) + | _ -> + debug "No tapdisk found for backend: %s" (Storage_interface.(rpc_of backend) backend |> Rpc.to_string) ; None @@ -360,23 +371,32 @@ let with_activated_disk ~dbg ~sr ~vdi ~dp f = in finally (fun () -> - let path = + let path_and_nbd = Option.map (fun (vdi, backend) -> - let _, blockdevs, files, _ = + let _xendisks, blockdevs, files, nbds = Storage_interface.implementations_of_backend backend in - match (blockdevs, files) with - | {path} :: _, _ | _, {path} :: _ -> + match (files, blockdevs, nbds) with + | {path} :: _, _, _ | _, {path} :: _, _ -> + Local.VDI.activate3 dbg dp sr vdi (vm_of_s "0") ; + (path, false) + | _, _, nbd :: _ -> Local.VDI.activate3 dbg dp sr vdi (vm_of_s "0") ; - path - | [], [] -> + let unix_socket_path, export_name = + Storage_interface.parse_nbd_uri nbd + in + ( Attach_helpers.NbdClient.start_nbd_client ~unix_socket_path + ~export_name + , true + ) + | [], [], [] -> raise (Storage_interface.Storage_error (Backend_error ( Api_errors.internal_error , [ - "No BlockDevice or File implementation in \ + "No File, BlockDevice or Nbd implementation in \ Datapath.attach response: " ^ (Storage_interface.(rpc_of backend) backend |> Jsonrpc.to_string @@ -389,8 +409,16 @@ let with_activated_disk ~dbg ~sr ~vdi ~dp f = attached_vdi in finally - (fun () -> f path) + (fun () -> f (Option.map (function path, _ -> path) path_and_nbd)) (fun () -> + Option.iter + (function + | path, true -> + Attach_helpers.NbdClient.stop_nbd_client ~nbd_device:path + | _ -> + () + ) + path_and_nbd ; Option.iter (fun vdi -> Local.VDI.deactivate dbg dp sr vdi (vm_of_s "0")) vdi