-
Notifications
You must be signed in to change notification settings - Fork 67
CA-334758 - Allow plug of vbds for sds vms #755
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.
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
Conversation
There was a problem hiding this 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
|
Marking as draft since it's a work in progress |
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>
87834fb to
616f9bf
Compare
|
It works: |
|
It works: |
There was a problem hiding this 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?
|
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). |
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.