Skip to content

Revert "oxenstored: Add 'depth' parameter handling for special watches" #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 16, 2025
Merged

Revert "oxenstored: Add 'depth' parameter handling for special watches" #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 3 additions & 42 deletions oxenstored/connections.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,21 +174,6 @@ let add_watch cons con path token depth =
let apath = Connection.get_watch_path con path in
(* fail on invalid paths early by calling key_of_str before adding watch *)
let key = key_of_str apath in
let is_special_watch = apath.[0] = '@' in
( if is_special_watch then
(* No depth can be specified for @releaseDomain/domid watches.
Only depth=1 can be specified for other special watches *)
match depth with
| Some _ when String.starts_with ~prefix:"@releaseDomain/" apath ->
raise
(Invalid_argument
"@releaseDomain/domid does not accept a depth parameter"
)
| Some x when x <> 1 ->
raise (Invalid_argument "If set, special watches' depth can only be 1")
| _ ->
()
) ;
let watch = Connection.add_watch con (path, apath) token depth in
let watches =
if Trie.mem cons.watches key then
Expand Down Expand Up @@ -235,36 +220,12 @@ let fire_watches ?oldroot source root cons path recurse =

let send_watchevents con = Connection.source_flush_watchevents con

let fire_spec_watches root cons specpath domid =
let fire_spec_watches root cons specpath =
let source = find_domain cons 0 in

(* Trigger @releaseDomain/domid watches as well *)
let specpaths =
specpath
::
( if specpath = "@releaseDomain" then
[Printf.sprintf "%s/%d" specpath domid]
else
[]
)
in
iter cons (fun con ->
List.iter
(fun specpath ->
List.iter
(fun w ->
(* Report the path as '@xDomain/domid' if depth is specified *)
let watch =
if w.Connection.depth = Some 1 then
{w with path= Printf.sprintf "%s/%d" specpath domid}
else
w
in
Connection.fire_single_watch source (None, root) 0 watch
)
(Connection.get_watches con specpath)
)
specpaths
(Connection.fire_single_watch source (None, root) 0)
(Connection.get_watches con specpath)
)

let set_target cons domain target_domain =
Expand Down
4 changes: 2 additions & 2 deletions oxenstored/process.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,7 @@ let do_introduce con t domains cons data =
let ndom = Domains.create ~remote_port domains domid mfn in
Connections.add_domain cons ndom ;
Connections.fire_spec_watches (Transaction.get_root t) cons
Store.Path.introduce_domain domid ;
Store.Path.introduce_domain ;
ndom
with _ -> raise Invalid_Cmd_Args
in
Expand All @@ -823,7 +823,7 @@ let do_release con t domains cons data =
Store.reset_permissions (Transaction.get_store t) domid ;
if fire_spec_watches then
Connections.fire_spec_watches (Transaction.get_root t) cons
Store.Path.release_domain domid
Store.Path.release_domain
else
raise Invalid_Cmd_Args

Expand Down
9 changes: 3 additions & 6 deletions oxenstored/xenstored.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,12 +567,9 @@ let main () =
let notify, deaddom = Domains.cleanup domains in
List.iter (Store.reset_permissions store) deaddom ;
List.iter (Connections.del_domain cons) deaddom ;
if notify then
List.iter
(Connections.fire_spec_watches (Store.get_root store) cons
Store.Path.release_domain
)
deaddom
if deaddom <> [] || notify then
Connections.fire_spec_watches (Store.get_root store) cons
Store.Path.release_domain
) else
let c = Connections.find_domain_by_port cons port in
match Connection.get_domain c with
Expand Down
88 changes: 0 additions & 88 deletions tests/unit_tests.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,93 +580,6 @@ let test_watches_depth () =
; (dom1, none, (Read, ["/a/1"]), (Error, ["ENOENT"]))
]

(* Check that @introduceDomain and @releaseDomain watches appear on
respective calls and depth is well-handled.
From documentation upstream:
The semantics for a specification of <depth> differ for generating
<wspecial> events: specifying "1" will report the related domid by using
@<wspecial>/<domid> for the reported path. Other <depth>
values are not supported. *)
let test_special_watches_depth () =
let store, doms, cons = initialize () in
let dom0 = create_dom0_conn cons doms in

(* Adding a special watch with a negative depth fails *)
run store cons doms
[
( dom0
, none
, (Watch, ["@introduceDomain"; "token"; "-1"])
, (Error, ["EINVAL"])
)
] ;
assert_watches dom0 [] ;

(* Adding a special watch with a depth other than 1 fails *)
run store cons doms
[
( dom0
, none
, (Watch, ["@releaseDomain"; "token"; "0"])
, (Error, ["EINVAL"])
)
] ;
assert_watches dom0 [] ;

(* Adding a @releaseDomain/domid with any depth fails *)
run store cons doms
[
( dom0
, none
, (Watch, ["@releaseDomain/5"; "token"; "1"])
, (Error, ["EINVAL"])
)
] ;
assert_watches dom0 [] ;

(* Adding a special watch with depth=1 works *)
run store cons doms
[(dom0, none, (Watch, ["@introduceDomain"; "token"; "1"]), (Watch, ["OK"]))] ;
assert_watches dom0 [("@introduceDomain", "token", Some 1)] ;

check_for_watchevent dom0 "@introduceDomain" "token" ;

run store cons doms
[(dom0, none, (Watch, ["@releaseDomain"; "token"; "1"]), (Watch, ["OK"]))] ;
assert_watches dom0
[("@releaseDomain", "token", Some 1); ("@introduceDomain", "token", Some 1)] ;
check_for_watchevent dom0 "@releaseDomain" "token" ;

run store cons doms
[(dom0, none, (Watch, ["@releaseDomain/5"; "token2"]), (Watch, ["OK"]))] ;
assert_watches dom0
[
("@releaseDomain", "token", Some 1)
; ("@introduceDomain", "token", Some 1)
; ("@releaseDomain/5", "token2", None)
] ;
check_for_watchevent dom0 "@releaseDomain/5" "token2" ;

(* Watchevents generated contain the new domid *)
run store cons doms
[
( dom0
, none
, (Introduce, ["5"; "5"; "5"])
, (Watchevent, ["@introduceDomain/5"; "token"])
)
] ;
let actual = Xenbus.Xb.unsafe_pop_output dom0.xb in
check_result actual (Introduce, ["OK"]) ;

run store cons doms
[
(dom0, none, (Release, ["5"]), (Watchevent, ["@releaseDomain/5"; "token"]))
] ;
check_for_watchevent dom0 "@releaseDomain/5" "token2" ;
let actual = Xenbus.Xb.unsafe_pop_output dom0.xb in
check_result actual (Release, ["OK"])

(* Check that a write failure doesn't generate a watch *)
let test_no_watch_on_error () =
let store, doms, cons = initialize () in
Expand Down Expand Up @@ -879,7 +792,6 @@ let () =
; ("test_recursive_rm_watch", `Quick, test_recursive_rm_watch)
; ("test_no_watch_on_error", `Quick, test_no_watch_on_error)
; ("test_watches_depth", `Quick, test_watches_depth)
; ("test_special_watches_depth", `Quick, test_special_watches_depth)
]
)
; ( "Quota tests"
Expand Down