Re: [Xen-devel] [PATCH 2/3] systemd: add xen-init-dom0 service

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Tue, Oct 21, 2014 at 10:56:11AM +0100, Wei Liu wrote:
> Also prevent xenstored.service from writing Dom0 nodes The
> initialisation is now done with xen-init-dom0.
> 
> Please rerun autoconf after applying this patch.
> 
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>

I've tested this patch, and it works fine. It also looks good.

One would just need to adds "systemctl enable xen-init-dom0" in the
doc, if there is one.

I just have one comment below.

> ---
>  tools/configure.ac                                   |    1 +
>  tools/hotplug/Linux/systemd/Makefile                 |    1 +
>  tools/hotplug/Linux/systemd/xen-init-dom0.service.in |   14 ++++++++++++++
>  tools/hotplug/Linux/systemd/xenstored.service.in     |    2 --
>  4 files changed, 16 insertions(+), 2 deletions(-)
>  create mode 100644 tools/hotplug/Linux/systemd/xen-init-dom0.service.in
> 
> diff --git a/tools/configure.ac b/tools/configure.ac
> index f584798..62ccf47 100644
> --- a/tools/configure.ac
> +++ b/tools/configure.ac
> @@ -15,6 +15,7 @@ hotplug/Linux/init.d/xendomains
>  hotplug/Linux/systemd/proc-xen.mount
>  hotplug/Linux/systemd/var-lib-xenstored.mount
>  hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service
> +hotplug/Linux/systemd/xen-init-dom0.service
>  hotplug/Linux/systemd/xen-watchdog.service
>  hotplug/Linux/systemd/xenconsoled.service
>  hotplug/Linux/systemd/xendomains.service
> diff --git a/tools/hotplug/Linux/systemd/Makefile 
> b/tools/hotplug/Linux/systemd/Makefile
> index 0c40a73..51c10fe 100644
> --- a/tools/hotplug/Linux/systemd/Makefile
> +++ b/tools/hotplug/Linux/systemd/Makefile
> @@ -14,6 +14,7 @@ XEN_SYSTEMD_SERVICE += xenconsoled.service
>  XEN_SYSTEMD_SERVICE += xen-qemu-dom0-disk-backend.service
>  XEN_SYSTEMD_SERVICE += xendomains.service
>  XEN_SYSTEMD_SERVICE += xen-watchdog.service
> +XEN_SYSTEMD_SERVICE += xen-init-dom0.service
>  
>  ALL_XEN_SYSTEMD =    $(XEN_SYSTEMD_MODULES)  \
>                       $(XEN_SYSTEMD_MOUNT)    \
> diff --git a/tools/hotplug/Linux/systemd/xen-init-dom0.service.in 
> b/tools/hotplug/Linux/systemd/xen-init-dom0.service.in
> new file mode 100644
> index 0000000..e947760
> --- /dev/null
> +++ b/tools/hotplug/Linux/systemd/xen-init-dom0.service.in
> @@ -0,0 +1,14 @@
> +[Unit]
> +Description=xen-init-dom0, initialise Dom0 configuration (xenstore nodes, 
> JSON configuration stub)
> +Requires=xenstored.socket
> +After=xenstored.service

This two lines looks weird. I think they needs to be both the same.

Right now, I think systemd is ignoring the "After=" because
xenstored.service is not yet on the list of services to start. But it
will work because as soon as xen-init-dom0 is going to try to access
xenstored, systemd is going to start it (because .socket).

So, IMHO, the right thing to do is either:
Requires=xenstored.socket
After=xenstored.socket
or:
Requires=xenstored.service
After=xenstored.service

> +ConditionPathExists=/proc/xen/capabilities
> +
> +[Service]
> +Type=oneshot
> +RemainAfterExit=true
> +ExecStartPre=/bin/grep -q control_d /proc/xen/capabilities
> +ExecStart=@LIBEXEC_BIN@/xen-init-dom0
> +
> +[Install]
> +WantedBy=multi-user.target
> diff --git a/tools/hotplug/Linux/systemd/xenstored.service.in 
> b/tools/hotplug/Linux/systemd/xenstored.service.in
> index 013e69e..e3bf837 100644
> --- a/tools/hotplug/Linux/systemd/xenstored.service.in
> +++ b/tools/hotplug/Linux/systemd/xenstored.service.in
> @@ -17,8 +17,6 @@ ExecStartPre=/bin/grep -q control_d /proc/xen/capabilities
>  ExecStartPre=-/bin/rm -f @XEN_LIB_STORED@/tdb*
>  ExecStartPre=/bin/mkdir -p @XEN_RUN_DIR@
>  ExecStart=/bin/sh -c "exec $XENSTORED --no-fork $XENSTORED_ARGS"
> -ExecStartPost=-@bindir@/xenstore-write "/local/domain/0/name" "Domain-0"
> -ExecStartPost=-@bindir@/xenstore-write "/local/domain/0/domid" 0
>  
>  [Install]
>  WantedBy=multi-user.target
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel