Re: [Xen-devel] [PATCH 25/28] libxl: emuids: Perhaps change dm xs control path

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Tue, 2015-12-22 at 18:45 +0000, Ian Jackson wrote:
> We are going to want to run two qemus, which will mean them having
> different xs control paths.ÂÂBut sometimes we will run only one qemu
> to do both jobs, in which case there has to be one xs control path,
> because otherwise the single qemu will only see in xenstore either the
> HVM DM work to do, or the PV (eg qdisk) backend work to do.
> 
> So we need to record whether any particular domain has been set up
> this way.ÂÂUse a new emuid flag bit EMUID_SPLIT which just tells us
> whether we are running two qemus in this way.

So, do I understand correctly that the states (combinations of bits) are:

0               No QEMUs
PV              Single QEMU only providing PV backends
DM              Single QEMU only providing device emulation
PV|DM           Single QEMU providing PV backends and device emulation
SPLIT|PV|DM     A pair of QEMUs, one for PV b/e and one for device emu
SPLIT|PV        Nonsense
SPLIT|DM        Nonsense        

?

Describing SPLIT as an "ID" is a bit odd (it's a kind of meta thing) but I
suppose I can see why it is done this way. An alternative might be to have
a separate bit for the PV|DM case together, maybe you discarded that
possibility though?

Ian.



> 
> If we do set this flag, we pass the emulator_id to qemu, so that qemu
> and libxl agree on the xenstore path.ÂÂObviously this depends on qemu
> understanding emulator_id, but we don't check that now, because:
> 
> We do not actually ever set EMUID_SPLIT, because when we run split
> qemus we also need to actually run two qemus which means a lot of
> non-xs-path-related changes which are more readable in a separate
> patch.
> 
> So right now there is no overall functional change.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> ---
> v6: Rewritten.
> ---
> Âdocs/misc/xenstore-paths.markdown |ÂÂÂ11 +++++++++++
> Âtools/libxl/libxl_dm.cÂÂÂÂÂÂÂÂÂÂÂÂ|ÂÂÂ15 +++++++++++++++
> Âtools/libxl/libxl_internal.cÂÂÂÂÂÂ|ÂÂÂ16 +++++++++++++++-
> Âtools/libxl/libxl_internal.hÂÂÂÂÂÂ|ÂÂÂÂ1 +
> Â4 files changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/misc/xenstore-paths.markdown b/docs/misc/xenstore-
> paths.markdown
> index 3bd31af..1dc54ac 100644
> --- a/docs/misc/xenstore-paths.markdown
> +++ b/docs/misc/xenstore-paths.markdown
> @@ -339,6 +339,17 @@ A PV console backend. Described in
> [console.txt](console.txt)
> ÂInformation relating to device models running in the domain. $DOMID is
> Âthe target domain of the device model.
> Â
> +#### ~/device-model/$DOMID[/$EMULATOR_ID]/* [INTERNAL]
> +
> +Information relating to device models running in the domain. $DOMID is
> +the target domain of the device model.
> +
> +$EMULATOR_ID indentifies a specific device model instance (multiple
> +device model instances for the same domain are possible).ÂÂThis path
> +component is present only if there are (going to be) more than one
> +device model for the domain.ÂÂSee `/libxl/$DOMID/dm-emuidmap` and
> +`EMUID_SPLIT` in `libxl_internal.h`.
> +
> Â#### ~/libxl/disable_udev = ("1"|"0") []
> Â
> ÂIndicates whether device hotplug scripts in this domain should be run
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 58760e7..d805800 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -1236,6 +1236,11 @@ static int
> libxl__build_device_model_args_new(libxl__gc *gc,
> ÂÂÂÂÂÂÂÂÂÂÂÂÂflexarray_append(dm_args, "-runas");
> ÂÂÂÂÂÂÂÂÂÂÂÂÂflexarray_append(dm_args, user);
> ÂÂÂÂÂÂÂÂÂ}
> +ÂÂÂÂÂÂÂÂif (state->emuidmap & (1u << EMUID_SPLIT)) {
> +ÂÂÂÂÂÂÂÂÂÂÂÂflexarray_append(dm_args, "-xenopts");
> +ÂÂÂÂÂÂÂÂÂÂÂÂflexarray_append(dm_args,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂGCSPRINTF("emulator_id=%u", emuid));
> +ÂÂÂÂÂÂÂÂ}
> ÂÂÂÂÂ}
> ÂÂÂÂÂflexarray_append(dm_args, NULL);
> ÂÂÂÂÂ*args = (char **) flexarray_contents(dm_args);
> @@ -1943,6 +1948,7 @@ void libxl__spawn_qdisk_backend(libxl__egc *egc,
> libxl__dm_spawn_state *dmss)
> ÂÂÂÂÂconst char *dm;
> ÂÂÂÂÂint logfile_w, null = -1, rc;
> ÂÂÂÂÂuint32_t domid = dmss->guest_domid;
> +ÂÂÂÂunsigned emuidmap;
> Â
> ÂÂÂÂÂ/* Always use qemu-xen as device model */
> ÂÂÂÂÂdm = qemu_xen_path(gc);
> @@ -1958,6 +1964,15 @@ void libxl__spawn_qdisk_backend(libxl__egc *egc,
> libxl__dm_spawn_state *dmss)
> ÂÂÂÂÂflexarray_vappend(dm_args, "-monitor", "/dev/null", NULL);
> ÂÂÂÂÂflexarray_vappend(dm_args, "-serial", "/dev/null", NULL);
> ÂÂÂÂÂflexarray_vappend(dm_args, "-parallel", "/dev/null", NULL);
> +
> +ÂÂÂÂrc = libxl__dm_emuidmap_get(gc, domid, &emuidmap);
> +ÂÂÂÂif (rc) goto error;
> +
> +ÂÂÂÂif (emuidmap & (1u << EMUID_SPLIT)) {
> +ÂÂÂÂÂÂÂÂflexarray_append(dm_args, "-xenopts");
> +ÂÂÂÂÂÂÂÂflexarray_append(dm_args,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂGCSPRINTF("emulator_id=%u", EMUID_PV));
> +ÂÂÂÂ}
> ÂÂÂÂÂflexarray_append(dm_args, NULL);
> ÂÂÂÂÂargs = (char **) flexarray_contents(dm_args);
> Â
> diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> index 30271c9..8dc34ad 100644
> --- a/tools/libxl/libxl_internal.c
> +++ b/tools/libxl/libxl_internal.c
> @@ -556,7 +556,21 @@ void libxl__update_domain_configuration(libxl__gc
> *gc,
> Âchar *libxl__dm_xs_path_rel(libxl__gc *gc,
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂuint32_t domid, int emuid)
> Â{
> -ÂÂÂÂreturn GCSPRINTF("device-model/%u", domid);
> +ÂÂÂÂunsigned emuidmap;
> +
> +ÂÂÂÂlibxl__dm_emuidmap_get_bodgeerrors(gc, domid, &emuidmap);
> +
> +ÂÂÂÂif (!(emuidmap & (1u << emuid))) {
> +ÂÂÂÂÂÂÂÂLOG(ERROR,
> + "Trying to find path for emulator %d but map is %#x, probable BUG",
> +ÂÂÂÂÂÂÂÂÂÂÂÂemuid, emuidmap);
> +ÂÂÂÂ}
> +
> +ÂÂÂÂif (!(emuidmap & (1u << EMUID_SPLIT))) {
> +ÂÂÂÂÂÂÂÂreturn GCSPRINTF("device-model/%u", domid);
> +ÂÂÂÂ} else {
> +ÂÂÂÂÂÂÂÂreturn GCSPRINTF("device-model/%u/%u", domid, emuid);
> +ÂÂÂÂ}
> Â}
> Â
> Âchar *libxl__device_model_xs_path(libxl__gc *gc, uint32_t dm_domid,
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 82f264a..02c35e0 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1962,6 +1962,7 @@ _hidden libxl_device_model_version
> libxl__default_device_model(libxl__gc *gc);
> Âenum {
> ÂÂÂÂÂEMUID_PV,
> ÂÂÂÂÂEMUID_DM,
> +ÂÂÂÂEMUID_SPLIT, /* we will run both PV and DM, put emuid in xs path */
> ÂÂÂÂÂ/* NB stubdom and its PV service domain not recorded here */
> Â};
> Â

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