On Thu, Feb 19, 2015 at 01:45:52PM -0700, Jim Fehlig wrote:
Marek Marczykowski-Górecki wrote:
> Xen have feature of having device model in separate domain (called stub
> domain). Add a 'type' attribute to 'emulator' element to allow
selecting
> such a configuration.
Or maybe 'mode', describing the mode in which the emulator runs: as a
process or as a VM.
> Emulator path is still used for qemu running in dom0 (if
> any). Libxl currently do not allow to select stubdomain path.
>
That seems to overload a single <emulator>. Would it be better to have
multiple <emulators>? E.g.
<emulator>/usr/lib/xen/bin/qemu-system-i386</emulator>
<emulator mode='stubdomain'>/usr/lib/xen/bin/stubdom-dm</emulator>
So the existence of <emulator mode='stubdomain'/> would enable this
feature?
What if someday the default will be to run stubdomain emulator - how
the user can disable it in such case?
> Signed-off-by: Marek Marczykowski-Górecki
<marmarek(a)invisiblethingslab.com>
> ---
>
> I think it would be good idea to introduce the same change to capabilities XML.
> The problem is I can't include domain_conf.h from capabilities.h, so probably
> that enum declaration needs to be moved to capabilities.h. Is it the right way?
> Or it should be done somehow different?
Any hints here? IMHO it would be hardly useful without mentioning
possible values in capabilities XML...
In case of multiple <emulator>s, should them be just listed (possible
values), or the whole possible combinations (which 'process' emulator
with which 'stubdomain' one if applicable)? I think the later option is
better.
> docs/formatdomain.html.in | 14 ++++++++++++++
> docs/schemas/domaincommon.rng | 23 ++++++++++++++++++++++-
> src/conf/domain_conf.c | 36 ++++++++++++++++++++++++++++++++++--
> src/conf/domain_conf.h | 9 +++++++++
> src/libxl/libxl_conf.c | 17 +++++++++++++++++
> 5 files changed, 96 insertions(+), 3 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 38c42d5..4f539e2 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1652,6 +1652,20 @@
> The <a href="formatcaps.html">capabilities XML</a>
specifies
> the recommended default emulator to use for each particular
> domain type / architecture combination.
> +
> + <span class="since">Since 1.2.13</span>, the
<code>emulator</code>
> + element may contain <code>type</code> attribute. Possible
values are:
> + <dl>
> + <dt><code>type='default'</code></dt>
> + <dd>Equivalent to not setting <code>type</code>
attribute at all.
> + </dd>
> +
> + <dt><code>type='stubdom'</code></dt>
> + <dd>Launch emulator in stub domain (Xen only). The emulator path
> + still indicate which binary is used in dom0 - there is no control
> + which binary is used as a stub domain.
> + </dd>
> + </dl>
> </dd>
> </dl>
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index a4321f1..2a12073 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -2519,7 +2519,28 @@
> -->
> <define name="emulator">
> <element name="emulator">
> - <ref name="absFilePath"/>
> + <choice>
> + <group>
> + <optional>
> + <attribute name="type">
> + <choice>
> + <value>qemu</value>
> + <value>stubdom</value>
> + </choice>
> + </attribute>
> + </optional>
> + <ref name="absFilePath"/>
> + </group>
> + <group>
> + <attribute name="type">
> + <choice>
> + <value>qemu</value>
> + <value>stubdom</value>
> + </choice>
> + </attribute>
> + <empty/>
> + </group>
> + </choice>
> </element>
> </define>
> <!--
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 17b699a..c268091 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -741,6 +741,10 @@ VIR_ENUM_IMPL(virDomainDiskTray, VIR_DOMAIN_DISK_TRAY_LAST,
> "closed",
> "open");
>
> +VIR_ENUM_IMPL(virDomainEmulatorType, VIR_DOMAIN_EMULATOR_TYPE_LAST,
> + "qemu",
> + "stubdom");
> +
> VIR_ENUM_IMPL(virDomainRNGModel,
> VIR_DOMAIN_RNG_MODEL_LAST,
> "virtio");
> @@ -13712,6 +13716,14 @@ virDomainDefParseXML(xmlDocPtr xml,
> }
>
> def->emulator = virXPathString("string(./devices/emulator[1])",
ctxt);
> + if ((tmp = virXPathString("string(./devices/emulator/@type)", ctxt)))
{
> + def->emulator_type = virDomainEmulatorTypeTypeFromString(tmp);
> + VIR_FREE(tmp);
> + if (def->emulator_type < 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Unknown emulator type '%s'"),
tmp);
> + }
> + }
>
> /* analysis of the disk devices */
> if ((n = virXPathNodeSet("./devices/disk", ctxt, &nodes)) <
0)
> @@ -15690,6 +15702,14 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
> goto error;
> }
>
> + if (src->emulator_type != dst->emulator_type) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Target domain emulator type %s does not match source
%s"),
> + virDomainEmulatorTypeTypeToString(dst->emulator_type),
> + virDomainEmulatorTypeTypeToString(src->emulator_type));
> + goto error;
> + }
> +
> if (!virDomainDefFeaturesCheckABIStability(src, dst))
> goto error;
>
> @@ -19893,8 +19913,20 @@ virDomainDefFormatInternal(virDomainDefPtr def,
> virBufferAddLit(buf, "<devices>\n");
> virBufferAdjustIndent(buf, 2);
>
> - virBufferEscapeString(buf, "<emulator>%s</emulator>\n",
> - def->emulator);
> + if (def->emulator ||
> + def->emulator_type != VIR_DOMAIN_EMULATOR_TYPE_DEFAULT) {
> + virBufferAddLit(buf, "<emulator");
> + if (def->emulator_type != VIR_DOMAIN_EMULATOR_TYPE_DEFAULT) {
> + virBufferAsprintf(buf, " type='%s'",
> +
virDomainEmulatorTypeTypeToString(def->emulator_type));
> + }
> + if (!def->emulator) {
> + virBufferAddLit(buf, "/>\n");
> + } else {
> + virBufferEscapeString(buf, ">%s</emulator>\n",
> + def->emulator);
> + }
> + }
>
> for (n = 0; n < def->ndisks; n++)
> if (virDomainDiskDefFormat(buf, def->disks[n], flags) < 0)
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 28c6920..38b9037 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1918,6 +1918,13 @@ struct _virBlkioDevice {
> };
>
> typedef enum {
> + VIR_DOMAIN_EMULATOR_TYPE_DEFAULT,
> + VIR_DOMAIN_EMULATOR_TYPE_STUBDOM,
> +
> + VIR_DOMAIN_EMULATOR_TYPE_LAST
> +} virDomainEmulatorType;
> +
> +typedef enum {
> VIR_DOMAIN_RNG_MODEL_VIRTIO,
>
> VIR_DOMAIN_RNG_MODEL_LAST
> @@ -2083,6 +2090,7 @@ struct _virDomainDef {
>
> virDomainOSDef os;
> char *emulator;
> + virDomainEmulatorType emulator_type;
> /* These three options are of type virTristateSwitch,
> * except VIR_DOMAIN_FEATURE_CAPABILITIES that is of type
> * virDomainCapabilitiesPolicy */
> @@ -2841,6 +2849,7 @@ VIR_ENUM_DECL(virDomainGraphicsSpiceMouseMode)
> VIR_ENUM_DECL(virDomainGraphicsVNCSharePolicy)
> VIR_ENUM_DECL(virDomainHyperv)
> VIR_ENUM_DECL(virDomainKVM)
> +VIR_ENUM_DECL(virDomainEmulatorType)
> VIR_ENUM_DECL(virDomainRNGModel)
> VIR_ENUM_DECL(virDomainRNGBackend)
> VIR_ENUM_DECL(virDomainTPMModel)
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index b1131ea..a84ce09 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -768,6 +768,23 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> b_info->device_model_version = libxlDomainGetEmulatorType(def);
> }
>
> + /* In case of stubdom there will be two qemu instances:
> + * - in stubdom (libxl uses hardcoded path for this one),
> + * - in dom0 as a backend for stubdom (if needed).
>
Your comment here provoked my thinking above about supporting multiple
<emulator>s. ISTM there should be an <emulator> device for each of
these qemu instances.
> + * Emulator path control only the second one. It makes a perfect sense
> + * to use <emulator type='stubdom'/> (yes, without emulator
path).
>
I'm not so sure. Maybe we should first add support in libxl for
specifying a stubdomain emulator path.
Yes, this would be a good idea (especially when someone implements
alternative stubdomain emulator). But Xen <=4.5 still should be somehow
handled - I think it can be plain <emulator mode='stubdomain'/> (without
a path). Possibly rejecting a path if not supported by underlying libxl.
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?