| Message ID | 20230425194622.114869-1-jandryuk@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix install.sh for systemd | expand |
Tue, 25 Apr 2023 15:46:20 -0400 Jason Andryuk <jandryuk@gmail.com>:
> Skip populating /var/run/xen when systemd is being used.
It was wrong to do it like that from day one.
Such directory has to be populated on demand at runtime.
Olaf
On 26.04.2023 09:15, Olaf Hering wrote: > Tue, 25 Apr 2023 15:46:20 -0400 Jason Andryuk <jandryuk@gmail.com>: > >> Skip populating /var/run/xen when systemd is being used. > > It was wrong to do it like that from day one. > Such directory has to be populated on demand at runtime. Is this to be translated to Reviewed-by: <you>? Jan
Wed, 26 Apr 2023 10:28:38 +0200 Jan Beulich <jbeulich@suse.com>:
> Is this to be translated to Reviewed-by: <you>?
It was a Nack, in case such thing exists, and if it has any meaning if sent by me.
There are a few places already which do a mkdir -p XEN_RUN_DIR prior usage.
But a few are missing.
XEN_RUN_DIR and most likely also XEN_RUN_STORED have to be removed from make install.
Olaf
On 26.04.2023 10:47, Olaf Hering wrote: > Wed, 26 Apr 2023 10:28:38 +0200 Jan Beulich <jbeulich@suse.com>: > >> Is this to be translated to Reviewed-by: <you>? > > It was a Nack, in case such thing exists, and if it has any meaning if sent by me. Now I'm confused: Your reply didn't read like a nack at all, at least to me. Furthermore ... > There are a few places already which do a mkdir -p XEN_RUN_DIR prior usage. > But a few are missing. > XEN_RUN_DIR and most likely also XEN_RUN_STORED have to be removed from make install. ... this suggests to me that you really mean the change doesn't go far enough, but that's then different from nack-ing a change. Can you please clarify this for me (and maybe also for Jason, depending on how he has read your replies)? Jan
Wed, 26 Apr 2023 11:07:17 +0200 Jan Beulich <jbeulich@suse.com>: > On 26.04.2023 10:47, Olaf Hering wrote: > > XEN_RUN_DIR and most likely also XEN_RUN_STORED have to be removed from make install. > ... this suggests to me that you really mean the change doesn't go far > enough, but that's then different from nack-ing a change. Can you please > clarify this for me (and maybe also for Jason, depending on how he has > read your replies)? I think the change should look like this, the runtime directories have to be created at runtime. tools/Makefile | 2 -- tools/hotplug/FreeBSD/rc.d/xencommons.in | 1 + tools/hotplug/FreeBSD/rc.d/xendriverdomain.in | 1 + tools/hotplug/Linux/init.d/xendriverdomain.in | 1 + tools/hotplug/Linux/systemd/xenconsoled.service.in | 2 +- tools/hotplug/NetBSD/rc.d/xendriverdomain.in | 2 +- --- a/tools/Makefile +++ b/tools/Makefile @@ -58,9 +58,7 @@ build all: subdirs-all install: $(INSTALL_DIR) -m 700 $(DESTDIR)$(XEN_DUMP_DIR) $(INSTALL_DIR) $(DESTDIR)$(XEN_LOG_DIR) - $(INSTALL_DIR) $(DESTDIR)$(XEN_RUN_DIR) $(INSTALL_DIR) $(DESTDIR)$(XEN_LIB_DIR) - $(INSTALL_DIR) $(DESTDIR)$(XEN_RUN_STORED) $(INSTALL_DIR) $(DESTDIR)$(PKG_INSTALLDIR) $(MAKE) subdirs-install --- a/tools/hotplug/FreeBSD/rc.d/xencommons.in +++ b/tools/hotplug/FreeBSD/rc.d/xencommons.in @@ -34,6 +34,7 @@ xen_startcmd() local time=0 local timeout=30 + mkdir -p "@XEN_RUN_DIR@" xenstored_pid=$(check_pidfile ${XENSTORED_PIDFILE} ${XENSTORED}) if test -z "$xenstored_pid"; then printf "Starting xenservices: xenstored, xenconsoled." --- a/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in +++ b/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in @@ -27,6 +27,7 @@ xendriverdomain_start() { printf "Starting xenservices: xl devd." + mkdir -p "@XEN_RUN_DIR@" PATH="${bindir}:${sbindir}:$PATH" ${sbindir}/xl devd --pidfile ${XLDEVD_PIDFILE} ${XLDEVD_ARGS} printf "\n" --- a/tools/hotplug/Linux/init.d/xendriverdomain.in +++ b/tools/hotplug/Linux/init.d/xendriverdomain.in @@ -49,6 +49,7 @@ fi do_start () { echo Starting xl devd... + mkdir -m700 -p ${XEN_RUN_DIR} ${sbindir}/xl devd --pidfile=$XLDEVD_PIDFILE $XLDEVD_ARGS } do_stop () { --- a/tools/hotplug/Linux/systemd/xenconsoled.service.in +++ b/tools/hotplug/Linux/systemd/xenconsoled.service.in @@ -11,7 +11,7 @@ Environment=XENCONSOLED_TRACE=none Environment=XENCONSOLED_LOG_DIR=@XEN_LOG_DIR@/console EnvironmentFile=-@CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons ExecStartPre=/bin/grep -q control_d /proc/xen/capabilities -ExecStartPre=/bin/mkdir -p ${XENCONSOLED_LOG_DIR} +ExecStartPre=/bin/mkdir -p ${XENCONSOLED_LOG_DIR} @XEN_RUN_DIR@ ExecStart=@sbindir@/xenconsoled -i --log=${XENCONSOLED_TRACE} --log-dir=${XENCONSOLED_LOG_DIR} $XENCONSOLED_ARGS [Install] --- a/tools/hotplug/NetBSD/rc.d/xendriverdomain.in +++ b/tools/hotplug/NetBSD/rc.d/xendriverdomain.in @@ -23,7 +23,7 @@ XLDEVD_PIDFILE="@XEN_RUN_DIR@/xldevd.pid" xendriverdomain_precmd() { - : + mkdir -p "@XEN_RUN_DIR@" } xendriverdomain_startcmd() Olaf
On Wed, Apr 26, 2023 at 6:40 AM Olaf Hering <olaf@aepfle.de> wrote: > > Wed, 26 Apr 2023 11:07:17 +0200 Jan Beulich <jbeulich@suse.com>: > > > On 26.04.2023 10:47, Olaf Hering wrote: > > > XEN_RUN_DIR and most likely also XEN_RUN_STORED have to be removed from make install. > > ... this suggests to me that you really mean the change doesn't go far > > enough, but that's then different from nack-ing a change. Can you please > > clarify this for me (and maybe also for Jason, depending on how he has > > read your replies)? > > I think the change should look like this, the runtime directories have to be created at runtime. Thanks, Olaf. Yes, I think your approach is better. Will you submit it as a formal patch? I'm happy to test it. > --- a/tools/hotplug/Linux/init.d/xendriverdomain.in > +++ b/tools/hotplug/Linux/init.d/xendriverdomain.in > @@ -49,6 +49,7 @@ fi > > do_start () { > echo Starting xl devd... > + mkdir -m700 -p ${XEN_RUN_DIR} This one should be "@XEN_RUN_DIR@"? Thanks, Jason
Wed, 26 Apr 2023 09:31:44 -0400 Jason Andryuk <jandryuk@gmail.com>: > On Wed, Apr 26, 2023 at 6:40 AM Olaf Hering <olaf@aepfle.de> wrote: > > +++ b/tools/hotplug/Linux/init.d/xendriverdomain.in > > @@ -49,6 +49,7 @@ fi > > > > do_start () { > > echo Starting xl devd... > > + mkdir -m700 -p ${XEN_RUN_DIR} > This one should be "@XEN_RUN_DIR@"? I changed it, it was a result of copy&paste from another file. Since hotplug.sh is included in many scripts, all such users could be converted to a shell variable. But this would be a separate change. Olaf
diff --git a/tools/Makefile b/tools/Makefile index 4906fdbc23..32c8b0a2a2 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -58,9 +58,11 @@ build all: subdirs-all install: $(INSTALL_DIR) -m 700 $(DESTDIR)$(XEN_DUMP_DIR) $(INSTALL_DIR) $(DESTDIR)$(XEN_LOG_DIR) - $(INSTALL_DIR) $(DESTDIR)$(XEN_RUN_DIR) $(INSTALL_DIR) $(DESTDIR)$(XEN_LIB_DIR) +ifneq ($(CONFIG_SYSTEMD),y) + $(INSTALL_DIR) $(DESTDIR)$(XEN_RUN_DIR) $(INSTALL_DIR) $(DESTDIR)$(XEN_RUN_STORED) +endif $(INSTALL_DIR) $(DESTDIR)$(PKG_INSTALLDIR) $(MAKE) subdirs-install
On a fedora system, if you run `sudo sh install.sh` you break your system. The installation clobbers /var/run, a symlink to /run. A subsequent boot fails when /var/run and /run are different since accesses through /var/run can't find items that now only exist in /run and vice-versa. Skip populating /var/run/xen when systemd is being used. systemd expects an empty /run on boot and works properly without one. Signed-off-by: Jason Andryuk <jandryuk@gmail.com> --- tools/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)