Daniel De Graaf
2013-Apr-22 17:06 UTC
[PATCH] xenstore: create pidfile in init-xenstore-domain
Since libxl checks for the existance of /var/run/xenstored.pid in order
to ensure xenstore is running, create this file when starting the
xenstore stub domain. This also changes the Makefile to enable the
creation of the init-xenstore-domain tool during tools compilation,
since the existing Makefile incorrectly added to the ALL_TARGETS list
when compiling the stubdom, when this variable is not used.
Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
tools/xenstore/Makefile | 5 ++++-
tools/xenstore/init-xenstore-domain.c | 10 +++++++++-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
index 9172d3a..1bb6e58 100644
--- a/tools/xenstore/Makefile
+++ b/tools/xenstore/Makefile
@@ -29,9 +29,12 @@ endif
ALL_TARGETS = libxenstore.so libxenstore.a clients xs_tdb_dump xenstored
+ifeq ($(CONFIG_Linux),y)
+ALL_TARGETS += init-xenstore-domain
+endif
+
ifdef CONFIG_STUBDOM
CFLAGS += -DNO_SOCKETS=1
-ALL_TARGETS += init-xenstore-domain
endif
.PHONY: all
diff --git a/tools/xenstore/init-xenstore-domain.c
b/tools/xenstore/init-xenstore-domain.c
index 18c075b..57585f4 100644
--- a/tools/xenstore/init-xenstore-domain.c
+++ b/tools/xenstore/init-xenstore-domain.c
@@ -1,4 +1,5 @@
#include <fcntl.h>
+#include <unistd.h>
#include <stdio.h>
#include <string.h>
#include <stdint.h>
@@ -69,7 +70,7 @@ int main(int argc, char** argv)
xc_interface *xch;
struct xs_handle *xsh;
char buf[16];
- int rv;
+ int rv, fd;
if (argc != 4) {
printf("Use: %s <xenstore-kernel> <memory_mb>
<flask-label>\n", argv[0]);
@@ -90,5 +91,12 @@ int main(int argc, char** argv)
xs_write(xsh, XBT_NULL, "/tool/xenstored/domid", buf, rv);
xs_daemon_close(xsh);
+ fd = creat("/var/run/xenstored.pid", 0666);
+ if (fd >= 0) {
+ rv = snprintf(buf, 16, "domid:%d", domid);
+ write(fd, buf, rv);
+ close(fd);
+ }
+
return 0;
}
--
1.8.1.4
Ian Campbell
2013-Apr-24 11:24 UTC
Re: [PATCH] xenstore: create pidfile in init-xenstore-domain
On Mon, 2013-04-22 at 18:06 +0100, Daniel De Graaf wrote:> @@ -90,5 +91,12 @@ int main(int argc, char** argv) > xs_write(xsh, XBT_NULL, "/tool/xenstored/domid", buf, rv); > xs_daemon_close(xsh); > > + fd = creat("/var/run/xenstored.pid", 0666);This is world writeable which doesn''t seem like a good idea. Most of the pidfiles on my system seem to use 0644. Ian.
Daniel De Graaf
2013-Apr-24 13:05 UTC
Re: [PATCH] xenstore: create pidfile in init-xenstore-domain
On 04/24/2013 07:24 AM, Ian Campbell wrote:> On Mon, 2013-04-22 at 18:06 +0100, Daniel De Graaf wrote: > >> @@ -90,5 +91,12 @@ int main(int argc, char** argv) >> xs_write(xsh, XBT_NULL, "/tool/xenstored/domid", buf, rv); >> xs_daemon_close(xsh); >> >> + fd = creat("/var/run/xenstored.pid", 0666); > > This is world writeable which doesn''t seem like a good idea. > > Most of the pidfiles on my system seem to use 0644. > > Ian. >The mode specified here is ANDed with your ~umask, which is usually something like 0022. If you prefer an explicit 0644, that''s easy to change. I also noticed that the file contents are missing a trailing newline after submission. -- Daniel De Graaf National Security Agency
Ian Campbell
2013-Apr-24 13:36 UTC
Re: [PATCH] xenstore: create pidfile in init-xenstore-domain
On Wed, 2013-04-24 at 14:05 +0100, Daniel De Graaf wrote:> On 04/24/2013 07:24 AM, Ian Campbell wrote: > > On Mon, 2013-04-22 at 18:06 +0100, Daniel De Graaf wrote: > > > >> @@ -90,5 +91,12 @@ int main(int argc, char** argv) > >> xs_write(xsh, XBT_NULL, "/tool/xenstored/domid", buf, rv); > >> xs_daemon_close(xsh); > >> > >> + fd = creat("/var/run/xenstored.pid", 0666); > > > > This is world writeable which doesn''t seem like a good idea. > > > > Most of the pidfiles on my system seem to use 0644. > > > > Ian. > > > > The mode specified here is ANDed with your ~umask, which is usually > something like 0022. If you prefer an explicit 0644, that''s easy to > change.I''ve no idea what umask typically looks like when running an initscript from /sbin/init though. But on Debian: $ grep umask /etc/init.d/rc* /etc/init.d/rc:umask 022 OK, I think I''m sold on 0666, thanks!> I also noticed that the file contents are missing a trailing > newline after submission.Shall I just add \n as I commit? Ian.
Daniel De Graaf
2013-Apr-24 13:45 UTC
Re: [PATCH] xenstore: create pidfile in init-xenstore-domain
On 04/24/2013 09:36 AM, Ian Campbell wrote:> On Wed, 2013-04-24 at 14:05 +0100, Daniel De Graaf wrote: >> On 04/24/2013 07:24 AM, Ian Campbell wrote: >>> On Mon, 2013-04-22 at 18:06 +0100, Daniel De Graaf wrote: >>> >>>> @@ -90,5 +91,12 @@ int main(int argc, char** argv) >>>> xs_write(xsh, XBT_NULL, "/tool/xenstored/domid", buf, rv); >>>> xs_daemon_close(xsh); >>>> >>>> + fd = creat("/var/run/xenstored.pid", 0666); >>> >>> This is world writeable which doesn''t seem like a good idea. >>> >>> Most of the pidfiles on my system seem to use 0644. >>> >>> Ian. >>> >> >> The mode specified here is ANDed with your ~umask, which is usually >> something like 0022. If you prefer an explicit 0644, that''s easy to >> change. > > I''ve no idea what umask typically looks like when running an initscript > from /sbin/init though. But on Debian: > $ grep umask /etc/init.d/rc* > /etc/init.d/rc:umask 022 > > OK, I think I''m sold on 0666, thanks! > >> I also noticed that the file contents are missing a trailing >> newline after submission. > > Shall I just add \n as I commit? > > Ian.Yes, I don''t think a patch v2 just for that change is needed. -- Daniel De Graaf National Security Agency
Ian Campbell
2013-Apr-24 15:55 UTC
Re: [PATCH] xenstore: create pidfile in init-xenstore-domain
On Wed, 2013-04-24 at 14:45 +0100, Daniel De Graaf wrote:> On 04/24/2013 09:36 AM, Ian Campbell wrote: > > On Wed, 2013-04-24 at 14:05 +0100, Daniel De Graaf wrote: > >> On 04/24/2013 07:24 AM, Ian Campbell wrote: > >>> On Mon, 2013-04-22 at 18:06 +0100, Daniel De Graaf wrote: > >>> > >>>> @@ -90,5 +91,12 @@ int main(int argc, char** argv) > >>>> xs_write(xsh, XBT_NULL, "/tool/xenstored/domid", buf, rv); > >>>> xs_daemon_close(xsh); > >>>> > >>>> + fd = creat("/var/run/xenstored.pid", 0666); > >>> > >>> This is world writeable which doesn''t seem like a good idea. > >>> > >>> Most of the pidfiles on my system seem to use 0644. > >>> > >>> Ian. > >>> > >> > >> The mode specified here is ANDed with your ~umask, which is usually > >> something like 0022. If you prefer an explicit 0644, that''s easy to > >> change. > > > > I''ve no idea what umask typically looks like when running an initscript > > from /sbin/init though. But on Debian: > > $ grep umask /etc/init.d/rc* > > /etc/init.d/rc:umask 022 > > > > OK, I think I''m sold on 0666, thanks! > > > >> I also noticed that the file contents are missing a trailing > >> newline after submission. > > > > Shall I just add \n as I commit? > > > > Ian. > > Yes, I don''t think a patch v2 just for that change is needed.I tried to apply but: init-xenstore-domain.c: In function ''main'': init-xenstore-domain.c:97:8: error: ignoring return value of ''write'', declared with attribute warn_unused_result [-Werror=unused-result] cc1: all warnings being treated as errors make[3]: *** [init-xenstore-domain.o] Error 1 make[3]: *** Waiting for unfinished jobs.... Guess my gcc is more picky than yours :-/ Ian.>