Re: [libvirt] [PATCH 09/10] libxl: Stubdom emulator type

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>

> Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com&gt; > --- > > 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?

> 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.