* [PATCH v2] xenstore: create pidfile in init-xenstore-domain
@ 2013-04-24 16:44 Daniel De Graaf
2013-04-25 8:19 ` Christoph Egger
0 siblings, 1 reply; 6+ messages in thread
From: Daniel De Graaf @ 2013-04-24 16:44 UTC (permalink / raw)
To: Ian.Campbell
Cc: Andrew.Cooper3, Daniel De Graaf, steve_1991, Ian.Jackson,
xen-devel
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 | 12 +++++++++++-
2 files changed, 15 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..35f1aa3 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,14 @@ 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)
+ return 3;
+ rv = snprintf(buf, 16, "domid:%d\n", domid);
+ rv = write(fd, buf, rv);
+ close(fd);
+ if (rv < 0)
+ return 3;
+
return 0;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] xenstore: create pidfile in init-xenstore-domain
2013-04-24 16:44 [PATCH v2] xenstore: create pidfile in init-xenstore-domain Daniel De Graaf
@ 2013-04-25 8:19 ` Christoph Egger
2013-04-25 8:51 ` Ian Campbell
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Egger @ 2013-04-25 8:19 UTC (permalink / raw)
To: xen-devel
On 24.04.13 18:44, Daniel De Graaf wrote:
> 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 | 12 +++++++++++-
> 2 files changed, 15 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
> +
Please explain what is Linux-specific?
> 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..35f1aa3 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,14 @@ 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)
> + return 3;
> + rv = snprintf(buf, 16, "domid:%d\n", domid);
Use sizeof(buf). That's less error-prone whenever the size
of buf changes.
> + rv = write(fd, buf, rv);
> + close(fd);
> + if (rv < 0)
> + return 3;
> +
> return 0;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] xenstore: create pidfile in init-xenstore-domain
2013-04-25 8:19 ` Christoph Egger
@ 2013-04-25 8:51 ` Ian Campbell
2013-04-25 9:05 ` Egger, Christoph
0 siblings, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2013-04-25 8:51 UTC (permalink / raw)
To: Christoph Egger; +Cc: xen-devel@lists.xen.org
On Thu, 2013-04-25 at 09:19 +0100, Christoph Egger wrote:
> @@ -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
> > +
>
> Please explain what is Linux-specific?
Can you confirm that this file at least builds as is on NetBSD? Was it
built there previously?
Ian.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] xenstore: create pidfile in init-xenstore-domain
2013-04-25 8:51 ` Ian Campbell
@ 2013-04-25 9:05 ` Egger, Christoph
2013-04-25 9:56 ` Ian Campbell
0 siblings, 1 reply; 6+ messages in thread
From: Egger, Christoph @ 2013-04-25 9:05 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel@lists.xen.org
On 25.04.13 10:51, Ian Campbell wrote:
> On Thu, 2013-04-25 at 09:19 +0100, Christoph Egger wrote:
>> @@ -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
>>> +
>>
>> Please explain what is Linux-specific?
>
> Can you confirm that this file at least builds as is on NetBSD? Was it
> built there previously?
No. It was only enabled for stubdom.
Building it manually says:
<xen/sys/xenbus_dev.h>: No such file or directory.
Christoph
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] xenstore: create pidfile in init-xenstore-domain
2013-04-25 9:05 ` Egger, Christoph
@ 2013-04-25 9:56 ` Ian Campbell
2013-05-01 12:34 ` Ian Campbell
0 siblings, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2013-04-25 9:56 UTC (permalink / raw)
To: Egger, Christoph; +Cc: xen-devel@lists.xen.org
On Thu, 2013-04-25 at 10:05 +0100, Egger, Christoph wrote:
> On 25.04.13 10:51, Ian Campbell wrote:
> > On Thu, 2013-04-25 at 09:19 +0100, Christoph Egger wrote:
> >> @@ -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
> >>> +
> >>
> >> Please explain what is Linux-specific?
> >
> > Can you confirm that this file at least builds as is on NetBSD? Was it
> > built there previously?
>
> No. It was only enabled for stubdom.
> Building it manually says:
> <xen/sys/xenbus_dev.h>: No such file or directory.
OK, so this should be Linux only until that interface is implemented for
BSD (should probably be in libxc in any case)
Ian/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] xenstore: create pidfile in init-xenstore-domain
2013-04-25 9:56 ` Ian Campbell
@ 2013-05-01 12:34 ` Ian Campbell
0 siblings, 0 replies; 6+ messages in thread
From: Ian Campbell @ 2013-05-01 12:34 UTC (permalink / raw)
To: Egger, Christoph
Cc: Andrew Cooper, Daniel De Graaf, steve_1991@hushmail.com,
Ian Jackson, xen-devel@lists.xen.org
Putting back the original CC list which Christoph dropped a few mails
back. Please be careful of this.
On Thu, 2013-04-25 at 10:56 +0100, Ian Campbell wrote:
> On Thu, 2013-04-25 at 10:05 +0100, Egger, Christoph wrote:
> > On 25.04.13 10:51, Ian Campbell wrote:
> > > On Thu, 2013-04-25 at 09:19 +0100, Christoph Egger wrote:
> > >> @@ -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
> > >>> +
> > >>
> > >> Please explain what is Linux-specific?
> > >
> > > Can you confirm that this file at least builds as is on NetBSD? Was it
> > > built there previously?
> >
> > No. It was only enabled for stubdom.
> > Building it manually says:
> > <xen/sys/xenbus_dev.h>: No such file or directory.
>
> OK, so this should be Linux only until that interface is implemented for
> BSD (should probably be in libxc in any case)
With this in mind I have acked + applied this. Christoph's comment about
using sizeof(buf) is a good one but not worth delaying this patch for.
Ian.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-01 12:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-24 16:44 [PATCH v2] xenstore: create pidfile in init-xenstore-domain Daniel De Graaf
2013-04-25 8:19 ` Christoph Egger
2013-04-25 8:51 ` Ian Campbell
2013-04-25 9:05 ` Egger, Christoph
2013-04-25 9:56 ` Ian Campbell
2013-05-01 12:34 ` Ian Campbell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.