Skip to content

Commit 38ff2a7

Browse files
authored
libs: add Stat module in Unixext to handle special device IDs (#6812)
These device IDs are handled in an ad-hoc way, and none of the implementations follow the actual linux behaviour, replace all of them with an in imeplementation that uses the glibc macros to handle these. Also change vhd_wrapper_tool to print better error messages when there's an error when getting the base VHD. Reproduction steps: - Create a pool with 2 hosts - SSH into one of the hosts and run `for i in $(seq 1 512) ; do tap-ctl allocate; done`. This forces the minor device ID to be higher than 255 for the local VBD created. - Create a guest in the local SR of the host where the previous command was run, with the guest tools installed to support live migration. - Live-migrate the guest between the hosts: ``` xe vm-migrate live=true uuid=ff50e9b1-c155-ed39-a2cc-680f9acaada4 host=pau-xcp-01 vdi:f613c010-7c06-4288-b0db-d1bde01fe0ef=23df0f9c-a9d8-5756-c829-e3655a225224 remote-master=pau-xcp-00 remote-username=root remote-password=xxx Performing a storage live migration. Your VM's VDIs will be migrated with the VM. Will migrate to remote host: pau-xcp-01, using remote network: Pool-wide network associated with eth0. Here is the VDI mapping: VDI f613c010-7c06-4288-b0db-d1bde01fe0ef -> SR 23df0f9c-a9d8-5756-c829-e3655a225224 The server failed to handle your request, due to an internal error. The given message may give details useful for debugging the problem. message: Storage_error ([S(Internal_error);S(Storage_error ([S(Migration_mirror_fd_failure);S(VDI Not Attached)]))]) ``` The migration succeeds when both hosts in the pool have the changes in this PR running.
2 parents 4e2ef11 + 097d1bc commit 38ff2a7

14 files changed

Lines changed: 303 additions & 56 deletions

File tree

ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/test/dune

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,13 @@
2020
(action (run %{deps} -v -bt))
2121
)
2222

23+
(test
24+
(modes exe)
25+
(name test_stat)
26+
(package xapi-stdext-unix)
27+
(modules test_stat)
28+
(libraries alcotest fmt xapi-stdext-unix unix))
29+
2330
(test
2431
(modes exe)
2532
(name test_systemd)
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
let major_nbd = 43
2+
3+
(** This module type helps us to implement alternative modules for [Stat].
4+
In particular one that uses the previous ad-hoc functions that where
5+
incorrect, and one that we can use as reference in case the behaviour
6+
changes and possibly change the users of [Stat].
7+
*)
8+
module type S = sig
9+
type device
10+
11+
val device : major:int -> minor:int -> device option
12+
13+
val encode_st_dev : device -> int
14+
15+
val decode_st_dev : int -> device
16+
17+
val major : device -> int
18+
19+
val minor : device -> int
20+
21+
val pp : Format.formatter -> device -> unit
22+
end
23+
24+
module Stat : S = struct
25+
include Xapi_stdext_unix.Unixext.Stat
26+
27+
let major {major; _} = major
28+
29+
let minor {minor; _} = minor
30+
31+
let pp =
32+
Fmt.(
33+
record ~sep:(any ", ")
34+
[
35+
field "major" (fun d -> d.major) int
36+
; field "minor" (fun d -> d.minor) int
37+
]
38+
)
39+
end
40+
41+
module Stat_reference : S = struct
42+
type device = {major: int; minor: int}
43+
44+
let ( << ) = Stdlib.( lsl )
45+
46+
let ( >> ) = Stdlib.( lsr )
47+
48+
let ( &^ ) = Stdlib.( land )
49+
50+
let ( |^ ) = Stdlib.( lor )
51+
52+
let device ~major ~minor =
53+
(* Linux's devids are 32-bit wide and the major and minor ones are 16-bit
54+
wide, but we can support well up to 32-bit-wide minors *)
55+
let minor_max = (1 << 32) - 1 in
56+
let major_max = (1 << 16) - 1 in
57+
if major < 0 || major_max < major || minor < 0 || minor_max < minor then
58+
None
59+
else
60+
Some {major; minor}
61+
62+
let encode_st_dev {major; minor} =
63+
0
64+
|^ (major &^ 0x00000fff << 8)
65+
|^ (major &^ 0x7ffff000 << 32)
66+
|^ (minor &^ 0x000000ff << 0)
67+
|^ (minor &^ 0xffffff00 << 12)
68+
69+
let decode_st_dev dev =
70+
(* follow glibc's implementation, with an exception: the most significant
71+
bit is ignored because ints are 63 bits in ocaml. In any case,
72+
[Unix.stat] returns a 63-bit int, so we can't do much in this code to
73+
avoid this. *)
74+
let major =
75+
0 |^ (dev &^ 0x7ffff00000000000 >> 32) |^ (dev &^ 0x00000000000fff00 >> 8)
76+
in
77+
let minor =
78+
0 |^ (dev &^ 0x00000ffffff00000 >> 12) |^ (dev &^ 0x00000000000000ff >> 0)
79+
in
80+
{major; minor}
81+
82+
let major {major; _} = major
83+
84+
let minor {minor; _} = minor
85+
86+
let pp =
87+
Fmt.(
88+
record ~sep:(any ", ")
89+
[
90+
field "major" (fun d -> d.major) int
91+
; field "minor" (fun d -> d.minor) int
92+
]
93+
)
94+
end
95+
96+
let hex = Alcotest.testable (Fmt.of_to_string (Format.sprintf "0x%x")) ( = )
97+
98+
let current_t = Alcotest.testable Stat.pp ( = )
99+
100+
let test_combinations f ~major:lst_a ~minor:lst_b =
101+
let test a b = (Printf.sprintf "major %i, minor %i" a b, `Quick, f a b) in
102+
List.concat_map (fun a -> List.map (test a) lst_b) lst_a
103+
104+
let spec_minor = [0; 31; 65; 256; 1025; 4098; (1 lsl 32) - 1]
105+
106+
let spec_major = [0; major_nbd; (1 lsl 16) - 1]
107+
108+
let test_reference =
109+
let test major minor () =
110+
let current = Stat.device ~major ~minor |> Option.get in
111+
let reference = Stat_reference.device ~major ~minor |> Option.get in
112+
let encoded_cur = Stat.encode_st_dev current in
113+
let encoded_ref = Stat_reference.encode_st_dev reference in
114+
115+
Alcotest.check hex "Encode must match reference implementation" encoded_ref
116+
encoded_cur ;
117+
118+
let decoded_cur = Stat.decode_st_dev encoded_ref in
119+
let decoded_ref = Stat_reference.decode_st_dev encoded_ref in
120+
121+
Alcotest.(check @@ pair int int)
122+
"Decode must match reference implementation"
123+
Stat_reference.(major decoded_ref, minor decoded_ref)
124+
Stat.(major decoded_cur, minor decoded_cur)
125+
in
126+
let tests = test_combinations test ~major:spec_major ~minor:spec_minor in
127+
("Compare with reference", tests)
128+
129+
let test_roundtrip =
130+
let test major minor () =
131+
let current = Stat.device ~major ~minor |> Option.get in
132+
let encoded_cur = Stat.encode_st_dev current in
133+
134+
let decoded_cur = Stat.decode_st_dev encoded_cur in
135+
Alcotest.check current_t "Roundtripped current" current decoded_cur
136+
in
137+
let tests = test_combinations test ~major:spec_major ~minor:spec_minor in
138+
("Roundtrip", tests)
139+
140+
let tests = [test_reference; test_roundtrip]
141+
142+
let () = Alcotest.run "Uniext.Stat suite" tests

ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/test/test_stat.mli

Whitespace-only changes.

ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext.ml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,3 +1002,19 @@ let with_socket_timeout fd timeout_opt f =
10021002
set_socket_timeout fd t ; Fun.protect ~finally f
10031003
| None ->
10041004
f ()
1005+
1006+
module Stat = struct
1007+
type device = {major: int; minor: int}
1008+
1009+
let device ~major ~minor = Some {major; minor}
1010+
1011+
external makedev : int -> int -> int = "stub_makedev" [@@noalloc]
1012+
1013+
let encode_st_dev {major; minor} = makedev major minor
1014+
1015+
external get_major : int -> int = "stub_major" [@@noalloc]
1016+
1017+
external get_minor : int -> int = "stub_minor" [@@noalloc]
1018+
1019+
let decode_st_dev dev = {major= get_major dev; minor= get_minor dev}
1020+
end

ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext.mli

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,3 +312,29 @@ module Daemon : sig
312312
313313
See sd_booted(3) for more information. *)
314314
end
315+
316+
module Stat : sig
317+
type device = private {major: int; minor: int}
318+
(* A Linux-specific device ID *)
319+
320+
val decode_st_dev : int -> device
321+
(** [decode_st_dev st_dev] decodes the integer [st_dev] into a device, with
322+
separate major and minor device IDs. *)
323+
324+
(**/**)
325+
326+
(* Testing-specific functions.
327+
For more information on how device IDs are handles in linux, see
328+
https://github.com/torvalds/linux/blob/ea1013c1539270e372fc99854bc6e4d94eaeff66/include/linux/kdev_t.h#L39
329+
and how glibc handles them, see
330+
https://elixir.bootlin.com/glibc/glibc-2.42.9000/source/bits/sysmacros.h#L37
331+
*)
332+
333+
val device : major:int -> minor:int -> device option
334+
(** [device ~major ~minor] creates a device datatype if [major] and [minor]
335+
are 32-bit wide or less, or returns [None]. *)
336+
337+
val encode_st_dev : device -> int
338+
(** [encode_st_dev device] encodes [device] into a single integer, using
339+
glibc's [makedev] macro *)
340+
end

ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_stubs.c

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <stdio.h> /* snprintf */
2323
#include <sys/ioctl.h>
2424
#include <sys/statvfs.h>
25+
#include <sys/sysmacros.h> /* needed for minor and major macros */
2526
#if defined(__linux__)
2627
# include <linux/fs.h>
2728
#endif
@@ -170,3 +171,33 @@ CAMLprim value stub_statvfs(value filename)
170171

171172
CAMLreturn(v);
172173
}
174+
175+
CAMLprim value stub_makedev(value majo, value mino)
176+
{
177+
CAMLparam2(majo, mino);
178+
long ret;
179+
180+
ret = makedev(Long_val(majo), Long_val(mino));
181+
182+
CAMLreturn(Val_long(ret));
183+
}
184+
185+
CAMLprim value stub_major(value dev)
186+
{
187+
CAMLparam1(dev);
188+
long ret;
189+
190+
ret = major(Long_val(dev));
191+
192+
CAMLreturn(Val_long(ret));
193+
}
194+
195+
CAMLprim value stub_minor(value dev)
196+
{
197+
CAMLparam1(dev);
198+
long ret;
199+
200+
ret = minor(Long_val(dev));
201+
202+
CAMLreturn(Val_long(ret));
203+
}

ocaml/tapctl/tapctl.ml

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -507,18 +507,15 @@ let stats ctx t =
507507
let read_proc_devices () : (int * string) list =
508508
let parse_line x =
509509
match List.filter (fun x -> x <> "") (String.split_on_char ' ' x) with
510-
| [x; y] -> (
511-
try Some (int_of_string x, y) with _ -> None
512-
)
510+
| [x; y] ->
511+
Option.bind (int_of_string_opt x) (fun x -> Some (x, y))
513512
| _ ->
514513
None
515514
in
516-
List.concat
517-
(List.map Option.to_list
518-
(Unixext.file_lines_fold
519-
(fun acc x -> parse_line x :: acc)
520-
[] "/proc/devices"
521-
)
515+
List.concat_map Option.to_list
516+
(Unixext.file_lines_fold
517+
(fun acc x -> parse_line x :: acc)
518+
[] "/proc/devices"
522519
)
523520

524521
let driver_of_major major = List.assoc major (read_proc_devices ())
@@ -529,9 +526,9 @@ exception Not_a_device
529526

530527
let of_device ctx path =
531528
let stat = Unix.stat path in
529+
let module Stat = Unixext.Stat in
532530
if stat.Unix.st_kind <> Unix.S_BLK then raise Not_a_device ;
533-
let major = stat.Unix.st_rdev / 256 in
534-
let minor = stat.Unix.st_rdev mod 256 in
531+
let Stat.{major; minor} = Stat.decode_st_dev stat.Unix.st_rdev in
535532
if driver_of_major major <> "tapdev" then raise Not_blktap ;
536533
match List.filter (fun (tapdev, _, _) -> tapdev.minor = minor) (list ctx) with
537534
| [t] ->

ocaml/vhd-tool/cli/dune

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
vhd-format-lwt
2020
xapi-idl
2121
xapi-log
22+
xapi-stdext-unix
2223
xenstore_transport.unix
2324
ezxenstore
2425
)

ocaml/vhd-tool/cli/sparse_dd.ml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,10 +213,11 @@ let after f g =
213213
let find_backend_device path =
214214
try
215215
let open Ezxenstore_core.Xenstore in
216+
let module Stat = Xapi_stdext_unix.Unixext.Stat in
216217
(* If we're looking at a xen frontend device, see if the backend
217218
is in the same domain. If so check if it looks like a .vhd *)
218219
let rdev = (Unix.LargeFile.stat path).Unix.LargeFile.st_rdev in
219-
let major = rdev / 256 and minor = rdev mod 256 in
220+
let Stat.{major; minor} = Stat.decode_st_dev rdev in
220221
let link =
221222
Unix.readlink (Printf.sprintf "/sys/dev/block/%d:%d/device" major minor)
222223
in

ocaml/vhd-tool/src/image.ml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1+
module Stat = Xapi_stdext_unix.Unixext.Stat
2+
13
let get_device_numbers path =
2-
let rdev = (Unix.LargeFile.stat path).Unix.LargeFile.st_rdev in
3-
let major = rdev / 256 and minor = rdev mod 256 in
4-
(major, minor)
4+
Unix.LargeFile.((stat path).st_rdev) |> Stat.decode_st_dev
55

66
let is_nbd_device path =
77
let nbd_device_num = 43 in
8-
let major, _ = get_device_numbers path in
8+
let Stat.{major; _} = get_device_numbers path in
99
major = nbd_device_num
1010

1111
type t = [`Vhd of string | `Raw of string | `Nbd of string * string]

0 commit comments

Comments
 (0)