Skip to content

Conversation

@kc284
Copy link

@kc284 kc284 commented Jun 13, 2016

No description provided.

simonjbeaumont and others added 2 commits June 13, 2016 15:26
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>
@simonjbeaumont
Copy link
Contributor

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.

(* 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
Copy link
Author

Choose a reason for hiding this comment

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

Use host.control_domain

@simonjbeaumont
Copy link
Contributor

Just made some notes with @kc284 as to what should be changed now that we have the new Host.control_domain field from bff6184.

@simonjbeaumont
Copy link
Contributor

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.

@simonjbeaumont
Copy link
Contributor

simonjbeaumont commented Jun 15, 2016

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!

(* 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;
Copy link
Contributor

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

@simonjbeaumont
Copy link
Contributor

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 Host.control_domain being defaulted to Ref.null.

(* 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;
Copy link
Contributor

@euanh euanh Jun 20, 2016

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.

Konstantina Chremmou and others added 3 commits June 20, 2016 22:43
… 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>
@euanh euanh merged commit c76b956 into xapi-project:master Jun 21, 2016
@kc284 kc284 deleted the cp-17481 branch July 4, 2016 08:26
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.

3 participants