Skip to content

Conversation

@BenSimsCitrix
Copy link
Contributor

This is another piece of logic that was blocking vbds being plugged into dom0

I need to run a final BST to ensure no regressions, hence the no merge flag.

Copy link
Contributor

@edwintorok edwintorok left a comment

Choose a reason for hiding this comment

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

This looks good functionally

@psafont
Copy link
Member

psafont commented May 25, 2021

Marking as draft since it's a work in progress

@psafont psafont marked this pull request as draft May 25, 2021 07:41
Cache Dom0's UUID in Xenops_server.dom0_uuid: it doesn't change
and needs to be known on each VBD plug operation.

When storage driver domains are used we need to plug VBDs into Dom0,
because the backend runs in another domain and we don't have direct access
to the VDIs.
Xenopsd only considers VMs that it manages as valid targets for VBD
plug, so Dom0 needs to be added to that condition explicitly.

The 'is managed by me' condition is not changed, because we only want to
allow this for VBD operations, and not for other operations.

Signed-off-by: bensi <ben.sims@citrix.com>
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
@edwintorok edwintorok force-pushed the private/bensi/CA-334758 branch from 87834fb to 616f9bf Compare July 23, 2021 15:28
@edwintorok edwintorok changed the base branch from feature/REQ-802-master to master July 23, 2021 15:29
@edwintorok
Copy link
Contributor

It works:

# /opt/xensource/debug/with-vdi 7d89b745-a8e2-4514-bbf9-1f339819af4d
DEVICE=xvda
sh-4.2#

@edwintorok edwintorok marked this pull request as ready for review July 23, 2021 17:16
@edwintorok
Copy link
Contributor

It works:

# /opt/xensource/debug/with-vdi 7d89b745-a8e2-4514-bbf9-1f339819af4d
DEVICE=xvda
sh-4.2#

Copy link
Contributor

@lindig lindig left a comment

Choose a reason for hiding this comment

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

Should we add an assertion that dom0_uuid is not the empty string before it is used? Or use an option type for this such that we force the client to handle this case?

@edwintorok
Copy link
Contributor

I think the xenopsd simulator won't have a Dom0. I can try to add a unit test for this and modify the simulator in a separate PR (it'll also need changes in XAPI, which is where the simulator is used).

@edwintorok edwintorok merged commit 3fe01ff into master Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants