xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Daniel De Graaf <dgdegra@tycho.nsa.gov>
To: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH 19/24] xenstored: support running in minios stubdom
Date: Fri, 27 Jan 2012 11:11:26 -0500	[thread overview]
Message-ID: <4F22CCAE.3000808@tycho.nsa.gov> (raw)
In-Reply-To: <alpine.DEB.2.00.1201271115030.3196@kaball-desktop>

On 01/27/2012 06:22 AM, Stefano Stabellini wrote:
> On Thu, 26 Jan 2012, Daniel De Graaf wrote:
>> A previous versions of this patch has been sent to xen-devel. See
>> http://lists.xensource.com/archives/html/xen-devel/2009-03/msg01655.html
>>
>> Signed-off-by: Diego Ongaro <diego.ongaro@citrix.com>
>> Signed-off-by: Alex Zeffertt <alex.zeffertt@eu.citrix.com>
>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> 
> The patch series is definitely going in the right direction.
> 
> 
>> +
>>  .PHONY: all
>>  all: $(ALL_TARGETS)
>>  
>> @@ -45,10 +49,13 @@ xenstored_probes.o: xenstored_solaris.o
>>  
>>  CFLAGS += -DHAVE_DTRACE=1
>>  endif
>> - 
>> +
>>  xenstored: $(XENSTORED_OBJS)
>>  	$(CC) $(LDFLAGS) $^ $(LDLIBS_libxenctrl) $(SOCKET_LIBS) -o $@ $(APPEND_LDFLAGS)
>>  
>> +xenstored.a: $(XENSTORED_OBJS)
>> +	$(AR) cr $@ $^
>> +
>>  $(CLIENTS): xenstore
>>  	ln -f xenstore $@
>>  
>> diff --git a/tools/xenstore/utils.h b/tools/xenstore/utils.h
>> index f378343..2effd17 100644
>> --- a/tools/xenstore/utils.h
>> +++ b/tools/xenstore/utils.h
>> @@ -19,7 +19,9 @@ static inline bool strends(const char *a, const char *b)
>>  	return streq(a + strlen(a) - strlen(b), b);
>>  }
>>  
>> +#ifndef ARRAY_SIZE
>>  #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
>> +#endif
>>  
>>  void barf(const char *fmt, ...) __attribute__((noreturn));
>>  void barf_perror(const char *fmt, ...) __attribute__((noreturn));
>> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
>> index 4b12cf2..0b9d4f2 100644
>> --- a/tools/xenstore/xenstored_core.c
>> +++ b/tools/xenstore/xenstored_core.c
>> @@ -224,7 +224,6 @@ static void reopen_log(void)
>>  	}
>>  }
>>  
>> -
>>  static bool write_messages(struct connection *conn)
>>  {
>>  	int ret;
>> @@ -327,7 +326,8 @@ static int initialize_set(fd_set *inset, fd_set *outset, int sock, int ro_sock,
>>  		set_fd(sock, inset, &max);
>>  	if (ro_sock != -1)
>>  		set_fd(ro_sock, inset, &max);
>> -	set_fd(reopen_log_pipe[0], inset, &max);
>> +	if (reopen_log_pipe[0] != -1)
>> +		set_fd(reopen_log_pipe[0], inset, &max);
>>  
>>  	if (xce_handle != NULL)
>>  		set_fd(xc_evtchn_fd(xce_handle), inset, &max);
>> @@ -1664,6 +1664,19 @@ static void corrupt(struct connection *conn, const char *fmt, ...)
>>  }
>>  
>>  
>> +#ifdef __MINIOS__
>> +static void write_pidfile(const char *pidfile)
>> +{
>> +}
>> +
>> +static void daemonize(void)
>> +{
>> +}
>> +
>> +static void finish_daemonize(void)
>> +{
>> +}
>> +#else
>>  static void write_pidfile(const char *pidfile)
>>  {
>>  	char buf[100];
>> @@ -1711,6 +1724,19 @@ static void daemonize(void)
>>  	umask(0);
>>  }
>>  
>> +static void finish_daemonize(void)
>> +{
>> +	int devnull = open("/dev/null", O_RDWR);
>> +	if (devnull == -1)
>> +		barf_perror("Could not open /dev/null\n");
>> +	dup2(devnull, STDIN_FILENO);
>> +	dup2(devnull, STDOUT_FILENO);
>> +	dup2(devnull, STDERR_FILENO);
>> +	close(devnull);
>> +	xprintf = trace;
>> +}
>> +#endif
>> +
>>  #ifdef NO_SOCKETS
>>  static void init_sockets(int **psock, int **pro_sock)
>>  {
> 
> At this point we could have the MiniOS version of write_pidfile,
> daemonize, finish_daemonize in tools/xenstore/xenstored_minios.c and the
> Linux/NetBSD version of them in tools/xenstore/xenstored_linux.c.
> 

Are you suggesting this for just these functions, or all functions that are
different on minios?

Since we already have xenstored_{linux,netbsd,solaris}.c, should the POSIX
versions be duplicated or placed in a common POSIX file (as Ian suggested)?

> 
>> ---
>>  tools/xenstore/Makefile           |    9 ++++-
>>  tools/xenstore/utils.h            |    2 +
>>  tools/xenstore/xenstored_core.c   |   74 +++++++++++++++++++++++-------------
>>  tools/xenstore/xenstored_domain.c |   11 +++++
>>  4 files changed, 68 insertions(+), 28 deletions(-)
>>
>> diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
>> index 4facb62..be892fd 100644
>> --- a/tools/xenstore/Makefile
>> +++ b/tools/xenstore/Makefile
>> @@ -28,6 +28,10 @@ endif
>>  
>>  ALL_TARGETS = libxenstore.so libxenstore.a clients xs_tdb_dump xenstored
>>  
>> +ifdef CONFIG_STUBDOM
>> +CFLAGS += -DNO_SOCKETS=1
>> +endif
> 
>> @@ -1822,6 +1848,11 @@ int main(int argc, char *argv[])
>>  	int evtchn_fd = -1;
>>  	struct timeval *timeout;
>>  
>> +#ifdef __MINIOS__
>> +	/* minios always uses internal DB */
>> +	tdb_flags = TDB_INTERNAL|TDB_NOLOCK;
>> +#endif
> 
> can you use the "internal-db" command line option?
> 

Yes, but that begins to clutter up the xenstore stub domain's command line
with mandatory options (which seems self-contradictory).

> 
>>  	while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RLVW:", options,
>>  				  NULL)) != -1) {
>>  		switch (opt) {
>> @@ -1874,20 +1905,10 @@ int main(int argc, char *argv[])
>>  
>>  	reopen_log();
>>  
>> -	/* make sure xenstored directory exists */
>> -	if (mkdir(xs_daemon_rundir(), 0755)) {
>> -		if (errno != EEXIST) {
>> -			perror("error: mkdir daemon rundir");
>> -			exit(-1);
>> -		}
>> -	}
>> -
>> -	if (mkdir(xs_daemon_rootdir(), 0755)) {
>> -		if (errno != EEXIST) {
>> -			perror("error: mkdir daemon rootdir");
>> -			exit(-1);
>> -		}
>> -	}
>> +	/* make sure xenstored directories exist */
>> +	/* Errors ignored here, will be reported when we open files */
>> +	mkdir(xs_daemon_rundir(), 0755);
>> +	mkdir(xs_daemon_rootdir(), 0755);
>>  
>>  	if (dofork) {
>>  		openlog("xenstored", 0, LOG_DAEMON);
>> @@ -1905,9 +1926,14 @@ int main(int argc, char *argv[])
>>  
>>  	init_sockets(&sock, &ro_sock);
>>  
>> +#ifdef __MINIOS__
>> +	reopen_log_pipe[0] = -1;
>> +	reopen_log_pipe[1] = -1;
>> +#else
>>  	if (pipe(reopen_log_pipe)) {
>>  		barf_perror("pipe");
>>  	}
>> +#endif
> 
> maybe we could have open/read/write_log_pipe functions?

That would be useless here, since the pipe is only used to receive signals
(which minios can't do) in order to reopen a log file that minios doesn't open.

> 
>>  	/* Setup the database */
>>  	setup_structure();
>> @@ -1925,16 +1951,8 @@ int main(int argc, char *argv[])
>>  	}
>>  
>>  	/* redirect to /dev/null now we're ready to accept connections */
>> -	if (dofork) {
>> -		int devnull = open("/dev/null", O_RDWR);
>> -		if (devnull == -1)
>> -			barf_perror("Could not open /dev/null\n");
>> -		dup2(devnull, STDIN_FILENO);
>> -		dup2(devnull, STDOUT_FILENO);
>> -		dup2(devnull, STDERR_FILENO);
>> -		close(devnull);
>> -		xprintf = trace;
>> -	}
>> +	if (dofork)
>> +		finish_daemonize();
>>  
>>  	signal(SIGHUP, trigger_reopen_log);
>>  
>> @@ -1944,8 +1962,10 @@ int main(int argc, char *argv[])
>>  	/* Get ready to listen to the tools. */
>>  	max = initialize_set(&inset, &outset, *sock, *ro_sock, &timeout);
>>  
>> +#ifndef __MINIOS__
>>  	/* Tell the kernel we're up and running. */
>>  	xenbus_notify_running();
>> +#endif
>>  
>>  	/* Main loop. */
>>  	for (;;) {
>> @@ -1957,7 +1977,7 @@ int main(int argc, char *argv[])
>>  			barf_perror("Select failed");
>>  		}
>>  
>> -		if (FD_ISSET(reopen_log_pipe[0], &inset)) {
>> +		if (reopen_log_pipe[0] != -1 && FD_ISSET(reopen_log_pipe[0], &inset)) {
>>  			char c;
>>  			if (read(reopen_log_pipe[0], &c, 1) != 1)
>>  				barf_perror("read failed");
>> diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
>> index c521e52..4243f91 100644
>> --- a/tools/xenstore/xenstored_domain.c
>> +++ b/tools/xenstore/xenstored_domain.c
>> @@ -197,12 +197,16 @@ static int destroy_domain(void *_domain)
>>  	}
>>  
>>  	if (domain->interface) {
>> +#ifdef __MINIOS__
>> +		unmap_interface(domain->interface);
>> +#else
>>  		/* Domain 0 was mapped by dom0_init, so it must be unmapped
>>  		   using munmap() and not the grant unmap call. */
>>  		if (domain->domid == 0)
>>  			munmap(domain->interface, getpagesize());
>>  		else
>>  			unmap_interface(domain->interface);
>> +#endif
>>  	}
>>  
>>  	fire_watches(NULL, "@releaseDomain", false);
>> @@ -597,6 +601,12 @@ void restore_existing_connections(void)
>>  {
>>  }
>>  
>> +#ifdef __MINIOS__
>> +static int dom0_init(void)
>> +{
>> +	return 0;
>> +}
>> +#else
>>  static int dom0_init(void) 
>>  { 
>>  	evtchn_port_t port;
>> @@ -620,6 +630,7 @@ static int dom0_init(void)
>>  
>>  	return 0; 
>>  }
>> +#endif
> 
> another candidate to be moved to xenstored_minios/linux
> 

  parent reply	other threads:[~2012-01-27 16:11 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-26 19:44 [PATCH v5 00/24] Xenstore stub domain Daniel De Graaf
2012-01-26 19:44 ` [PATCH 01/24] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall Daniel De Graaf
2012-01-26 19:44 ` [PATCH 02/24] xen: allow global VIRQ handlers to be delegated to other domains Daniel De Graaf
2012-01-26 19:44 ` [PATCH 03/24] xen: change virq parameters from int to uint32_t Daniel De Graaf
2012-01-26 19:44 ` [PATCH 04/24] xen: use XSM instead of IS_PRIV for getdomaininfo Daniel De Graaf
2012-01-26 19:44 ` [PATCH 05/24] xen: Preserve reserved grant entries when switching versions Daniel De Graaf
2012-01-27  9:54   ` Ian Campbell
2012-01-27 15:12     ` Daniel De Graaf
2012-01-26 19:44 ` [PATCH 06/24] tools/libxl: pull xenstore/console domids from xenstore Daniel De Graaf
2012-01-26 19:44 ` [PATCH 07/24] lib{xc, xl}: Seed grant tables with xenstore and console grants Daniel De Graaf
2012-01-27 10:00   ` Ian Campbell
2012-01-26 19:44 ` [PATCH 08/24] mini-os: avoid crash if no console is provided Daniel De Graaf
2012-01-26 19:44 ` [PATCH 09/24] mini-os: remove per-fd evtchn limit Daniel De Graaf
2012-01-26 19:44 ` [PATCH 10/24] mini-os: create app-specific configuration Daniel De Graaf
2012-01-27 10:04   ` Ian Campbell
2012-01-27 15:26     ` Daniel De Graaf
2012-01-26 19:44 ` [PATCH 11/24] mini-os: Move test functions into test.c Daniel De Graaf
2012-01-27 10:05   ` Ian Campbell
2012-01-26 19:44 ` [PATCH 12/24] mini-os: make frontends and xenbus optional Daniel De Graaf
2012-01-27 10:12   ` Ian Campbell
2012-01-26 19:45 ` [PATCH 13/24] mini-os: fix list.h include guard name Daniel De Graaf
2012-01-27 13:06   ` Ian Campbell
2012-01-27 13:09     ` Ian Campbell
2012-01-27 15:49     ` Daniel De Graaf
2012-01-27 15:53       ` Ian Campbell
2012-01-28 13:52     ` Keir Fraser
2012-01-31 16:38     ` Ian Jackson
2012-01-26 19:45 ` [PATCH 14/24] xenstored: use grant references instead of map_foreign_range Daniel De Graaf
2012-01-26 19:45 ` [PATCH 15/24] xenstored: refactor socket setup code Daniel De Graaf
2012-01-27 10:17   ` Ian Campbell
2012-01-26 19:45 ` [PATCH 16/24] xenstored: add NO_SOCKETS compilation option Daniel De Graaf
2012-01-27 10:20   ` Ian Campbell
2012-01-26 19:45 ` [PATCH 17/24] xenstored: support for tdb_copy with TDB_INTERNAL Daniel De Graaf
2012-01-26 19:45 ` [PATCH 18/24] xenstored: add --internal-db flag Daniel De Graaf
2012-01-27 10:32   ` Ian Campbell
2012-01-26 19:45 ` [PATCH 19/24] xenstored: support running in minios stubdom Daniel De Graaf
2012-01-27 10:34   ` Ian Campbell
2012-01-27 11:22   ` Stefano Stabellini
2012-01-27 11:49     ` Ian Campbell
2012-01-27 16:11     ` Daniel De Graaf [this message]
2012-01-27 16:40       ` Stefano Stabellini
2012-01-26 19:45 ` [PATCH 20/24] stubdom: enable xenstored build Daniel De Graaf
2012-01-27 10:35   ` Ian Campbell
2012-01-26 19:45 ` [PATCH 21/24] xenstored: add --event parameter for bootstrapping Daniel De Graaf
2012-01-26 19:45 ` [PATCH 22/24] xenstored: use domain_is_unprivileged instead of checking conn->id Daniel De Graaf
2012-01-27 10:37   ` Ian Campbell
2012-01-26 19:45 ` [PATCH 23/24] xenstored: add --priv-domid parameter Daniel De Graaf
2012-01-27 10:38   ` Ian Campbell
2012-01-26 19:45 ` [PATCH 24/24] xenstored: Add stub domain builder Daniel De Graaf
  -- strict thread matches above, loose matches on Subject: below --
2012-01-27 22:15 [PATCH v6 00/24] Xenstore stub domain Daniel De Graaf
2012-01-27 22:15 ` [PATCH 19/24] xenstored: support running in minios stubdom Daniel De Graaf
2012-01-30 11:08   ` Stefano Stabellini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F22CCAE.3000808@tycho.nsa.gov \
    --to=dgdegra@tycho.nsa.gov \
    --cc=ian.campbell@citrix.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).