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@xxxxxxxxxxxxxxxxxxxxxx>
> > ---
> >
> > 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?
Attachment:
pgpSAAEVTFh1r.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list