Re: [Xen-devel] [PATCH 19/24] xenstored: support running in minios stubdom

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

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@xxxxxxxxxx>
>> Signed-off-by: Alex Zeffertt <alex.zeffertt@xxxxxxxxxxxxx>
>> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> 
> 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
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel