Re: [Xen-devel] [PATCH 18/23] xenstored: support running in minios stubdom

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 25 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>
> ---
>  tools/xenstore/Makefile           |    9 ++++++++-
>  tools/xenstore/tdb.c              |    6 +++---
>  tools/xenstore/utils.h            |    2 ++
>  tools/xenstore/xenstored_core.c   |   29 ++++++++++++++++++++++++++---
>  tools/xenstore/xenstored_domain.c |   11 +++++++++++
>  5 files changed, 50 insertions(+), 7 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
> +
>  .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/tdb.c b/tools/xenstore/tdb.c
> index 3ecd3fc..cb66ea7 100644
> --- a/tools/xenstore/tdb.c
> +++ b/tools/xenstore/tdb.c
> @@ -1334,7 +1334,7 @@ static int tdb_next_lock(TDB_CONTEXT *tdb, struct 
> tdb_traverse_lock *tlock,
>  
>               /* Iterate through chain */
>               while( tlock->off) {
> -                     tdb_off current;
> +                     tdb_off mycurrent;
>                       if (rec_read(tdb, tlock->off, rec) == -1)
>                               goto fail;
>  
> @@ -1352,10 +1352,10 @@ static int tdb_next_lock(TDB_CONTEXT *tdb, struct 
> tdb_traverse_lock *tlock,
>                       }
>  
>                       /* Try to clean dead ones from old traverses */
> -                     current = tlock->off;
> +                     mycurrent = tlock->off;
>                       tlock->off = rec->next;
>                       if (!tdb->read_only && 
> -                         do_delete(tdb, current, rec) != 0)
> +                         do_delete(tdb, mycurrent, rec) != 0)
>                               goto fail;
>               }
>               tdb_unlock(tdb, tlock->hash, F_WRLCK);

Why are you renaming current to mycurrent?


> 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 c1ee932..7564edd 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -92,8 +92,10 @@ TDB_CONTEXT *tdb_context(struct connection *conn)
>  
>  bool replace_tdb(const char *newname, TDB_CONTEXT *newtdb)
>  {
> +#ifndef __MINIOS__
>       if (rename(newname, xs_daemon_tdb()) != 0)
>               return false;
> +#endif
>       tdb_close(tdb_ctx);
>       tdb_ctx = talloc_steal(talloc_autofree_context(), newtdb);
>       return true;

Are you doing this because the database is not on file anymore, correct?
Maybe there is some room for introducing this concept more generally
because it could be useful on Linux too and it would avoid this and
other ifdef's.


> @@ -223,7 +225,6 @@ static void reopen_log(void)
>       }
>  }
>  
> -
>  static bool write_messages(struct connection *conn)
>  {
>       int ret;
> @@ -326,7 +327,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);
> @@ -1410,7 +1412,11 @@ static void accept_connection(int sock, bool canwrite)
>  }
>  #endif
>  
> +#ifdef __MINIOS__
> +#define TDB_FLAGS TDB_INTERNAL|TDB_NOLOCK
> +#else
>  #define TDB_FLAGS 0
> +#endif

See above

>  /* We create initial nodes manually. */
>  static void manual_node(const char *name, const char *child)
> @@ -1435,7 +1441,11 @@ static void setup_structure(void)
>  {
>       char *tdbname;
>       tdbname = talloc_strdup(talloc_autofree_context(), xs_daemon_tdb());
> +#ifdef __MINIOS__
> +     tdb_ctx = NULL;
> +#else
>       tdb_ctx = tdb_open(tdbname, 0, TDB_FLAGS, O_RDWR, 0);
> +#endif

ditto

>       if (tdb_ctx) {
>               /* XXX When we make xenstored able to restart, this will have
> @@ -1661,6 +1671,7 @@ static void corrupt(struct connection *conn, const char 
> *fmt, ...)
>  }
>  
>  
> +#ifndef __MINIOS__
>  static void write_pidfile(const char *pidfile)
>  {
>       char buf[100];
> @@ -1707,6 +1718,7 @@ static void daemonize(void)
>       /* Discard our parent's old-fashioned umask prejudices. */
>       umask(0);
>  }
> +#endif
>  
>  #ifdef NO_SOCKETS
>  static void init_sockets(int **psock, int **pro_sock)
> @@ -1866,6 +1878,7 @@ int main(int argc, char *argv[])
>  
>       reopen_log();
>  
> +#ifndef __MINIOS__
>       /* make sure xenstored directory exists */
>       if (mkdir(xs_daemon_rundir(), 0755)) {
>               if (errno != EEXIST) {
> @@ -1887,6 +1900,7 @@ int main(int argc, char *argv[])
>       }
>       if (pidfile)
>               write_pidfile(pidfile);
> +#endif

Maybe we could just have this fail gracefully: after all even on Linux
if the socket interface is not available everything should be able to
run correctly.


>       /* Talloc leak reports go to stderr, which is closed if we fork. */
>       if (!dofork)
> @@ -1897,9 +1911,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
>  
>       /* Setup the database */
>       setup_structure();
> @@ -1916,6 +1935,7 @@ int main(int argc, char *argv[])
>               fflush(stdout);
>       }
>  
> +#ifndef __MINIOS__
>       /* redirect to /dev/null now we're ready to accept connections */
>       if (dofork) {
>               int devnull = open("/dev/null", O_RDWR);
> @@ -1927,6 +1947,7 @@ int main(int argc, char *argv[])
>               close(devnull);
>               xprintf = trace;
>       }
> +#endif

we could probably handle /dev/null in minios from sys.c


>       signal(SIGHUP, trigger_reopen_log);
>  
> @@ -1936,8 +1957,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 (;;) {
> @@ -1949,7 +1972,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 661d955..435f76a 100644
> --- a/tools/xenstore/xenstored_domain.c
> +++ b/tools/xenstore/xenstored_domain.c
> @@ -197,10 +197,14 @@ static int destroy_domain(void *_domain)
>       }
>  
>       if (domain->interface) {
> +#ifdef __MINIOS__
> +             unmap_interface(domain->interface);
> +#else
>               if (domain->domid == 0)
>                       munmap(domain->interface, getpagesize());
>               else
>                       unmap_interface(domain->interface);
> +#endif
>       }

Argh. 
Why dom0 cannot be handled inside unmap_interface?


>       fire_watches(NULL, "@releaseDomain", false);
> @@ -595,6 +599,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;
> @@ -618,6 +628,7 @@ static int dom0_init(void)
>  
>       return 0; 
>  }
> +#endif
>  
>  void domain_init(void)
>  {

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