[PATCH v1 0/4] tools/ocaml/libs/xc: domid control at domain creation time

[PATCH v1 0/4] tools/ocaml/libs/xc: domid control at domain creation time

[Edwin Török]

The xl toolstack allows some control over the domid at VM creation time,
allow xenopsd similar control by exposing the appropriate domid field in the OCaml xenctrl bindings.
A new API function is introduced to preserve backwards compatibility without merge ordering
requirements between the Xen and xenopsd patches: Xen can merge the patch and xenopsd will keep
building with the old function, and a new version of xenopsd will start using the new function.

I've also included some build system fixes to allow me to test the build
in an upstream build environment:
```
cd automation/build
podman build -t registry.gitlab.com/xen-project/xen/ubuntu:focal -f ubuntu/focal.dockerfile ubuntu
DOCKER_CMD=podman CONTAINER_NO_PULL=1 CONTAINER=registry.gitlab.com/xen-project/xen/ubuntu:focal automation/scripts/containerize make build-tools-oxenstored
```

It'd be good if someone could test whether containerize still works on non-SELinux systems now, or
whether we need more detection logic in the script.

This works around bugs in the OCaml makefiles that end up in "inconsistent assumptions" by doing a
'make clean' before building the OCaml files every time. This is inefficient, but works.
Long term it would be beneficial to switch to Dune as build system,
which can do correct incremental builds with minimal configuration.
I'll send a separate patch series for that.

Edwin Török (4):
  automation/scripts/containerize: fix DOCKER_CMD=podman
  automation/: add Ubuntu:focal container
  Makefile: add build-tools-oxenstored
  tools/ocaml/libs/xc: backward compatible domid control at domain
    creation time

 Makefile                                 |  6 +++
 automation/build/ubuntu/focal.dockerfile | 50 ++++++++++++++++++++++++
 automation/scripts/containerize          |  7 ++--
 tools/ocaml/Makefile                     |  8 ++++
 tools/ocaml/libs/xc/xenctrl.ml           |  3 ++
 tools/ocaml/libs/xc/xenctrl.mli          |  2 +
 tools/ocaml/libs/xc/xenctrl_stubs.c      |  9 ++++-
 7 files changed, 80 insertions(+), 5 deletions(-)
 create mode 100644 automation/build/ubuntu/focal.dockerfile

-- 
2.18.4

[PATCH v1 1/4] automation/scripts/containerize: fix DOCKER_CMD=podman

[Edwin Török]

On CentOS 8 with SELinux containerize doesn't work at all:

Make sure that the source code and SSH agent directories are passed on
with SELinux relabeling enabled.
(`-security-opt label=disabled` would be another option)

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 automation/scripts/containerize | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/automation/scripts/containerize b/automation/scripts/containerize
index a75d54566c..ed991bb79c 100755
--- a/automation/scripts/containerize
+++ b/automation/scripts/containerize
@@ -7,7 +7,7 @@
 # and /etc/subgid.
 #
 docker_cmd=${DOCKER_CMD:-"docker"}
-[ "$DOCKER_CMD" = "podman" ] && userns_podman="--userns=keep-id"
+[ "$DOCKER_CMD" = "podman" ] && userns_podman="--userns=keep-id" selinux=",z"
 
 einfo() {
     echo "$*" >&2
@@ -95,9 +95,9 @@ einfo "*** Launching container ..."
 exec ${docker_cmd} run \
     ${userarg} \
     ${SSH_AUTH_SOCK:+-e SSH_AUTH_SOCK="/tmp/ssh-agent/${SSH_AUTH_NAME}"} \
-    -v "${CONTAINER_PATH}":/build:rw \
+    -v "${CONTAINER_PATH}":/build:rw${selinux} \
     -v "${HOME}/.ssh":/root/.ssh:ro \
-    ${SSH_AUTH_DIR:+-v "${SSH_AUTH_DIR}":/tmp/ssh-agent} \
+    ${SSH_AUTH_DIR:+-v "${SSH_AUTH_DIR}":/tmp/ssh-agent${selinux}} \
     ${XEN_CONFIG_EXPERT:+-e XEN_CONFIG_EXPERT=${XEN_CONFIG_EXPERT}} \
     ${CONTAINER_ARGS} \
     -${termint}i --rm -- \
-- 
2.18.4

[PATCH v1 2/4] automation/: add Ubuntu:focal container

[Edwin Török]

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 automation/build/ubuntu/focal.dockerfile | 50 ++++++++++++++++++++++++
 automation/scripts/containerize          |  1 +
 2 files changed, 51 insertions(+)
 create mode 100644 automation/build/ubuntu/focal.dockerfile

diff --git a/automation/build/ubuntu/focal.dockerfile b/automation/build/ubuntu/focal.dockerfile
new file mode 100644
index 0000000000..1f014b67bc
--- /dev/null
+++ b/automation/build/ubuntu/focal.dockerfile
@@ -0,0 +1,50 @@
+FROM ubuntu:20.04
+LABEL maintainer.name="The Xen Project " \
+      maintainer.email="xen-devel@lists.xenproject.org"
+
+ENV DEBIAN_FRONTEND=noninteractive
+ENV USER root
+
+RUN mkdir /build
+WORKDIR /build
+
+# build depends
+RUN apt-get update && \
+    apt-get --quiet --yes install \
+        build-essential \
+        zlib1g-dev \
+        libncurses5-dev \
+        libssl-dev \
+        python-dev \
+        python3-dev \
+        xorg-dev \
+        uuid-dev \
+        libyajl-dev \
+        libaio-dev \
+        libglib2.0-dev \
+        clang \
+        libpixman-1-dev \
+        pkg-config \
+        flex \
+        bison \
+        gettext \
+        acpica-tools \
+        bin86 \
+        bcc \
+        liblzma-dev \
+        libc6-dev-i386 \
+        libnl-3-dev \
+        ocaml-nox \
+        libfindlib-ocaml-dev \
+        libsystemd-dev \
+        markdown \
+        transfig \
+        pandoc \
+        checkpolicy \
+        wget \
+        git \
+        nasm \
+        && \
+        apt-get autoremove -y && \
+        apt-get clean && \
+        rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/*
diff --git a/automation/scripts/containerize b/automation/scripts/containerize
index ed991bb79c..94ff8b1ca8 100755
--- a/automation/scripts/containerize
+++ b/automation/scripts/containerize
@@ -29,6 +29,7 @@ case "_${CONTAINER}" in
     _centos7) CONTAINER="${BASE}/centos:7" ;;
     _centos72) CONTAINER="${BASE}/centos:7.2" ;;
     _fedora) CONTAINER="${BASE}/fedora:29";;
+    _focal) CONTAINER="${BASE}/ubuntu:focal" ;;
     _jessie) CONTAINER="${BASE}/debian:jessie" ;;
     _stretch|_) CONTAINER="${BASE}/debian:stretch" ;;
     _unstable|_) CONTAINER="${BASE}/debian:unstable" ;;
-- 
2.18.4

[PATCH v1 3/4] Makefile: add build-tools-oxenstored

[Edwin Török]

As a convenience so that oxenstored patches can be compile-tested
using upstream's build-system before submitting upstream.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 Makefile             | 6 ++++++
 tools/ocaml/Makefile | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/Makefile b/Makefile
index 9ad2602f63..96d32cfd50 100644
--- a/Makefile
+++ b/Makefile
@@ -62,6 +62,12 @@ build-xen:
 build-tools: build-tools-public-headers
 	$(MAKE) -C tools build
 
+.PHONY: build-tools-oxenstored
+build-tools-oxenstored: build-tools-public-headers
+	$(MAKE) -s -C tools/ocaml clean
+	$(MAKE) -s -C tools/libs
+	$(MAKE) -C tools/ocaml build-tools-oxenstored
+
 .PHONY: build-stubdom
 build-stubdom: mini-os-dir build-tools-public-headers
 	$(MAKE) -C stubdom build
diff --git a/tools/ocaml/Makefile b/tools/ocaml/Makefile
index 66f2d6b131..a7c04b6546 100644
--- a/tools/ocaml/Makefile
+++ b/tools/ocaml/Makefile
@@ -26,3 +26,11 @@ clean: subdirs-clean
 
 .PHONY: distclean
 distclean: subdirs-distclean
+
+.PHONY: build-tools-oxenstored
+build-tools-oxenstored:
+	$(MAKE) -s -C libs/eventchn
+	$(MAKE) -s -C libs/mmap
+	$(MAKE) -s -C libs/xb
+	$(MAKE) -s -C libs/xc
+	$(MAKE) -C xenstored
-- 
2.18.4

[PATCH v1 4/4] tools/ocaml/libs/xc: backward compatible domid control at domain creation time

[Edwin Török]

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"
+
 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)
 {
-- 
2.18.4

Re: [PATCH v1 0/4] tools/ocaml/libs/xc: domid control at domain creation time

[Christian Lindig]

I like the improvements for the build process but I wonder whether these should be mixed with functional code changes. But that is only a cosmetic concern as it might impact identifying patches when they are backported. The code change looks good to me, too. I support moving to Dune for building the OCaml part in the future.

Acked-by: Christian Lindig <christian.lindig@citrix.com>

________________________________________
From: Edwin Török <edvin.torok@citrix.com>
Sent: 17 November 2020 18:24
To: xen-devel@lists.xenproject.org
Cc: Edwin Torok; Doug Goldstein; Andrew Cooper; George Dunlap; Ian Jackson; Jan Beulich; Julien Grall; Stefano Stabellini; Wei Liu; Christian Lindig; David Scott
Subject: [PATCH v1 0/4] tools/ocaml/libs/xc: domid control at domain creation time

The xl toolstack allows some control over the domid at VM creation time,
allow xenopsd similar control by exposing the appropriate domid field in the OCaml xenctrl bindings.
A new API function is introduced to preserve backwards compatibility without merge ordering
requirements between the Xen and xenopsd patches: Xen can merge the patch and xenopsd will keep
building with the old function, and a new version of xenopsd will start using the new function.

I've also included some build system fixes to allow me to test the build
in an upstream build environment:
```
cd automation/build
podman build -t registry.gitlab.com/xen-project/xen/ubuntu:focal -f ubuntu/focal.dockerfile ubuntu
DOCKER_CMD=podman CONTAINER_NO_PULL=1 CONTAINER=registry.gitlab.com/xen-project/xen/ubuntu:focal automation/scripts/containerize make build-tools-oxenstored
```

It'd be good if someone could test whether containerize still works on non-SELinux systems now, or
whether we need more detection logic in the script.

This works around bugs in the OCaml makefiles that end up in "inconsistent assumptions" by doing a
'make clean' before building the OCaml files every time. This is inefficient, but works.
Long term it would be beneficial to switch to Dune as build system,
which can do correct incremental builds with minimal configuration.
I'll send a separate patch series for that.

Edwin Török (4):
  automation/scripts/containerize: fix DOCKER_CMD=podman
  automation/: add Ubuntu:focal container
  Makefile: add build-tools-oxenstored
  tools/ocaml/libs/xc: backward compatible domid control at domain
    creation time

 Makefile                                 |  6 +++
 automation/build/ubuntu/focal.dockerfile | 50 ++++++++++++++++++++++++
 automation/scripts/containerize          |  7 ++--
 tools/ocaml/Makefile                     |  8 ++++
 tools/ocaml/libs/xc/xenctrl.ml           |  3 ++
 tools/ocaml/libs/xc/xenctrl.mli          |  2 +
 tools/ocaml/libs/xc/xenctrl_stubs.c      |  9 ++++-
 7 files changed, 80 insertions(+), 5 deletions(-)
 create mode 100644 automation/build/ubuntu/focal.dockerfile

--
2.18.4

Re: [PATCH v1 1/4] automation/scripts/containerize: fix DOCKER_CMD=podman

[Doug Goldstein]

On 11/17/20 12:24 PM, Edwin Török wrote:
> On CentOS 8 with SELinux containerize doesn't work at all:
> 
> Make sure that the source code and SSH agent directories are passed on
> with SELinux relabeling enabled.
> (`-security-opt label=disabled` would be another option)
> 
> Signed-off-by: Edwin Török <edvin.torok@citrix.com>

Looks reasonable.

Acked-by: Doug Goldstein <cardoe@cardoe.com>

Re: [PATCH v1 2/4] automation/: add Ubuntu:focal container

[Doug Goldstein]

On 11/17/20 12:24 PM, Edwin Török wrote:
> Signed-off-by: Edwin Török <edvin.torok@citrix.com>

Looks good. Do you have permissions to push the container or do you need
me to?

Acked-by: Doug Goldstein <cardoe@cardoe.com>

Re: [PATCH v1 2/4] automation/: add Ubuntu:focal container

[Edwin Torok]

On Wed, 2020-11-18 at 10:40 -0600, Doug Goldstein wrote:
> 
> 
> On 11/17/20 12:24 PM, Edwin Török wrote:
> > Signed-off-by: Edwin Török <edvin.torok@citrix.com>
> 
> Looks good. Do you have permissions to push the container or do you
> need
> me to?
> 
> Acked-by: Doug Goldstein <cardoe@cardoe.com>

Thanks, if you could push it that'd be great.
I don't have any special permissions on the gitlab registry.

Best regards,
--Edwin

Re: [PATCH v1 2/4] automation/: add Ubuntu:focal container

[Andrew Cooper]

On 17/11/2020 18:24, Edwin Török wrote:
> Signed-off-by: Edwin Török <edvin.torok@citrix.com>
> ---
>  automation/build/ubuntu/focal.dockerfile | 50 ++++++++++++++++++++++++
>  automation/scripts/containerize          |  1 +
>  2 files changed, 51 insertions(+)
>  create mode 100644 automation/build/ubuntu/focal.dockerfile
>
> diff --git a/automation/build/ubuntu/focal.dockerfile b/automation/build/ubuntu/focal.dockerfile
> new file mode 100644
> index 0000000000..1f014b67bc
> --- /dev/null
> +++ b/automation/build/ubuntu/focal.dockerfile
> @@ -0,0 +1,50 @@
> +FROM ubuntu:20.04
> +LABEL maintainer.name="The Xen Project " \
> +      maintainer.email="xen-devel@lists.xenproject.org"
> +
> +ENV DEBIAN_FRONTEND=noninteractive
> +ENV USER root
> +
> +RUN mkdir /build
> +WORKDIR /build
> +
> +# build depends
> +RUN apt-get update && \
> +    apt-get --quiet --yes install \
> +        build-essential \
> +        zlib1g-dev \
> +        libncurses5-dev \
> +        libssl-dev \
> +        python-dev \

Python2 is legacy in Focal, and shouldn't be necessary for 4.14 and later.

> +        python3-dev \
> +        xorg-dev \
> +        uuid-dev \
> +        libyajl-dev \
> +        libaio-dev \
> +        libglib2.0-dev \
> +        clang \
> +        libpixman-1-dev \
> +        pkg-config \
> +        flex \
> +        bison \
> +        gettext \
> +        acpica-tools \
> +        bin86 \
> +        bcc \
> +        liblzma-dev \
> +        libc6-dev-i386 \
> +        libnl-3-dev \
> +        ocaml-nox \
> +        libfindlib-ocaml-dev \
> +        libsystemd-dev \
> +        markdown \

We dropped markdown as a dependency a release or two ago.

Both these dependences should be fine to drop, if we're happy to not
role Focal testing out to all the older branches.

> +        transfig \
> +        pandoc \
> +        checkpolicy \
> +        wget \

The build has absolutely no business reaching out into the internet.

I'm tempted to forcibly clobber it in the main build script.  (Perhaps
this is best not conflated with the Focal change.)

~Andrew

Re: [PATCH v1 4/4] tools/ocaml/libs/xc: backward compatible domid control at domain creation time

[Andrew Cooper]

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)
>  {

Re: [PATCH v1 4/4] tools/ocaml/libs/xc: backward compatible domid control at domain creation time

[Edwin Torok]

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)
> >  {
> 

Re: [PATCH v1 3/4] Makefile: add build-tools-oxenstored

[Wei Liu]

On Tue, Nov 17, 2020 at 06:24:11PM +0000, Edwin Török wrote:
> As a convenience so that oxenstored patches can be compile-tested
> using upstream's build-system before submitting upstream.
> 
> Signed-off-by: Edwin Török <edvin.torok@citrix.com>

Acked-by: Wei Liu <wl@xen.org>

Seeing that there is still pending comments from Andrew I won't commit
this series any time soon, despite Christian and Doug having acked this
series.

FAOD Andrew feel free to commit these patches once the comments are
addressed.

Wei.