-
Couldn't load subscription status.
- Fork 293
CP-17481: Allow other VMs to use is_control_domain #2675
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
The API has always had documentation that allows for more than one VM to have `is_control_domain = true`: > field ... "is_control_domain" "true if this is a control domain (domain 0 or a driver domain)"; Most of Xapi has been written with this in mind, e.g. `Xapi_host.destroy` which iterates through a list, `my_control_domains`. However there are a few places where it is assumed that dom0 is the only VM with `is_control_domain = true` and this patch makes these a bit more sane. Where the original code assumed only one control domain and did an operation that only makes sense for domain 0, an additional filter is applied to only operate on VMs with `domid = 0`. Where the original code assumed only one control domain and did an operation that makes sense for all control domains then this has been extended to operate on the list of VMs with `is_control_domain = true`. Signed-off-by: Si Beaumont <simon.beaumont@citrix.com>
…shable from other control domains Signed-off-by: Konstantina Chremmou <konstantina.chremmou@citrix.com>
|
Ah wait, there's some stuff I forgot. We want to change the way Xapi distinguishes dom0 in this pull request too. We can do that tomorrow. |
ocaml/xapi/helpers.ml
Outdated
| (* Expects only 1 control domain per host; just return first in list for now if multiple.. *) | ||
| let is_domain_zero ~__context vm_ref = | ||
| Db.VM.get_is_control_domain ~__context ~self:vm_ref | ||
| && Db.VM.get_domid ~__context ~self:vm_ref = 0L |
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.
Use host.control_domain
|
Note to self: need to work out why the hell xen-git passed when travis failed. It should have failed as there are some places where the ~host parameter was missing to the ensure_domain_zero_records. But xen-git looks like it didn't even try and build xen-api.git... Investigation ongoing. |
|
Right. I've fixed the jenkins CI now (xenserver/xs-pull-request-build-scripts@67be5dc) and the job is now failing in the same way as Travis... this is good! |
ocaml/test/test_common.ml
Outdated
| (* Dbsync_slave.refresh_localhost_info ~__context host_info; *) | ||
| Xapi_globs.localhost_ref := Helpers.get_localhost ~__context; | ||
| Create_misc.ensure_domain_zero_records ~__context host_info; | ||
| Create_misc.ensure_domain_zero_records ~__context ~host:Xapi_globs.localhost_ref host_info; |
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 is missing a !. Should be !Xapi_globs.localhost_ref since this is a ref type, assigned above using :=, not =. This is the reason for the build failure:
File "test_common.ml", line 63, characters 57-81:
Error: This expression has type [ `host ] Ref.t ref
but an expression was expected of type [ `host ] Ref.t
|
I think this is looking in a good state now. It's missing one more commit (which I'm happy to do) to make a database upgrade rule for this new field so that an existing host won't try and create a new dom0 when upgraded to this Xapi on account of |
| (* Dbsync_slave.refresh_localhost_info ~__context host_info; *) | ||
| Xapi_globs.localhost_ref := Helpers.get_localhost ~__context; | ||
| Create_misc.ensure_domain_zero_records ~__context ~host:Xapi_globs.localhost_ref host_info; | ||
| Create_misc.ensure_domain_zero_records ~__context ~host:!Xapi_globs.localhost_ref host_info; |
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.
Please squash this into the previous commit.
… host.control_domain field. Signed-off-by: Konstantina Chremmou <konstantina.chremmou@citrix.com>
Signed-off-by: Konstantina Chremmou <konstantina.chremmou@citrix.com>
This needs to be done before the "hi-level" database upgrade because the code to ensure the VM record for dom0 exists is in the dbsync logic which is called as part of starting the database engine. This all happens before the DB upgrade, and therefore, the dbsync logic tries to create an additional dom0 record because it does not have the Host.control_domain link. This patch adds the Host.control_domain link during the dbsync if it finds a VM with the CONTROL_DOMAIN_UUID from the Xensource_inventory. Signed-off-by: Si Beaumont <simon.beaumont@citrix.com>
No description provided.