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

Re: [libvirt] [PATCH 1/2] XML schema for CPU flags



On Tue, Nov 03, 2009 at 04:40:00PM +0100, Jiri Denemark wrote:
> Firstly, CPU topology and model with optional features have to be
> advertised in host capabilities:
> 
>     <host>
>         <cpu>
>             <arch>ARCHITECTURE</arch>
>             <features>
>                 <!-- old-style features are here -->
>             </features>
>             <model>NAME</model>
>             <topology sockets="S" cores="C" threads="T"/>
>             <feature>NAME</feature>
>         </cpu>
>         ...
>     </host>
> 
> Secondly, drivers which support detailed CPU specification have to advertise
> it in guest capabilities:
> 
>     <guest>
>         ...
>         <features>
>             <cpu/>
>         </features>
>     </guest>

  maybe <cpu/> is a bit too generic. <cpuselection/> or something similar
might be a bit better, for example if we want to extend that later to
give there a list of the supported CPU names for example <cpu> would
be handy.

> And finally, CPU may be configured in domain XML configuration:
> 
> <domain>
>     ...
>     <cpu match="MATCH">
>         <model>NAME</model>
>         <topology sockets="S" cores="C" threads="T"/>
>         <feature policy="POLICY">NAME</feature>
>     </cpu>
> </domain>

  I wonder if I don't prefer to have the name in an attribute since
we're going with a single feature at a time.

> Where MATCH can be one of:
>     - 'minimum'     specified CPU is the minimum requested CPU
>     - 'exact'       disable all additional features provided by host CPU
>     - 'strict'      fail if host CPU doesn't exactly match
> 
> POLICY can be one of:
>     - 'force'       turn on the feature, even if host doesn't have it
>     - 'require'     fail if host doesn't have the feature
>     - 'optional'    match host
>     - 'disable'     turn off the feature, even if host has it
>     - 'forbid'      fail if host has the feature
> 
> 'force' and 'disable' policies turn on/off the feature regardless of its
> availability on host. 'force' is unlikely to be used but its there for
> completeness since Xen and VMWare allow it.
> 
> 'require' and 'forbid' policies prevent a guest from being started on a host
> which doesn't/does have the feature. 'forbid' is for cases where you disable
> the feature but a guest may still try to access it anyway and you don't want
> it to succeed.
> 
> 'optional' policy sets the feature according to its availability on host.
> When a guest is booted on a host that has the feature and then migrated to
> another host, the policy changes to 'require' as we can't take the feature
> away from a running guest.
> 
> Default policy for features provided by host CPU but not specified in domain
> configuration is set using match attribute of cpu tag. If 'minimum' match is
> requested, additional features will be treated as if they were specified
> with 'optional' policy. 'exact' match implies 'disable' policy and 'strict'
> match stands for 'forbid' policy.

  Okay, I think this reflects the consensus from previous discussions
about this. IMHO the nasty part is the set of feature names which
is undefined and unbounded at the moment.

> diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
> index 3e8944c..faeff4d 100644
> --- a/docs/schemas/capability.rng
> +++ b/docs/schemas/capability.rng
> @@ -25,6 +25,9 @@
>  	<optional>
>  	  <ref name='cpufeatures'/>
>  	</optional>
> +        <optional>
> +          <ref name='cpuspec'/>
> +        </optional>
>        </element>
>        <optional>
>  	<ref name='migration'/>

  One thing to note is that the order of the elements here is fixed
they must be present under <cpu> in the exact order given by the schemas
(fine by me).

> @@ -67,6 +70,28 @@
>      </element>
>    </define>
>  
> +  <define name='cpuspec'>
> +    <element name='model'>
> +      <text/>
> +    </element>
> +    <element name='topology'>
> +      <attribute name='sockets'>
> +        <ref name='uint'/>
> +      </attribute>
> +      <attribute name='cores'>
> +        <ref name='uint'/>
> +      </attribute>
> +      <attribute name='threads'>
> +        <ref name='uint'/>
> +      </attribute>

  Maybe we want provide a new type excluding 0 here.
by adding at the end

  <define name='positiveInteger'>
    <data type='positiveInteger'/>
  </define>

referencing http://www.w3.org/TR/xmlschema-2/#positiveInteger
since the grammar references
 datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes";

similary:

  <define name='uint'>
    <data type='unsignedInt'/>
  </define>

would be nicer than the current definition.

> +    </element>
> +    <zeroOrMore>
> +      <element name='feature'>
> +        <text/>
> +      </element>
> +    </zeroOrMore>
> +  </define>
> +
>    <define name='migration'>
>      <element name='migration_features'>
>        <optional>
> @@ -259,6 +284,11 @@
>  	  <empty/>
>  	</element>
>        </optional>
> +      <optional>
> +        <element name='cpu'>

   I would go for <element name='cpuselection'> or similar here

> +          <empty/>
> +        </element>
> +      </optional>
>      </element>
>    </define>
>  
> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
> index 0a6ab61..eb91572 100644
> --- a/docs/schemas/domain.rng
> +++ b/docs/schemas/domain.rng
> @@ -38,6 +38,9 @@
>          <optional>
>            <ref name="seclabel"/>
>          </optional>
> +        <optional>
> +          <ref name="cpu"/>
> +        </optional>
>        </interleave>
>      </element>
>    </define>

  even though we are in an interleave, i.e. the order of elements under
<domain> is not constrained, I would add the optional reference higher,
it's more sensible to see those constraints after description or before
<os>, so I would move that block a little up, even if this doesn't
change the semantic...

<note>Damn I'm pedantic this morning ...</note>

> @@ -1193,6 +1196,52 @@
>      </optional>
>    </define>
>    <!--
> +      CPU specification
> +      -->
> +  <define name="cpu">
> +    <element name="cpu">
> +      <attribute name="match">
> +        <choice>
> +          <value>minimum</value>
> +          <value>exact</value>
> +          <value>strict</value>
> +        </choice>
> +      </attribute>
> +      <interleave>
> +        <element name="model">
> +          <text/>
> +        </element>
> +        <optional>
> +          <element name="topology">
> +            <attribute name="sockets">
> +              <ref name="uint"/>
> +            </attribute>
> +            <attribute name="cores">
> +              <ref name="uint"/>
> +            </attribute>
> +            <attribute name="threads">
> +              <ref name="uint"/>

  similar we may want a positiveInteger here with a similar definition
as 0 makes no sense for any of those values.

> +            </attribute>
> +          </element>
> +        </optional>
> +        <zeroOrMore>
> +          <element name="feature">
> +            <attribute name="policy">
> +              <choice>
> +                <value>force</value>
> +                <value>require</value>
> +                <value>optional</value>
> +                <value>disable</value>
> +                <value>forbid</value>
> +              </choice>
> +            </attribute>

> +            <text/>

    sight text is completely unconstrainted... And somehow I think
I would prefer to have
               <attribute name="name">
                 <text/>
               </attribute>

and keep feature content empty for the moment, which would allow more
easilly to evolve the format for something more complex if the needs
arise.

> +          </element>
> +        </zeroOrMore>
> +      </interleave>
> +    </element>
> +  </define>
> +  <!--
>         Type library
>  
>         Our unsignedInt doesn't allow a leading '+' in its lexical form

  Sounds good to me, a couple of suggestions on the format, naming
and stucture, plus optional improvements on the types used.

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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