[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v1 05/10] libxl: synchronise configuration when we hotplug a device



On Thu, 2014-07-17 at 15:13 +0100, Wei Liu wrote:
> On Thu, Jul 17, 2014 at 12:44:43PM +0100, Ian Campbell wrote:
> > On Wed, 2014-07-16 at 18:12 +0100, Wei Liu wrote:
> > > > > As we don't have a JSON config file for libxl toolstack domain
> > > > > (currently Dom0) we need to skip JSON manipulation for it.
> > > > 
> > > > How hard would it be to create a stub/stunt JSON for dom0 when we notice
> > > > it is missing? Or from e.g. xencommons perhaps?
> > > > 
> > > 
> > > We need to determine:
> > > 1. when / where to generate such thing (xencommons?)
> > > 2. what to put in it
> > > 
> > > I have yet had answers for #2. The simplest version can be "{}" I think.
> > > That is an empty configuration that every fields gets the default value.
> > > But we probably need more than that.
> > 
> > I think you would want at least a name and perhaps a uuid? And cinfo
> > type == PV.
> > 
> > Device arrays are all empty at start of day.
> > 
> > Some of the stuff about target and maxmem you could perhaps infer at
> > start of day?
> > 
> 
> UUID (Dom0 doesn't seem to have one), name and memory targets can all be
> pulled from xenstore when they are required.
> 
> And it occurs to me as I discovered Dom0 doesn't have UUID that we need
> to special-case reading / writing of Dom0's JSON config. That's because
> all other guests' JSON config are to be named with UUID and domain id.
> How annoying. :-(

Indeed.

I think you could generate one on boot and set it with
XEN_DOMCTL_setdomainhandle. It might be a bug that we don't do that
today (that said I can't see any evidence that xend used to do
differently).

> I would like to put as few things as possible in the stub because there
> doesn't seem to be a way to conveniently generate a valid JSON config
> for Dom0. Will you be against the idea of having 'xl generate-dom0-json'
> in xl to do that? Otherwise we have to basically generate a
> semi-handcoded stub in xencommons, or even a hardcoded stub.

I'm in favour of some sort of command to "initialise dom0". Either part
of xl or a new helper app based on libxl.

I had a similar (unposted I think) patch to add xl launch-dom0-qemu so
that it could pick the correct arch (see below, warning: it's a bit
skanky). If I were to do it again today I'd probably make a separate
$libexec helper instead of bolting it into xl though.

Probably it should subsume this bit of xencommons too:

                echo Setting domain 0 name and domid...
                ${BINDIR}/xenstore-write "/local/domain/0/name" "Domain-0"
                ${BINDIR}/xenstore-write "/local/domain/0/domid" 0

What do you think?

> > > > > +    DEVICE_ADD_JSON(vtpm, vtpms, num_vtpms, domid, &vtpm_saved,
> > > > 
> > > > The second and third of these arguments could be derived from the first.
> [...]
> > > > How close to being possible is it to do this as a proper helper function
> > > > which takes a size_t and a bunch of fn pointers with void where the type
> > > > would be?
> > > > 
> > > 
> > > We need to know the type of this structure otherwise we don't know what
> > > *_copy function to call. Sadly there's no way to pass in type information
> > > in C in runtime.
> > 
> > You could pass a copyfn as a function pointer with a void * argument for
> > the object to a helper function which doesn't need the specifics and
> > then have a macro which simply defines the type safe wrappers around
> > that. ISTR thinking  that the helper would need a sizeof passing to it
> > as well for the realloc.
> > 
> 
> I see. I will try to go with this approach.

I'd check with Ian J first, he might have some reason to prefer macros
over void * + wrapper.



Ian.


commit 7b5d54c9a5d09c4138bec905c9accea34173ba77
Author: Ian Campbell <ian.campbell@xxxxxxxxxx>
Date:   Wed May 15 16:34:55 2013 +0100

    tools: build and launch correct qemu for architecture
    
    xl now provides a launch-dom0-qemu command which avoids the need to have the
    initscripts be aware of the specific qermu binary name, which differs by
    architecture and which also may have been specified by the user via the
    --with-system-qemu=PATH option to configure.
    
    Perhaps this should be a separate binary hidden in libexec?
    
    Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

diff --git a/config/arm32.mk b/config/arm32.mk
index aa79d22..f3599a3 100644
--- a/config/arm32.mk
+++ b/config/arm32.mk
@@ -3,6 +3,8 @@ CONFIG_ARM_32 := y
 CONFIG_ARM_$(XEN_OS) := y
 
 CONFIG_XEN_INSTALL_SUFFIX :=
+CONFIG_QEMU_ARCH := arm
+CONFIG_QEMU_TARGET := arm-softmmu
 
 # -march= -mcpu=
 
diff --git a/config/arm64.mk b/config/arm64.mk
index 15b57a4..4ff15e0 100644
--- a/config/arm64.mk
+++ b/config/arm64.mk
@@ -3,6 +3,8 @@ CONFIG_ARM_64 := y
 CONFIG_ARM_$(XEN_OS) := y
 
 CONFIG_XEN_INSTALL_SUFFIX :=
+CONFIG_QEMU_ARCH := aarch64
+CONFIG_QEMU_TARGET := arm-softmmu
 
 CFLAGS += #-marm -march= -mcpu= etc
 
diff --git a/config/x86_32.mk b/config/x86_32.mk
index 7f76b25..da3111d 100644
--- a/config/x86_32.mk
+++ b/config/x86_32.mk
@@ -2,6 +2,9 @@ CONFIG_X86 := y
 CONFIG_X86_32 := y
 CONFIG_X86_$(XEN_OS) := y
 
+CONFIG_QEMU_ARCH := i386
+CONFIG_QEMU_TARGET := i386-softmmu
+
 CONFIG_HVM := y
 CONFIG_MIGRATE := y
 CONFIG_XCUTILS := y
diff --git a/config/x86_64.mk b/config/x86_64.mk
index 11104bd..f59e36d 100644
--- a/config/x86_64.mk
+++ b/config/x86_64.mk
@@ -2,6 +2,9 @@ CONFIG_X86 := y
 CONFIG_X86_64 := y
 CONFIG_X86_$(XEN_OS) := y
 
+CONFIG_QEMU_ARCH := x86_64
+CONFIG_QEMU_ARCH := i386-softmmu
+
 CONFIG_COMPAT := y
 CONFIG_HVM := y
 CONFIG_MIGRATE := y
diff --git a/tools/Makefile b/tools/Makefile
index 00c69ee..250b931 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -107,7 +107,7 @@ distclean: subdirs-distclean
                config.cache autom4te.cache
 
 ifneq ($(XEN_COMPILE_ARCH),$(XEN_TARGET_ARCH))
-IOEMU_CONFIGURE_CROSS ?= --cpu=$(XEN_TARGET_ARCH) \
+IOEMU_CONFIGURE_CROSS ?= --cpu=$(CONFIG_QEMU_ARCH) \
                         --cross-prefix=$(CROSS_COMPILE) \
                         --interp-prefix=$(CROSS_SYS_ROOT)
 endif
@@ -186,8 +186,7 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
                source=.; \
        fi; \
        cd qemu-xen-dir; \
-       $$source/configure --enable-xen --target-list=i386-softmmu \
-               $(QEMU_XEN_ENABLE_DEBUG) \
+       $$source/configure --enable-xen --target-list=$(CONFIG_QEMU_TARGET) \
                --prefix=$(PREFIX) \
                --source-path=$$source \
                --extra-cflags="-I$(XEN_ROOT)/tools/include \
diff --git a/tools/hotplug/Linux/init.d/xencommons 
b/tools/hotplug/Linux/init.d/xencommons
index 4ebd636..f568085 100644
--- a/tools/hotplug/Linux/init.d/xencommons
+++ b/tools/hotplug/Linux/init.d/xencommons
@@ -116,11 +116,7 @@ do_start () {
        echo Starting xenconsoled...
        test -z "$XENCONSOLED_TRACE" || XENCONSOLED_ARGS=" 
--log=$XENCONSOLED_TRACE"
        ${SBINDIR}/xenconsoled --pid-file=$XENCONSOLED_PIDFILE $XENCONSOLED_ARGS
-       echo Starting QEMU as disk backend for dom0
-       test -z "$QEMU_XEN" && QEMU_XEN="${LIBEXEC}/qemu-system-i386"
-       $QEMU_XEN -xen-domid 0 -xen-attach -name dom0 -nographic -M xenpv 
-daemonize \
-               -monitor /dev/null -serial /dev/null -parallel /dev/null \
-               -pidfile $QEMU_PIDFILE
+       xl launch-dom0-qemu $QEMU_XEN
 }
 do_stop () {
         echo Stopping xenconsoled
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index d8495bb..d8b6a5c 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -27,6 +27,7 @@ CFLAGS_LIBXL += $(CFLAGS_libxenguest)
 CFLAGS_LIBXL += $(CFLAGS_libxenstore)
 CFLAGS_LIBXL += $(CFLAGS_libblktapctl) 
 CFLAGS_LIBXL += -Wshadow
+CFLAGS_LIBXL += -DCONFIG_QEMU_ARCH=\"$(CONFIG_QEMU_ARCH)\"
 
 LIBXL_LIBS-$(CONFIG_ARM) += -lfdt
 
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 12d6c31..0287a35 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -605,6 +605,9 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
                     xentoollog_logger *lg);
 int libxl_ctx_free(libxl_ctx *ctx /* 0 is OK */);
 
+/* exec's device model for dom0 and does not return. */
+void libxl_launch_dom0_qemu(libxl_ctx *ctx, const char *qemu_path, const char 
*pidfile);
+
 /* domain related functions */
 
 int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config,
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index f6f7bbd..f0b13a9 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -38,7 +38,7 @@ static const char *qemu_xen_path(libxl__gc *gc)
 #ifdef QEMU_XEN_PATH
     return QEMU_XEN_PATH;
 #else
-    return libxl__abs_path(gc, "qemu-system-i386", libxl__libexec_path());
+    return libxl__abs_path(gc, "qemu-system-" CONFIG_QEMU_ARCH, 
libxl__libexec_path());
 #endif
 }
 
@@ -1560,6 +1560,37 @@ out:
     return ret;
 }
 
+void libxl_launch_dom0_qemu(libxl_ctx *ctx, const char *qemu_path, const char 
*pidfile)
+{
+    GC_INIT(ctx);
+
+    flexarray_t *dm_args = flexarray_make(gc, 16, 1);
+
+    if (qemu_path == NULL)
+        qemu_path = qemu_xen_path(gc);
+
+    flexarray_vappend(dm_args,
+                      "-xen-domid", "0",
+                      "-xen-attach",
+                      "-name", "dom0",
+                      "-nographic",
+                      "-M" "xenpv",
+                      "-daemonize",
+                      "-monitor", "/dev/null",
+                      "-serial", "/dev/null",
+                      "-parallel", "/dev/null",
+                      NULL);
+    if (pidfile)
+        flexarray_append_pair(dm_args, "-pidfile", libxl__strdup(gc, pidfile));
+
+    libxl__exec(gc, -1, -1, -1,
+                qemu_path, (char **)flexarray_contents(dm_args), NULL);
+
+    /* Shouldn't return */
+    LOG(CRITICAL, "Failed to exec dom0 device model");
+    GC_FREE;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index c876a33..1d7602c 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -106,6 +106,7 @@ int main_setenforce(int argc, char **argv);
 int main_loadpolicy(int argc, char **argv);
 int main_remus(int argc, char **argv);
 int main_devd(int argc, char **argv);
+int main_launch_dom0_qemu(int argc, char **argv);
 
 void help(const char *command);
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index bd26bcc..108dfac 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -7313,6 +7313,31 @@ out:
     return ret;
 }
 
+int main_launch_dom0_qemu(int argc, char **argv)
+{
+    int opt;
+    const char *qemu = NULL;
+    const char *pidfile = NULL;
+
+    SWITCH_FOREACH_OPT(opt, "p:", NULL, "launch-dom-qemu", 0) {
+    case 'p':
+        pidfile = optarg;
+        break;
+        /* No options */
+    }
+    if (optind < argc)
+        qemu = argv[optind];
+
+    fprintf(stderr, "argc %d\n", argc);
+    fprintf(stderr, "qemu = %s", qemu ? : "<default>");
+
+    fprintf(stdout, "Starting QEMU as disk backend for dom0");
+
+    libxl_launch_dom0_qemu(ctx, qemu, pidfile);
+
+    return 0;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index ebe0220..ab4d56c 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -494,6 +494,12 @@ struct cmd_spec cmd_table[] = {
       "[options]",
       "-F                      Run in the foreground",
     },
+    { "launch-dom0-qemu",
+      &main_launch_dom0_qemu, 0, 1,
+      "Start qemu process to service dom0 disk backends",
+      "[options] [QEMU_PATH]",
+      "-p PIDFILE              Write a PIDFILE\n",
+    },
 };
 
 int cmdtable_len = sizeof(cmd_table)/sizeof(struct cmd_spec);



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.