Skip to content

xapi/attach_helpers: remove useless check for leaked vbds #7035

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

Draft
psafont wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

xapi/attach_helpers: remove useless check for leaked vbds #7035

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
87ddfe6
attach_helpers: only control domains can leak VBDs
psafont Apr 24, 2026
d78f943
xapi/attach_helpers: remove useless check for leaked vbds
psafont Apr 24, 2026
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
4 changes: 3 additions & 1 deletion ocaml/xapi/attach_helpers.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,12 @@ let safe_unplug rpc session_id self =
frontend (if so it will be linked to another frontend) *)
let has_vbd_leaked __context vbd =
let other_config = Db.VBD.get_other_config ~__context ~self:vbd in
let vm = Db.VBD.get_VM ~__context ~self:vbd in
let can_leak = Db.VM.get_is_control_domain ~__context ~self:vm in
let device = Db.VBD.get_device ~__context ~self:vbd in
let has_task = List.mem_assoc Xapi_globs.vbd_task_key other_config in
let has_related = List.mem_assoc Xapi_globs.related_to_key other_config in
if (not has_task) && not has_related then (
if (not can_leak) || ((not has_task) && not has_related) then (
Copy link
Copy Markdown
Member

@robhoes robhoes Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not can_leak means "not a control domain", doesn't this conflict with the comment below?

Copy link
Copy Markdown
Member

@robhoes robhoes Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh it's all gone in the second commit...

Copy link
Copy Markdown
Member Author

@psafont psafont Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can find this code is used on vdi clone and copy, so there was nothing to ensure this code applied to only control domains. I believe the comment to be wrong.

info "Ignoring orphaned disk attached to control domain (device = %s)"
device ;
false
Expand Down