| Message ID | 8d8515cabfdc666bd1afb40f6b7e44ccbe362dbf.1579055705.git-series.marmarek@invisiblethingslab.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for qemu-xen runnning in a Linux-based stubdomain. | expand |
On Tue, Jan 14, 2020 at 9:42 PM Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> wrote: > > Access to QMP of QEMU in Linux stubdomain is possible over vchan > connection. Handle the actual vchan connection in a separate process > (vchan-socket-proxy). This simplified integration with QMP (already > quite complex), but also allows preliminary filtering of (potentially > malicious) QMP input. > Since only one client can be connected to vchan server at the same time > and it is not enforced by the libxenvchan itself, additional client-side > locking is needed. It is implicitly implemented by vchan-socket-proxy, > as it handle only one connection at a time. Note that qemu supports only > one simultaneous client on a control socket anyway (but in UNIX socket > case, it enforce it server-side), so it doesn't add any extra > limitation. > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > --- > Changes in v4: > - new patch, in place of both "libxl: use vchan for QMP access ..." > --- > tools/configure.ac | 9 ++- > tools/libxl/libxl_dm.c | 159 ++++++++++++++++++++++++++++++++++-- > tools/libxl/libxl_internal.h | 1 +- > 3 files changed, 161 insertions(+), 8 deletions(-) > > diff --git a/tools/configure.ac b/tools/configure.ac > index 8d86c42..20bbdbf 100644 > --- a/tools/configure.ac > +++ b/tools/configure.ac > @@ -192,6 +192,15 @@ AC_SUBST(qemu_xen) > AC_SUBST(qemu_xen_path) > AC_SUBST(qemu_xen_systemd) > > +AC_ARG_WITH([stubdom-qmp-proxy], > + AC_HELP_STRING([--stubdom-qmp-proxy@<:@=PATH@:>@], > + [Use supplied binary PATH as a QMP proxy into stubdomain]),[ Thanks for making it configurable :) > + stubdom_qmp_proxy="$withval" > +],[ > + stubdom_qmp_proxy="$bindir/vchan-socket-proxy" > +]) > +AC_DEFINE_UNQUOTED([STUBDOM_QMP_PROXY_PATH], ["$stubdom_qmp_proxy"], [QMP proxy path]) > + > AC_ARG_WITH([system-seabios], > AS_HELP_STRING([--with-system-seabios@<:@=PATH@:>@], > [Use system supplied seabios PATH instead of building and installing > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > index 528ca3e..23ac7e4 100644 > --- a/tools/libxl/libxl_dm.c > +++ b/tools/libxl/libxl_dm.c > @@ -1183,7 +1183,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, > "-xen-domid", > GCSPRINTF("%d", guest_domid), NULL); > > - /* There is currently no way to access the QMP socket in the stubdom */ > + /* QMP access to qemu running in stubdomain is done over vchan, stubdomain setup it itself */ I think this would be clearer: /* QMP access to qemu running in stubdomain is done over vchan. The stubdomain init script * adds the appropriate monitor options for vchan-socket-proxy. */ In the block below, the -no-shutdown option is added to qemu, which will not be done for linux stubdomain. -no-shutdown Don't exit QEMU on guest shutdown, but instead only stop the emulation. This allows for instance switching to monitor to commit changes to the disk image. It's something I noticed, but I don't know if it matters to us. > if (!is_stubdom) { > flexarray_append(dm_args, "-chardev"); > if (state->dm_monitor_fd >= 0) { > @@ -2178,6 +2178,23 @@ static void stubdom_pvqemu_unpaused(libxl__egc *egc, <snip> > @@ -2460,24 +2477,150 @@ static void spawn_stub_launch_dm(libxl__egc *egc, > goto out; > } > > + sdss->qmp_proxy_spawn.ao = ao; > + if (libxl__stubdomain_is_linux(&guest_config->b_info)) { > + spawn_qmp_proxy(egc, sdss); > + } else { > + qmp_proxy_spawn_outcome(egc, sdss, 0); > + } > + > + return; > + > +out: > + assert(ret); > + qmp_proxy_spawn_outcome(egc, sdss, ret); > +} > + > +static void spawn_qmp_proxy(libxl__egc *egc, > + libxl__stub_dm_spawn_state *sdss) > +{ > + STATE_AO_GC(sdss->qmp_proxy_spawn.ao); > + const uint32_t guest_domid = sdss->dm.guest_domid; > + const uint32_t dm_domid = sdss->pvqemu.guest_domid; > + const char *dom_path = libxl__xs_get_dompath(gc, dm_domid); > + char **args; > + int nr = 0; > + int rc, logfile_w, null; > + > + if (access(STUBDOM_QMP_PROXY_PATH, X_OK) < 0) { > + LOGED(ERROR, guest_domid, "qmp proxy %s is not executable", STUBDOM_QMP_PROXY_PATH); > + rc = ERROR_FAIL; > + goto out; > + } > + > + sdss->qmp_proxy_spawn.what = GCSPRINTF("domain %d device model qmp proxy", guest_domid); > + sdss->qmp_proxy_spawn.pidpath = GCSPRINTF("%s/image/qmp-proxy-pid", dom_path); > + sdss->qmp_proxy_spawn.xspath = GCSPRINTF("%s/image/qmp-proxy-state", dom_path); Since this is the vchan-socket-proxy in dom0, should it write to "device-model/%u/qmp-proxy-state" underneath dom0? > + > + sdss->qmp_proxy_spawn.timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000; > + sdss->qmp_proxy_spawn.midproc_cb = libxl__spawn_record_pid; > + sdss->qmp_proxy_spawn.confirm_cb = qmp_proxy_confirm; > + sdss->qmp_proxy_spawn.failure_cb = qmp_proxy_startup_failed; > + sdss->qmp_proxy_spawn.detached_cb = qmp_proxy_detached; > + > + const int arraysize = 6; > + GCNEW_ARRAY(args, arraysize); > + args[nr++] = STUBDOM_QMP_PROXY_PATH; > + args[nr++] = GCSPRINTF("--state-path=%s", sdss->qmp_proxy_spawn.xspath); > + args[nr++] = GCSPRINTF("%u", dm_domid); > + args[nr++] = GCSPRINTF("%s/device-model/%u/qmp-vchan", dom_path, guest_domid); Thinking of OpenXT"s qmp-helper, this path isn't useful. But it is for vchan-socket-proxy, so qmp-helper could just change to ignore it. > + args[nr++] = (char*)libxl__qemu_qmp_path(gc, guest_domid); qmp-helper takes just the stub_domid and domid. The domid is just used to generate the above path, but taking the path would be cleaner. > + args[nr++] = NULL; > + assert(nr == arraysize); This generally looks good. Regards, Jason
On Tue, Jan 21, 2020 at 03:17:39PM -0500, Jason Andryuk wrote: > On Tue, Jan 14, 2020 at 9:42 PM Marek Marczykowski-Górecki > <marmarek@invisiblethingslab.com> wrote: > > > > Access to QMP of QEMU in Linux stubdomain is possible over vchan > > connection. Handle the actual vchan connection in a separate process > > (vchan-socket-proxy). This simplified integration with QMP (already > > quite complex), but also allows preliminary filtering of (potentially > > malicious) QMP input. > > Since only one client can be connected to vchan server at the same time > > and it is not enforced by the libxenvchan itself, additional client-side > > locking is needed. It is implicitly implemented by vchan-socket-proxy, > > as it handle only one connection at a time. Note that qemu supports only > > one simultaneous client on a control socket anyway (but in UNIX socket > > case, it enforce it server-side), so it doesn't add any extra > > limitation. > > > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > > --- > > Changes in v4: > > - new patch, in place of both "libxl: use vchan for QMP access ..." > > --- > > tools/configure.ac | 9 ++- > > tools/libxl/libxl_dm.c | 159 ++++++++++++++++++++++++++++++++++-- > > tools/libxl/libxl_internal.h | 1 +- > > 3 files changed, 161 insertions(+), 8 deletions(-) > > > > diff --git a/tools/configure.ac b/tools/configure.ac > > index 8d86c42..20bbdbf 100644 > > --- a/tools/configure.ac > > +++ b/tools/configure.ac > > @@ -192,6 +192,15 @@ AC_SUBST(qemu_xen) > > AC_SUBST(qemu_xen_path) > > AC_SUBST(qemu_xen_systemd) > > > > +AC_ARG_WITH([stubdom-qmp-proxy], > > + AC_HELP_STRING([--stubdom-qmp-proxy@<:@=PATH@:>@], > > + [Use supplied binary PATH as a QMP proxy into stubdomain]),[ > > Thanks for making it configurable :) > > > + stubdom_qmp_proxy="$withval" > > +],[ > > + stubdom_qmp_proxy="$bindir/vchan-socket-proxy" > > +]) > > +AC_DEFINE_UNQUOTED([STUBDOM_QMP_PROXY_PATH], ["$stubdom_qmp_proxy"], [QMP proxy path]) > > + > > AC_ARG_WITH([system-seabios], > > AS_HELP_STRING([--with-system-seabios@<:@=PATH@:>@], > > [Use system supplied seabios PATH instead of building and installing > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > > index 528ca3e..23ac7e4 100644 > > --- a/tools/libxl/libxl_dm.c > > +++ b/tools/libxl/libxl_dm.c > > @@ -1183,7 +1183,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, > > "-xen-domid", > > GCSPRINTF("%d", guest_domid), NULL); > > > > - /* There is currently no way to access the QMP socket in the stubdom */ > > + /* QMP access to qemu running in stubdomain is done over vchan, stubdomain setup it itself */ > > I think this would be clearer: > /* QMP access to qemu running in stubdomain is done over vchan. The > stubdomain init script > * adds the appropriate monitor options for vchan-socket-proxy. */ Yes, clearer. > In the block below, the -no-shutdown option is added to qemu, which > will not be done for linux stubdomain. > -no-shutdown > Don't exit QEMU on guest shutdown, but instead only stop the > emulation. This allows for instance switching to monitor to commit > changes to the disk image. > > It's something I noticed, but I don't know if it matters to us. I'll move it outside, looks like unrelated change. > > if (!is_stubdom) { > > flexarray_append(dm_args, "-chardev"); > > if (state->dm_monitor_fd >= 0) { > > @@ -2178,6 +2178,23 @@ static void stubdom_pvqemu_unpaused(libxl__egc *egc, > > <snip> > > > @@ -2460,24 +2477,150 @@ static void spawn_stub_launch_dm(libxl__egc *egc, > > goto out; > > } > > > > + sdss->qmp_proxy_spawn.ao = ao; > > + if (libxl__stubdomain_is_linux(&guest_config->b_info)) { > > + spawn_qmp_proxy(egc, sdss); > > + } else { > > + qmp_proxy_spawn_outcome(egc, sdss, 0); > > + } > > + > > + return; > > + > > +out: > > + assert(ret); > > + qmp_proxy_spawn_outcome(egc, sdss, ret); > > +} > > + > > +static void spawn_qmp_proxy(libxl__egc *egc, > > + libxl__stub_dm_spawn_state *sdss) > > +{ > > + STATE_AO_GC(sdss->qmp_proxy_spawn.ao); > > + const uint32_t guest_domid = sdss->dm.guest_domid; > > + const uint32_t dm_domid = sdss->pvqemu.guest_domid; > > + const char *dom_path = libxl__xs_get_dompath(gc, dm_domid); > > + char **args; > > + int nr = 0; > > + int rc, logfile_w, null; > > + > > + if (access(STUBDOM_QMP_PROXY_PATH, X_OK) < 0) { > > + LOGED(ERROR, guest_domid, "qmp proxy %s is not executable", STUBDOM_QMP_PROXY_PATH); > > + rc = ERROR_FAIL; > > + goto out; > > + } > > + > > + sdss->qmp_proxy_spawn.what = GCSPRINTF("domain %d device model qmp proxy", guest_domid); > > + sdss->qmp_proxy_spawn.pidpath = GCSPRINTF("%s/image/qmp-proxy-pid", dom_path); > > + sdss->qmp_proxy_spawn.xspath = GCSPRINTF("%s/image/qmp-proxy-state", dom_path); > > Since this is the vchan-socket-proxy in dom0, should it write to > "device-model/%u/qmp-proxy-state" underneath dom0? Yes, that would be more consistent. But pid should stay where it is (it's also where dom0 qemu pid is being written). > > + > > + sdss->qmp_proxy_spawn.timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000; > > + sdss->qmp_proxy_spawn.midproc_cb = libxl__spawn_record_pid; > > + sdss->qmp_proxy_spawn.confirm_cb = qmp_proxy_confirm; > > + sdss->qmp_proxy_spawn.failure_cb = qmp_proxy_startup_failed; > > + sdss->qmp_proxy_spawn.detached_cb = qmp_proxy_detached; > > + > > + const int arraysize = 6; > > + GCNEW_ARRAY(args, arraysize); > > + args[nr++] = STUBDOM_QMP_PROXY_PATH; > > + args[nr++] = GCSPRINTF("--state-path=%s", sdss->qmp_proxy_spawn.xspath); > > + args[nr++] = GCSPRINTF("%u", dm_domid); > > + args[nr++] = GCSPRINTF("%s/device-model/%u/qmp-vchan", dom_path, guest_domid); > > Thinking of OpenXT"s qmp-helper, this path isn't useful. But it is > for vchan-socket-proxy, so qmp-helper could just change to ignore it. For vchan we could use also a port number (and then it will encode it into a xenstore path). This is in fact how we use libvchan in Qubes. I opted for explicit path only because of lack of idea for some meaningful port number ;) But I'm open for suggestions. I guess that would be useful for Argo version then. > > + args[nr++] = (char*)libxl__qemu_qmp_path(gc, guest_domid); > > qmp-helper takes just the stub_domid and domid. The domid is just > used to generate the above path, but taking the path would be cleaner. > > > + args[nr++] = NULL; > > + assert(nr == arraysize); > > This generally looks good. > > Regards, > Jason
On Tue, Jan 21, 2020 at 6:46 PM Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> wrote: <snip> > > > +static void spawn_qmp_proxy(libxl__egc *egc, > > > + libxl__stub_dm_spawn_state *sdss) > > > +{ > > > + STATE_AO_GC(sdss->qmp_proxy_spawn.ao); > > > + const uint32_t guest_domid = sdss->dm.guest_domid; > > > + const uint32_t dm_domid = sdss->pvqemu.guest_domid; > > > + const char *dom_path = libxl__xs_get_dompath(gc, dm_domid); > > > + char **args; > > > + int nr = 0; > > > + int rc, logfile_w, null; > > > + > > > + if (access(STUBDOM_QMP_PROXY_PATH, X_OK) < 0) { > > > + LOGED(ERROR, guest_domid, "qmp proxy %s is not executable", STUBDOM_QMP_PROXY_PATH); > > > + rc = ERROR_FAIL; > > > + goto out; > > > + } > > > + > > > + sdss->qmp_proxy_spawn.what = GCSPRINTF("domain %d device model qmp proxy", guest_domid); > > > + sdss->qmp_proxy_spawn.pidpath = GCSPRINTF("%s/image/qmp-proxy-pid", dom_path); > > > + sdss->qmp_proxy_spawn.xspath = GCSPRINTF("%s/image/qmp-proxy-state", dom_path); > > > > Since this is the vchan-socket-proxy in dom0, should it write to > > "device-model/%u/qmp-proxy-state" underneath dom0? > > Yes, that would be more consistent. But pid should stay where it is > (it's also where dom0 qemu pid is being written). Hmm, that split between pids and device-model info is a little weird. But it is specified in docs misc/xenstore-paths.pandoc > > > + > > > + sdss->qmp_proxy_spawn.timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000; > > > + sdss->qmp_proxy_spawn.midproc_cb = libxl__spawn_record_pid; > > > + sdss->qmp_proxy_spawn.confirm_cb = qmp_proxy_confirm; > > > + sdss->qmp_proxy_spawn.failure_cb = qmp_proxy_startup_failed; > > > + sdss->qmp_proxy_spawn.detached_cb = qmp_proxy_detached; > > > + > > > + const int arraysize = 6; > > > + GCNEW_ARRAY(args, arraysize); > > > + args[nr++] = STUBDOM_QMP_PROXY_PATH; > > > + args[nr++] = GCSPRINTF("--state-path=%s", sdss->qmp_proxy_spawn.xspath); > > > + args[nr++] = GCSPRINTF("%u", dm_domid); > > > + args[nr++] = GCSPRINTF("%s/device-model/%u/qmp-vchan", dom_path, guest_domid); > > > > Thinking of OpenXT"s qmp-helper, this path isn't useful. But it is > > for vchan-socket-proxy, so qmp-helper could just change to ignore it. > > For vchan we could use also a port number (and then it will encode it > into a xenstore path). This is in fact how we use libvchan in Qubes. I > opted for explicit path only because of lack of idea for some meaningful > port number ;) But I'm open for suggestions. > I guess that would be useful for Argo version then. The argo version hard codes the port number, so it's not a command line argument. The port number would need to get passed to the stubdom or it would need to be standardized. I think the arguments for vchan-socket-proxy make sense. Since it's the one that's submitted upstream, it makes sense to use them. Put another way, do we want this to support alternate implementations for a qmp proxy? Should the arguments be generic for that case? Since the existing arguments have enough information for either proxy, I think it's fine to leave it as is. While you could have a wrapper generate all the above from just the domid & stub_domid, that's kinda hacky. Thanks, Jason
diff --git a/tools/configure.ac b/tools/configure.ac index 8d86c42..20bbdbf 100644 --- a/tools/configure.ac +++ b/tools/configure.ac @@ -192,6 +192,15 @@ AC_SUBST(qemu_xen) AC_SUBST(qemu_xen_path) AC_SUBST(qemu_xen_systemd) +AC_ARG_WITH([stubdom-qmp-proxy], + AC_HELP_STRING([--stubdom-qmp-proxy@<:@=PATH@:>@], + [Use supplied binary PATH as a QMP proxy into stubdomain]),[ + stubdom_qmp_proxy="$withval" +],[ + stubdom_qmp_proxy="$bindir/vchan-socket-proxy" +]) +AC_DEFINE_UNQUOTED([STUBDOM_QMP_PROXY_PATH], ["$stubdom_qmp_proxy"], [QMP proxy path]) + AC_ARG_WITH([system-seabios], AS_HELP_STRING([--with-system-seabios@<:@=PATH@:>@], [Use system supplied seabios PATH instead of building and installing diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 528ca3e..23ac7e4 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -1183,7 +1183,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, "-xen-domid", GCSPRINTF("%d", guest_domid), NULL); - /* There is currently no way to access the QMP socket in the stubdom */ + /* QMP access to qemu running in stubdomain is done over vchan, stubdomain setup it itself */ if (!is_stubdom) { flexarray_append(dm_args, "-chardev"); if (state->dm_monitor_fd >= 0) { @@ -2178,6 +2178,23 @@ static void stubdom_pvqemu_unpaused(libxl__egc *egc, static void stubdom_xswait_cb(libxl__egc *egc, libxl__xswait_state *xswait, int rc, const char *p); +static void spawn_qmp_proxy(libxl__egc *egc, + libxl__stub_dm_spawn_state *sdss); + +static void qmp_proxy_confirm(libxl__egc *egc, libxl__spawn_state *spawn, + const char *xsdata); + +static void qmp_proxy_startup_failed(libxl__egc *egc, + libxl__spawn_state *spawn, + int rc); + +static void qmp_proxy_detached(libxl__egc *egc, + libxl__spawn_state *spawn); + +static void qmp_proxy_spawn_outcome(libxl__egc *egc, + libxl__stub_dm_spawn_state *sdss, + int rc); + char *libxl__stub_dm_name(libxl__gc *gc, const char *guest_name) { return GCSPRINTF("%s-dm", guest_name); @@ -2460,24 +2477,150 @@ static void spawn_stub_launch_dm(libxl__egc *egc, goto out; } + sdss->qmp_proxy_spawn.ao = ao; + if (libxl__stubdomain_is_linux(&guest_config->b_info)) { + spawn_qmp_proxy(egc, sdss); + } else { + qmp_proxy_spawn_outcome(egc, sdss, 0); + } + + return; + +out: + assert(ret); + qmp_proxy_spawn_outcome(egc, sdss, ret); +} + +static void spawn_qmp_proxy(libxl__egc *egc, + libxl__stub_dm_spawn_state *sdss) +{ + STATE_AO_GC(sdss->qmp_proxy_spawn.ao); + const uint32_t guest_domid = sdss->dm.guest_domid; + const uint32_t dm_domid = sdss->pvqemu.guest_domid; + const char *dom_path = libxl__xs_get_dompath(gc, dm_domid); + char **args; + int nr = 0; + int rc, logfile_w, null; + + if (access(STUBDOM_QMP_PROXY_PATH, X_OK) < 0) { + LOGED(ERROR, guest_domid, "qmp proxy %s is not executable", STUBDOM_QMP_PROXY_PATH); + rc = ERROR_FAIL; + goto out; + } + + sdss->qmp_proxy_spawn.what = GCSPRINTF("domain %d device model qmp proxy", guest_domid); + sdss->qmp_proxy_spawn.pidpath = GCSPRINTF("%s/image/qmp-proxy-pid", dom_path); + sdss->qmp_proxy_spawn.xspath = GCSPRINTF("%s/image/qmp-proxy-state", dom_path); + + sdss->qmp_proxy_spawn.timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000; + sdss->qmp_proxy_spawn.midproc_cb = libxl__spawn_record_pid; + sdss->qmp_proxy_spawn.confirm_cb = qmp_proxy_confirm; + sdss->qmp_proxy_spawn.failure_cb = qmp_proxy_startup_failed; + sdss->qmp_proxy_spawn.detached_cb = qmp_proxy_detached; + + const int arraysize = 6; + GCNEW_ARRAY(args, arraysize); + args[nr++] = STUBDOM_QMP_PROXY_PATH; + args[nr++] = GCSPRINTF("--state-path=%s", sdss->qmp_proxy_spawn.xspath); + args[nr++] = GCSPRINTF("%u", dm_domid); + args[nr++] = GCSPRINTF("%s/device-model/%u/qmp-vchan", dom_path, guest_domid); + args[nr++] = (char*)libxl__qemu_qmp_path(gc, guest_domid); + args[nr++] = NULL; + assert(nr == arraysize); + + logfile_w = libxl__create_qemu_logfile(gc, GCSPRINTF("qmp-proxy-%s", + sdss->dm_config.c_info.name)); + if (logfile_w < 0) { + rc = logfile_w; + goto out; + } + null = open("/dev/null", O_RDWR); + if (null < 0) { + LOGED(ERROR, guest_domid, "unable to open /dev/null"); + rc = ERROR_FAIL; + goto out_close; + } + + rc = libxl__spawn_spawn(egc, &sdss->qmp_proxy_spawn); + if (rc < 0) + goto out_close; + if (!rc) { /* inner child */ + setsid(); + libxl__exec(gc, null, null, logfile_w, STUBDOM_QMP_PROXY_PATH, args, NULL); + /* unreachable */ + } + + rc = 0; + +out_close: + if (logfile_w >= 0) + close(logfile_w); + if (null >= 0) + close(null); +out: + if (rc) + qmp_proxy_spawn_outcome(egc, sdss, rc); +} + +static void qmp_proxy_confirm(libxl__egc *egc, libxl__spawn_state *spawn, + const char *xsdata) +{ + STATE_AO_GC(spawn->ao); + + if (!xsdata) + return; + + if (strcmp(xsdata, "running")) + return; + + libxl__spawn_initiate_detach(gc, spawn); +} + +static void qmp_proxy_startup_failed(libxl__egc *egc, + libxl__spawn_state *spawn, + int rc) +{ + libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(spawn, *sdss, qmp_proxy_spawn); + qmp_proxy_spawn_outcome(egc, sdss, rc); +} + +static void qmp_proxy_detached(libxl__egc *egc, + libxl__spawn_state *spawn) +{ + libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(spawn, *sdss, qmp_proxy_spawn); + qmp_proxy_spawn_outcome(egc, sdss, 0); +} + +static void qmp_proxy_spawn_outcome(libxl__egc *egc, + libxl__stub_dm_spawn_state *sdss, + int rc) +{ + STATE_AO_GC(sdss->qmp_proxy_spawn.ao); + int need_pvqemu = libxl__need_xenpv_qemu(gc, &sdss->dm_config); + + if (rc) goto out; + + if (need_pvqemu < 0) { + rc = need_pvqemu; + goto out; + } + sdss->pvqemu.spawn.ao = ao; - sdss->pvqemu.guest_domid = dm_domid; sdss->pvqemu.guest_config = &sdss->dm_config; sdss->pvqemu.build_state = &sdss->dm_state; sdss->pvqemu.callback = spawn_stubdom_pvqemu_cb; - - if (!need_qemu) { + if (need_pvqemu) { + libxl__spawn_local_dm(egc, &sdss->pvqemu); + } else { /* If dom0 qemu not needed, do not launch it */ spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, 0); - } else { - libxl__spawn_local_dm(egc, &sdss->pvqemu); } return; out: - assert(ret); - spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, ret); + assert(rc); + spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, rc); } static void spawn_stubdom_pvqemu_cb(libxl__egc *egc, diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 2b4a1cc..895bb65 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -4129,6 +4129,7 @@ typedef struct { libxl__destroy_domid_state dis; libxl__multidev multidev; libxl__xswait_state xswait; + libxl__spawn_state qmp_proxy_spawn; } libxl__stub_dm_spawn_state; _hidden void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state*);
Access to QMP of QEMU in Linux stubdomain is possible over vchan connection. Handle the actual vchan connection in a separate process (vchan-socket-proxy). This simplified integration with QMP (already quite complex), but also allows preliminary filtering of (potentially malicious) QMP input. Since only one client can be connected to vchan server at the same time and it is not enforced by the libxenvchan itself, additional client-side locking is needed. It is implicitly implemented by vchan-socket-proxy, as it handle only one connection at a time. Note that qemu supports only one simultaneous client on a control socket anyway (but in UNIX socket case, it enforce it server-side), so it doesn't add any extra limitation. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v4: - new patch, in place of both "libxl: use vchan for QMP access ..." --- tools/configure.ac | 9 ++- tools/libxl/libxl_dm.c | 159 ++++++++++++++++++++++++++++++++++-- tools/libxl/libxl_internal.h | 1 +- 3 files changed, 161 insertions(+), 8 deletions(-)