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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2012-01-12 at 23:35 +0000, 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
> 
> Originally-by: Diego Ongaro <diego.ongaro@xxxxxxxxxx>
> Originally-by: Alex Zeffertt <alex.zeffertt@xxxxxxxxxxxxx>
> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>

There's a lot of ifdefery here. I don't have any particularly cunning
plans to improve it though :-(

One thing which might help is to provide nop versions of functions
instead of idef'ing both the definition and callsite. e.g. 
 static void write_pidfile(const char *pidfile)
+#ifndef __MINIOS__
     stuff
+#else
+    nothing
+endif

then you avoid another ifdef at the place which calls write_pidfile. I'm
not sure that pattern helps with many of the instances here though.

> ---
>  extras/mini-os/include/list.h          |    6 ++--
>  extras/mini-os/main.c                  |    4 +++
>  stubdom/Makefile                       |   29 +++++++++++++++++--
>  tools/xenstore/Makefile                |    9 +++++-
>  tools/xenstore/tdb.c                   |    6 ++--
>  tools/xenstore/utils.h                 |    2 +
>  tools/xenstore/xenstored_core.c        |   47 ++++++++++++++++++++++++++++++-
>  tools/xenstore/xenstored_domain.c      |    7 +++++
>  tools/xenstore/xenstored_transaction.c |    2 +
>  9 files changed, 100 insertions(+), 12 deletions(-)
> 
> diff --git a/extras/mini-os/include/list.h b/extras/mini-os/include/list.h
> index a60ae23..4e6a2ac 100644
> --- a/extras/mini-os/include/list.h
> +++ b/extras/mini-os/include/list.h
> @@ -1,5 +1,5 @@
> -#ifndef _LINUX_LIST_H
> -#define _LINUX_LIST_H
> +#ifndef _MINIOS_LIST_H
> +#define _MINIOS_LIST_H
> 
>  /*
>   * Simple doubly linked list implementation.
> @@ -186,5 +186,5 @@ static __inline__ void minios_list_splice(struct 
> minios_list_head *list, struct
>                 n = minios_list_entry(pos->member.next, typeof(*pos), 
> member);  \
>              &pos->member != (head);                                    \
>              pos = n, n = minios_list_entry(n->member.next, typeof(*n), 
> member))
> -#endif /* _LINUX_LIST_H */
> +#endif /* _MINIOS_LIST_H */
> 
> diff --git a/extras/mini-os/main.c b/extras/mini-os/main.c
> index b95b889..cd89849 100644
> --- a/extras/mini-os/main.c
> +++ b/extras/mini-os/main.c
> @@ -60,6 +60,7 @@ static void call_main(void *p)
>       * crashing. */
>      //sleep(1);
> 
> +#ifndef CONFIG_XENSTORE
>  #ifndef CONFIG_GRUB
>      sparse((unsigned long) &__app_bss_start, &__app_bss_end - 
> &__app_bss_start);
>  #if defined(HAVE_LWIP) && !defined(CONFIG_QEMU)
> @@ -67,6 +68,7 @@ static void call_main(void *p)
>  #endif
>  #endif
>      create_thread("pcifront", pcifront_watches, NULL);
> +#endif
> 
>  #ifdef CONFIG_QEMU
>      /* Fetch argc, argv from XenStore */
> @@ -169,9 +171,11 @@ void _exit(int ret)
>      close_all_files();
>      __libc_fini_array();
>      printk("main returned %d\n", ret);
> +#ifndef CONFIG_XENSTORE
>  #ifdef HAVE_LWIP
>      stop_networking();
>  #endif
> +#endif
>      stop_kernel();
>      if (!ret) {
>         /* No problem, just shutdown.  */
> diff --git a/stubdom/Makefile b/stubdom/Makefile
> index 3705059..e0a90a9 100644
> --- a/stubdom/Makefile
> +++ b/stubdom/Makefile
> @@ -74,14 +74,14 @@ TARGET_CPPFLAGS += -I$(XEN_ROOT)/xen/include
> 
>  TARGET_LDFLAGS += -nostdlib -L$(CROSS_PREFIX)/$(GNU_TARGET_ARCH)-xen-elf/lib
> 
> -TARGETS=ioemu c caml grub
> +TARGETS=ioemu c caml grub xenstore
> 
>  CROSS_MAKE := $(MAKE) DESTDIR=
> 
>  .PHONY: all
>  all: build
>  ifeq ($(STUBDOM_SUPPORTED),1)
> -build: genpath ioemu-stubdom c-stubdom pv-grub
> +build: genpath ioemu-stubdom c-stubdom pv-grub xenstore-stubdom
>  else
>  build: genpath
>  endif
> @@ -262,6 +262,11 @@ mk-headers-$(XEN_TARGET_ARCH): ioemu/linkfarm.stamp
>           ln -sf $(XEN_ROOT)/tools/libxc/$(XEN_TARGET_ARCH)/*.c . && \
>           ln -sf $(XEN_ROOT)/tools/libxc/$(XEN_TARGET_ARCH)/*.h . && \
>           ln -sf $(XEN_ROOT)/tools/libxc/$(XEN_TARGET_ARCH)/Makefile . )
> +       mkdir -p xenstore
> +       [ -h xenstore/Makefile ] || ( cd xenstore && \
> +         ln -sf $(XEN_ROOT)/tools/xenstore/*.c . && \
> +         ln -sf $(XEN_ROOT)/tools/xenstore/*.h . && \
> +         ln -sf $(XEN_ROOT)/tools/xenstore/Makefile . )
>         $(CROSS_MAKE) -C $(MINI_OS) links
>         touch mk-headers-$(XEN_TARGET_ARCH)
> 
> @@ -334,6 +339,14 @@ grub: grub-upstream $(CROSS_ROOT)
>         mkdir -p grub-$(XEN_TARGET_ARCH)
>         CPPFLAGS="$(TARGET_CPPFLAGS)" CFLAGS="$(TARGET_CFLAGS)" $(CROSS_MAKE) 
> -C $@ OBJ_DIR=$(CURDIR)/grub-$(XEN_TARGET_ARCH)
> 
> +##########
> +# xenstore
> +##########
> +
> +.PHONY: xenstore
> +xenstore: $(CROSS_ROOT)
> +       CPPFLAGS="$(TARGET_CPPFLAGS)" CFLAGS="$(TARGET_CFLAGS)" $(CROSS_MAKE) 
> -C $@ LWIPDIR=$(CURDIR)/lwip xenstored.a CONFIG_STUBDOM=y
> +
>  ########
>  # minios
>  ########
> @@ -355,12 +368,16 @@ c-stubdom: mini-os-$(XEN_TARGET_ARCH)-c 
> lwip-$(XEN_TARGET_ARCH) libxc c
>  pv-grub: mini-os-$(XEN_TARGET_ARCH)-grub libxc grub
>         DEF_CPPFLAGS="$(TARGET_CPPFLAGS)" DEF_CFLAGS="-DCONFIG_GRUB 
> $(TARGET_CFLAGS)" DEF_LDFLAGS="$(TARGET_LDFLAGS)" $(CROSS_MAKE) -C $(MINI_OS) 
> OBJ_DIR=$(CURDIR)/$< APP_OBJS=$(CURDIR)/grub-$(XEN_TARGET_ARCH)/main.a
> 
> +.PHONY: xenstore-stubdom
> +xenstore-stubdom: mini-os-$(XEN_TARGET_ARCH)-xenstore libxc xenstore
> +       DEF_CPPFLAGS="$(TARGET_CPPFLAGS)" DEF_CFLAGS="-DCONFIG_XENSTORE 
> $(TARGET_CFLAGS)" DEF_LDFLAGS="$(TARGET_LDFLAGS)" $(MAKE) -C $(MINI_OS) 
> OBJ_DIR=$(CURDIR)/$< LWIPDIR=$(CURDIR)/lwip-$(XEN_TARGET_ARCH) 
> APP_OBJS=$(CURDIR)/xenstore/xenstored.a
> +
>  #########
>  # install
>  #########
> 
>  ifeq ($(STUBDOM_SUPPORTED),1)
> -install: genpath install-readme install-ioemu install-grub
> +install: genpath install-readme install-ioemu install-grub install-xenstore
>  else
>  install: genpath
>  endif
> @@ -379,6 +396,10 @@ install-grub: pv-grub
>         $(INSTALL_DIR) "$(DESTDIR)$(XENFIRMWAREDIR)"
>         $(INSTALL_DATA) mini-os-$(XEN_TARGET_ARCH)-grub/mini-os.gz 
> "$(DESTDIR)$(XENFIRMWAREDIR)/pv-grub-$(XEN_TARGET_ARCH).gz"
> 
> +install-xenstore: xenstore-stubdom
> +       $(INSTALL_DIR) "$(DESTDIR)/usr/lib/xen/boot"
> +       $(INSTALL_PROG) mini-os-$(XEN_TARGET_ARCH)-xenstore/mini-os.gz 
> "$(DESTDIR)/usr/lib/xen/boot/xenstore-stubdom.gz"
> +
>  #######
>  # clean
>  #######
> @@ -390,12 +411,14 @@ clean:
>         rm -fr mini-os-$(XEN_TARGET_ARCH)-c
>         rm -fr mini-os-$(XEN_TARGET_ARCH)-caml
>         rm -fr mini-os-$(XEN_TARGET_ARCH)-grub
> +       rm -fr mini-os-$(XEN_TARGET_ARCH)-xenstore
>         $(CROSS_MAKE) -C caml clean
>         $(CROSS_MAKE) -C c clean
>         rm -fr grub-$(XEN_TARGET_ARCH)
>         rm -f $(STUBDOMPATH)
>         [ ! -d libxc-$(XEN_TARGET_ARCH) ] || $(CROSS_MAKE) -C 
> libxc-$(XEN_TARGET_ARCH) clean
>         -[ ! -d ioemu ] || $(CROSS_MAKE) -C ioemu clean
> +       -[ ! -d xenstore ] || $(CROSS_MAKE) -C xenstore clean
> 
>  # clean the cross-compilation result
>  .PHONY: crossclean
> diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
> index 4facb62..3a061d6 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 -DNO_LOCAL_XENBUS=1 -DNO_SYSLOG=1 -DNO_REOPEN_LOG=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 639ce6e..75ffd2a 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);
> 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 631bfe4..e51f2ad 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -32,7 +32,9 @@
>  #include <stdio.h>
>  #include <stdarg.h>
>  #include <stdlib.h>
> +#ifndef NO_SYSLOG
>  #include <syslog.h>
> +#endif
>  #include <string.h>
>  #include <errno.h>
>  #include <dirent.h>
> @@ -61,13 +63,24 @@ LIST_HEAD(connections);
>  static int tracefd = -1;
>  static bool recovery = true;
>  static bool remove_local = true;
> +#ifndef NO_REOPEN_LOG
>  static int reopen_log_pipe[2];
> +#endif
>  static char *tracefile = NULL;
>  static TDB_CONTEXT *tdb_ctx;
> 
>  static void corrupt(struct connection *conn, const char *fmt, ...);
>  static void check_store(void);
> 
> +#ifdef __MINIOS__
> +#define lockf(...) (-ENOSYS)
> +#endif
> +
> +#ifdef NO_SYSLOG
> +#define openlog(...) ((void) 0)
> +#define syslog(...)  ((void) 0)
> +#endif
> +
>  #define log(...)                                                       \
>         do {                                                            \
>                 char *s = talloc_asprintf(NULL, __VA_ARGS__);           \
> @@ -92,8 +105,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;
> @@ -195,6 +210,7 @@ void trace_destroy(const void *data, const char *type)
>         trace("DESTROY %s %p\n", type, data);
>  }
> 
> +#ifndef NO_REOPEN_LOG
>  /**
>   * Signal handler for SIGHUP, which requests that the trace log is reopened
>   * (in the main loop).  A single byte is written to reopen_log_pipe, to 
> awaken
> @@ -222,7 +238,7 @@ static void reopen_log(void)
>                         trace("\n***\n");
>         }
>  }
> -
> +#endif
> 
>  static bool write_messages(struct connection *conn)
>  {
> @@ -326,7 +342,9 @@ static int initialize_set(fd_set *inset, fd_set *outset, 
> int sock, int ro_sock,
>         set_fd(sock,               inset, &max);
>         set_fd(ro_sock,            inset, &max);
>  #endif
> +#ifndef NO_REOPEN_LOG
>         set_fd(reopen_log_pipe[0], inset, &max);
> +#endif
> 
>         if (xce_handle != NULL)
>                 set_fd(xc_evtchn_fd(xce_handle), inset, &max);
> @@ -1415,7 +1433,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
> 
>  /* We create initial nodes manually. */
>  static void manual_node(const char *name, const char *child)
> @@ -1440,7 +1462,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
> 
>         if (tdb_ctx) {
>                 /* XXX When we make xenstored able to restart, this will have
> @@ -1666,6 +1692,7 @@ static void corrupt(struct connection *conn, const char 
> *fmt, ...)
>  }
> 
> 
> +#ifndef __MINIOS__
>  static void write_pidfile(const char *pidfile)
>  {
>         char buf[100];
> @@ -1712,7 +1739,7 @@ static void daemonize(void)
>         /* Discard our parent's old-fashioned umask prejudices. */
>         umask(0);
>  }
> -
> +#endif
> 
>  static void usage(void)
>  {
> @@ -1767,7 +1794,11 @@ int main(int argc, char *argv[])
>         struct sockaddr_un addr;
>  #endif
>         fd_set inset, outset;
> +#ifdef __MINIOS__
> +       bool dofork = false;
> +#else
>         bool dofork = true;
> +#endif
>         bool outputpid = false;
>         bool no_domain_init = false;
>         const char *pidfile = NULL;
> @@ -1821,8 +1852,11 @@ int main(int argc, char *argv[])
>         if (optind != argc)
>                 barf("%s: No arguments desired", argv[0]);
> 
> +#ifndef NO_REOPEN_LOG
>         reopen_log();
> +#endif
> 
> +#ifndef __MINIOS__
>         /* make sure xenstored directory exists */
>         if (mkdir(xs_daemon_rundir(), 0755)) {
>                 if (errno != EEXIST) {
> @@ -1844,6 +1878,7 @@ int main(int argc, char *argv[])
>         }
>         if (pidfile)
>                 write_pidfile(pidfile);
> +#endif
> 
>         /* Talloc leak reports go to stderr, which is closed if we fork. */
>         if (!dofork)
> @@ -1890,9 +1925,11 @@ int main(int argc, char *argv[])
>                 barf_perror("Could not listen on sockets");
>  #endif
> 
> +#ifndef NO_REOPEN_LOG
>         if (pipe(reopen_log_pipe)) {
>                 barf_perror("pipe");
>         }
> +#endif
> 
>         /* Setup the database */
>         setup_structure();
> @@ -1921,7 +1958,9 @@ int main(int argc, char *argv[])
>                 xprintf = trace;
>         }
> 
> +#ifndef NO_REOPEN_LOG
>         signal(SIGHUP, trigger_reopen_log);
> +#endif
> 
>         if (xce_handle != NULL)
>                 evtchn_fd = xc_evtchn_fd(xce_handle);
> @@ -1929,8 +1968,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 (;;) {
> @@ -1942,12 +1983,14 @@ int main(int argc, char *argv[])
>                         barf_perror("Select failed");
>                 }
> 
> +#ifndef NO_REOPEN_LOG
>                 if (FD_ISSET(reopen_log_pipe[0], &inset)) {
>                         char c;
>                         if (read(reopen_log_pipe[0], &c, 1) != 1)
>                                 barf_perror("read failed");
>                         reopen_log();
>                 }
> +#endif
> 
>  #ifndef NO_SOCKETS
>                 if (FD_ISSET(*sock, &inset))
> diff --git a/tools/xenstore/xenstored_domain.c 
> b/tools/xenstore/xenstored_domain.c
> index 0b8353b..811ae3c 100644
> --- a/tools/xenstore/xenstored_domain.c
> +++ b/tools/xenstore/xenstored_domain.c
> @@ -588,6 +588,12 @@ void restore_existing_connections(void)
>  {
>  }
> 
> +#ifdef __MINIOS__
> +static inline int dom0_init(void)
> +{
> +       return 0;
> +}
> +#else
>  static int dom0_init(void)
>  {
>         evtchn_port_t port;
> @@ -611,6 +617,7 @@ static int dom0_init(void)
> 
>         return 0;
>  }
> +#endif
> 
>  void domain_init(void)
>  {
> diff --git a/tools/xenstore/xenstored_transaction.c 
> b/tools/xenstore/xenstored_transaction.c
> index 380c691..c59acfb 100644
> --- a/tools/xenstore/xenstored_transaction.c
> +++ b/tools/xenstore/xenstored_transaction.c
> @@ -120,7 +120,9 @@ static int destroy_transaction(void *_transaction)
>         trace_destroy(trans, "transaction");
>         if (trans->tdb)
>                 tdb_close(trans->tdb);
> +#ifndef __MINIOS__
>         unlink(trans->tdb_name);
> +#endif
>         return 0;
>  }
> 
> --
> 1.7.7.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



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