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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]