| Message ID | 559929d2ae95f6527e5050051c917b7586182ad2.1605636800.git.edvin.torok@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tools/ocaml/libs/xc: domid control at domain creation time | expand |
On 17/11/2020 18:24, Edwin Török wrote: > One can specify the domid to use when creating the domain, but this was hardcoded to 0. > > Keep the existing `domain_create` function (and the type of its parameters) as is to make > backwards compatibility easier. > Introduce a new `domain_create_domid` OCaml API that allows specifying the domid. > A new version of xenopsd can choose to start using this, while old versions of xenopsd will keep > building and using the old API. > > Controlling the domid can be useful during testing or migration. > > Signed-off-by: Edwin Török <edvin.torok@citrix.com> > --- > tools/ocaml/libs/xc/xenctrl.ml | 3 +++ > tools/ocaml/libs/xc/xenctrl.mli | 2 ++ > tools/ocaml/libs/xc/xenctrl_stubs.c | 9 +++++++-- > 3 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml > index e878699b0a..9d720886e9 100644 > --- a/tools/ocaml/libs/xc/xenctrl.ml > +++ b/tools/ocaml/libs/xc/xenctrl.ml > @@ -182,6 +182,9 @@ let with_intf f = > external domain_create: handle -> domctl_create_config -> domid > = "stub_xc_domain_create" > > +external domain_create_domid: handle -> domctl_create_config -> domid -> domid > + = "stub_xc_domain_create_domid" Wouldn't this be better as handle -> domid -> domctl_create_config -> domid ? I'm not overwhelmed with the name, but "domain_create_{specific,with}_domid" don't seem much better either. > + > external domain_sethandle: handle -> domid -> string -> unit > = "stub_xc_domain_sethandle" > > diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli > index e64907df8e..e629022901 100644 > --- a/tools/ocaml/libs/xc/xenctrl.mli > +++ b/tools/ocaml/libs/xc/xenctrl.mli > @@ -145,6 +145,8 @@ val close_handle: unit -> unit > > external domain_create : handle -> domctl_create_config -> domid > = "stub_xc_domain_create" > +external domain_create_domid : handle -> domctl_create_config -> domid -> domid > + = "stub_xc_domain_create_domid" > external domain_sethandle : handle -> domid -> string -> unit = "stub_xc_domain_sethandle" > external domain_max_vcpus : handle -> domid -> int -> unit > = "stub_xc_domain_max_vcpus" > diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c > index 94aba38a42..bb718fd164 100644 > --- a/tools/ocaml/libs/xc/xenctrl_stubs.c > +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c > @@ -175,7 +175,7 @@ static unsigned int ocaml_list_to_c_bitmap(value l) > return val; > } > > -CAMLprim value stub_xc_domain_create(value xch, value config) > +CAMLprim value stub_xc_domain_create_domid(value xch, value config, value want_domid) > { > CAMLparam2(xch, config); > CAMLlocal2(l, arch_domconfig); > @@ -191,7 +191,7 @@ CAMLprim value stub_xc_domain_create(value xch, value config) > #define VAL_MAX_MAPTRACK_FRAMES Field(config, 7) > #define VAL_ARCH Field(config, 8) > > - uint32_t domid = 0; > + uint32_t domid = Int_val(want_domid); wanted_domid would be a slightly better name, because it isn't ambiguous with a boolean flag. > int result; > struct xen_domctl_createdomain cfg = { > .ssidref = Int32_val(VAL_SSIDREF), > @@ -262,6 +262,11 @@ CAMLprim value stub_xc_domain_create(value xch, value config) > CAMLreturn(Val_int(domid)); > } > > +CAMLprim value stub_xc_domain_create(value xch, value config, value want_domid) > +{ > + return stub_xc_domain_create_domid(xch, config, Val_int(0)); > +} Using https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=36d94c17fa1e48cc9fb9ed15bc9a2237a1738bbb as reverse inspiration, couldn't we do the insertion of 0 at the Ocaml level and avoid doubling up the C stub? ~Andrew > + > CAMLprim value stub_xc_domain_max_vcpus(value xch, value domid, > value max_vcpus) > {
On Wed, 2020-11-18 at 18:13 +0000, Andrew Cooper wrote: > On 17/11/2020 18:24, Edwin Török wrote: > > One can specify the domid to use when creating the domain, but this > > was hardcoded to 0. > > > > Keep the existing `domain_create` function (and the type of its > > parameters) as is to make > > backwards compatibility easier. > > Introduce a new `domain_create_domid` OCaml API that allows > > specifying the domid. > > A new version of xenopsd can choose to start using this, while old > > versions of xenopsd will keep > > building and using the old API. > > > > Controlling the domid can be useful during testing or migration. > > > > Signed-off-by: Edwin Török <edvin.torok@citrix.com> > > --- > > tools/ocaml/libs/xc/xenctrl.ml | 3 +++ > > tools/ocaml/libs/xc/xenctrl.mli | 2 ++ > > tools/ocaml/libs/xc/xenctrl_stubs.c | 9 +++++++-- > > 3 files changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/tools/ocaml/libs/xc/xenctrl.ml > > b/tools/ocaml/libs/xc/xenctrl.ml > > index e878699b0a..9d720886e9 100644 > > --- a/tools/ocaml/libs/xc/xenctrl.ml > > +++ b/tools/ocaml/libs/xc/xenctrl.ml > > @@ -182,6 +182,9 @@ let with_intf f = > > external domain_create: handle -> domctl_create_config -> domid > > = "stub_xc_domain_create" > > > > +external domain_create_domid: handle -> domctl_create_config -> > > domid -> domid > > + = "stub_xc_domain_create_domid" > > Wouldn't this be better as handle -> domid -> domctl_create_config -> > domid ? > > I'm not overwhelmed with the name, but > "domain_create_{specific,with}_domid" don't seem much better either. > > > + > > external domain_sethandle: handle -> domid -> string -> unit > > = "stub_xc_domain_sethandle" > > > > diff --git a/tools/ocaml/libs/xc/xenctrl.mli > > b/tools/ocaml/libs/xc/xenctrl.mli > > index e64907df8e..e629022901 100644 > > --- a/tools/ocaml/libs/xc/xenctrl.mli > > +++ b/tools/ocaml/libs/xc/xenctrl.mli > > @@ -145,6 +145,8 @@ val close_handle: unit -> unit > > > > external domain_create : handle -> domctl_create_config -> domid > > = "stub_xc_domain_create" > > +external domain_create_domid : handle -> domctl_create_config -> > > domid -> domid > > + = "stub_xc_domain_create_domid" > > external domain_sethandle : handle -> domid -> string -> unit = > > "stub_xc_domain_sethandle" > > external domain_max_vcpus : handle -> domid -> int -> unit > > = "stub_xc_domain_max_vcpus" > > diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c > > b/tools/ocaml/libs/xc/xenctrl_stubs.c > > index 94aba38a42..bb718fd164 100644 > > --- a/tools/ocaml/libs/xc/xenctrl_stubs.c > > +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c > > @@ -175,7 +175,7 @@ static unsigned int > > ocaml_list_to_c_bitmap(value l) > > return val; > > } > > > > -CAMLprim value stub_xc_domain_create(value xch, value config) > > +CAMLprim value stub_xc_domain_create_domid(value xch, value > > config, value want_domid) > > { > > CAMLparam2(xch, config); > > CAMLlocal2(l, arch_domconfig); > > @@ -191,7 +191,7 @@ CAMLprim value stub_xc_domain_create(value xch, > > value config) > > #define VAL_MAX_MAPTRACK_FRAMES Field(config, 7) > > #define VAL_ARCH Field(config, 8) > > > > - uint32_t domid = 0; > > + uint32_t domid = Int_val(want_domid); > > wanted_domid would be a slightly better name, because it isn't > ambiguous > with a boolean flag. > > > int result; > > struct xen_domctl_createdomain cfg = { > > .ssidref = Int32_val(VAL_SSIDREF), > > @@ -262,6 +262,11 @@ CAMLprim value stub_xc_domain_create(value > > xch, value config) > > CAMLreturn(Val_int(domid)); > > } > > > > +CAMLprim value stub_xc_domain_create(value xch, value config, > > value want_domid) > > +{ > > + return stub_xc_domain_create_domid(xch, config, Val_int(0)); > > +} > > Using > https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=36d94c17fa1e48cc9fb9ed15bc9a2237a1738bbb > as reverse inspiration, couldn't we do the insertion of 0 at the > Ocaml > level and avoid doubling up the C stub? I wanted to retain the old API for backwards compatibility, but you are right that this could be done just on the OCaml level, I'll update the patch. If you upgrade Xen without upgrading xenopsd you'll get a fairly obvious failure to start xenopsd due to the missing symbol, but that could be solved with an appropriate dependency at the distro package level. As long as matching Xen and xenopsd gets installed (even if not booted into) xenopsd should succeed in restarting then. Best regards, --Edwin > > ~Andrew > > > + > > CAMLprim value stub_xc_domain_max_vcpus(value xch, value domid, > > value max_vcpus) > > { >
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml index e878699b0a..9d720886e9 100644 --- a/tools/ocaml/libs/xc/xenctrl.ml +++ b/tools/ocaml/libs/xc/xenctrl.ml @@ -182,6 +182,9 @@ let with_intf f = external domain_create: handle -> domctl_create_config -> domid = "stub_xc_domain_create" +external domain_create_domid: handle -> domctl_create_config -> domid -> domid + = "stub_xc_domain_create_domid" + external domain_sethandle: handle -> domid -> string -> unit = "stub_xc_domain_sethandle" diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli index e64907df8e..e629022901 100644 --- a/tools/ocaml/libs/xc/xenctrl.mli +++ b/tools/ocaml/libs/xc/xenctrl.mli @@ -145,6 +145,8 @@ val close_handle: unit -> unit external domain_create : handle -> domctl_create_config -> domid = "stub_xc_domain_create" +external domain_create_domid : handle -> domctl_create_config -> domid -> domid + = "stub_xc_domain_create_domid" external domain_sethandle : handle -> domid -> string -> unit = "stub_xc_domain_sethandle" external domain_max_vcpus : handle -> domid -> int -> unit = "stub_xc_domain_max_vcpus" diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c index 94aba38a42..bb718fd164 100644 --- a/tools/ocaml/libs/xc/xenctrl_stubs.c +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c @@ -175,7 +175,7 @@ static unsigned int ocaml_list_to_c_bitmap(value l) return val; } -CAMLprim value stub_xc_domain_create(value xch, value config) +CAMLprim value stub_xc_domain_create_domid(value xch, value config, value want_domid) { CAMLparam2(xch, config); CAMLlocal2(l, arch_domconfig); @@ -191,7 +191,7 @@ CAMLprim value stub_xc_domain_create(value xch, value config) #define VAL_MAX_MAPTRACK_FRAMES Field(config, 7) #define VAL_ARCH Field(config, 8) - uint32_t domid = 0; + uint32_t domid = Int_val(want_domid); int result; struct xen_domctl_createdomain cfg = { .ssidref = Int32_val(VAL_SSIDREF), @@ -262,6 +262,11 @@ CAMLprim value stub_xc_domain_create(value xch, value config) CAMLreturn(Val_int(domid)); } +CAMLprim value stub_xc_domain_create(value xch, value config, value want_domid) +{ + return stub_xc_domain_create_domid(xch, config, Val_int(0)); +} + CAMLprim value stub_xc_domain_max_vcpus(value xch, value domid, value max_vcpus) {
One can specify the domid to use when creating the domain, but this was hardcoded to 0. Keep the existing `domain_create` function (and the type of its parameters) as is to make backwards compatibility easier. Introduce a new `domain_create_domid` OCaml API that allows specifying the domid. A new version of xenopsd can choose to start using this, while old versions of xenopsd will keep building and using the old API. Controlling the domid can be useful during testing or migration. Signed-off-by: Edwin Török <edvin.torok@citrix.com> --- tools/ocaml/libs/xc/xenctrl.ml | 3 +++ tools/ocaml/libs/xc/xenctrl.mli | 2 ++ tools/ocaml/libs/xc/xenctrl_stubs.c | 9 +++++++-- 3 files changed, 12 insertions(+), 2 deletions(-)