[libvirt] [PATCH] docs: formatdomain: Document the CPU feature 'name' attribute

Kashyap Chamarthy posted 1 patch 6 years, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180112193116.21477-1-kchamart@redhat.com
There is a newer version of this series
docs/formatdomain.html.in | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
[libvirt] [PATCH] docs: formatdomain: Document the CPU feature 'name' attribute
Posted by Kashyap Chamarthy 6 years, 3 months ago
Currently, the CPU feature 'name' XML attribute, as in:

    [...]
    <cpu match='exact'>
      <model fallback='forbid'>IvyBridge</model>
      <vendor>Intel</vendor>
      <feature policy='require' name='pcid'/>
    </cpu>
    [...]

isn't explicitly documented in formatdomain.html.

Document it now.

Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
---
 docs/formatdomain.html.in | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index d272cc1ba..e717fb3aa 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1454,6 +1454,23 @@
 
         <span class="since">Since 0.8.5</span> the <code>policy</code>
         attribute can be omitted and will default to <code>require</code>.
+
+        Individual CPU feature names can be specified as part of the
+        <code>name</code> attribute. The list of known CPU feature
+        names (e.g. 'vmx', 'cmt', et cetera) can be found in the same
+        file as CPU models -- <code>cpu_map.xml</code>. For example,
+        to explicitly specify the 'pcid' feature with Intel IvyBridge
+        CPU model:
+
+<pre>
+...
+&lt;cpu match='exact'&gt;
+  &lt;model fallback='forbid'&gt;IvyBridge&lt;/model&gt;
+  &lt;vendor&gt;Intel&lt;/vendor&gt;
+  &lt;feature policy='require' name='pcid'/&gt;
+&lt;/cpu&gt;
+...</pre>
+
       </dd>
 
       <dt><code>cache</code></dt>
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: formatdomain: Document the CPU feature 'name' attribute
Posted by Eduardo Habkost 6 years, 3 months ago
On Fri, Jan 12, 2018 at 08:31:16PM +0100, Kashyap Chamarthy wrote:
> Currently, the CPU feature 'name' XML attribute, as in:
> 
>     [...]
>     <cpu match='exact'>
>       <model fallback='forbid'>IvyBridge</model>
>       <vendor>Intel</vendor>
>       <feature policy='require' name='pcid'/>
>     </cpu>
>     [...]
> 
> isn't explicitly documented in formatdomain.html.
> 
> Document it now.
> 
> Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
> ---
>  docs/formatdomain.html.in | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index d272cc1ba..e717fb3aa 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1454,6 +1454,23 @@
>  
>          <span class="since">Since 0.8.5</span> the <code>policy</code>
>          attribute can be omitted and will default to <code>require</code>.
> +
> +        Individual CPU feature names can be specified as part of the
> +        <code>name</code> attribute.

Isn't this "should" instead of "can"?  Does it make sense to have
a 'feature' element without a 'name' attribute?


>                                       The list of known CPU feature
> +        names (e.g. 'vmx', 'cmt', et cetera) can be found in the same
> +        file as CPU models -- <code>cpu_map.xml</code>. For example,
> +        to explicitly specify the 'pcid' feature with Intel IvyBridge
> +        CPU model:

Another paragraph above already says "The list of known feature
names can be found in the same file as CPU models".  If you think
the existing paragraph is not enough, I suggest rewriting it so
the document won't repeat exactly the same thing.

> +
> +<pre>
> +...
> +&lt;cpu match='exact'&gt;
> +  &lt;model fallback='forbid'&gt;IvyBridge&lt;/model&gt;
> +  &lt;vendor&gt;Intel&lt;/vendor&gt;
> +  &lt;feature policy='require' name='pcid'/&gt;
> +&lt;/cpu&gt;
> +...</pre>
> +
>        </dd>
>  
>        <dt><code>cache</code></dt>
> -- 
> 2.13.6
> 

-- 
Eduardo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: formatdomain: Document the CPU feature 'name' attribute
Posted by Kashyap Chamarthy 6 years, 3 months ago
On Tue, Jan 16, 2018 at 03:35:20PM -0200, Eduardo Habkost wrote:
> On Fri, Jan 12, 2018 at 08:31:16PM +0100, Kashyap Chamarthy wrote:
> > Currently, the CPU feature 'name' XML attribute, as in:

[...]

> > ---
> >  docs/formatdomain.html.in | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> > index d272cc1ba..e717fb3aa 100644
> > --- a/docs/formatdomain.html.in
> > +++ b/docs/formatdomain.html.in
> > @@ -1454,6 +1454,23 @@
> >  
> >          <span class="since">Since 0.8.5</span> the <code>policy</code>
> >          attribute can be omitted and will default to <code>require</code>.
> > +
> > +        Individual CPU feature names can be specified as part of the
> > +        <code>name</code> attribute.
> 
> Isn't this "should" instead of "can"?  Does it make sense to have
> a 'feature' element without a 'name' attribute?

Good catch.  Near as I see, it doesn't.  So I'll: s/can/should.

> 
> >                                       The list of known CPU feature
> > +        names (e.g. 'vmx', 'cmt', et cetera) can be found in the same
> > +        file as CPU models -- <code>cpu_map.xml</code>. For example,
> > +        to explicitly specify the 'pcid' feature with Intel IvyBridge
> > +        CPU model:
> 
> Another paragraph above already says "The list of known feature
> names can be found in the same file as CPU models".   If you think the
> existing paragraph is not enough, I suggest rewriting it so the
> document won't repeat exactly the same thing.

True.  How about this rewrite: 

    "Once you choose a feature (e.g. 'pcid') from the `cpu_map.xml`, to
    specify it explicitly with the Intel IvyBridge CPU model [...]"

I'll consider whether to also add a note that before specifying extra
CPU feature flags, one should check if the named CPU models provided by
libvirt already include the said flags.

[...]

-- 
/kashyap

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: formatdomain: Document the CPU feature 'name' attribute
Posted by Eduardo Habkost 6 years, 3 months ago
On Wed, Jan 17, 2018 at 09:04:26AM +0100, Kashyap Chamarthy wrote:
> On Tue, Jan 16, 2018 at 03:35:20PM -0200, Eduardo Habkost wrote:
> > On Fri, Jan 12, 2018 at 08:31:16PM +0100, Kashyap Chamarthy wrote:
> > > Currently, the CPU feature 'name' XML attribute, as in:
> 
> [...]
> 
> > > ---
> > >  docs/formatdomain.html.in | 17 +++++++++++++++++
> > >  1 file changed, 17 insertions(+)
> > > 
> > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> > > index d272cc1ba..e717fb3aa 100644
> > > --- a/docs/formatdomain.html.in
> > > +++ b/docs/formatdomain.html.in
> > > @@ -1454,6 +1454,23 @@
> > >  
> > >          <span class="since">Since 0.8.5</span> the <code>policy</code>
> > >          attribute can be omitted and will default to <code>require</code>.
> > > +
> > > +        Individual CPU feature names can be specified as part of the
> > > +        <code>name</code> attribute.
> > 
> > Isn't this "should" instead of "can"?  Does it make sense to have
> > a 'feature' element without a 'name' attribute?
> 
> Good catch.  Near as I see, it doesn't.  So I'll: s/can/should.
> 
> > 
> > >                                       The list of known CPU feature
> > > +        names (e.g. 'vmx', 'cmt', et cetera) can be found in the same
> > > +        file as CPU models -- <code>cpu_map.xml</code>. For example,
> > > +        to explicitly specify the 'pcid' feature with Intel IvyBridge
> > > +        CPU model:
> > 
> > Another paragraph above already says "The list of known feature
> > names can be found in the same file as CPU models".   If you think the
> > existing paragraph is not enough, I suggest rewriting it so the
> > document won't repeat exactly the same thing.
> 
> True.  How about this rewrite: 
> 
>     "Once you choose a feature (e.g. 'pcid') from the `cpu_map.xml`, to
>     specify it explicitly with the Intel IvyBridge CPU model [...]"

"Once you choose a feature (e.g. 'pcid') from the `cpu_map.xml`"
doesn't seem to convey any additional information that wasn't
mentioned before.  What about just "For example, to explicitly
specify the 'pcid' feature with Intel IvyBridge CPU model:"?

> 
> I'll consider whether to also add a note that before specifying extra
> CPU feature flags, one should check if the named CPU models provided by
> libvirt already include the said flags.

Maybe this would be too much information.  It's harmless to set a feature
explicitly to 'require' if the CPU model already contains the feature.

-- 
Eduardo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: formatdomain: Document the CPU feature 'name' attribute
Posted by Kashyap Chamarthy 6 years, 3 months ago
On Mon, Jan 22, 2018 at 04:43:35PM -0200, Eduardo Habkost wrote:
> On Wed, Jan 17, 2018 at 09:04:26AM +0100, Kashyap Chamarthy wrote:

[...]

> > > >                                       The list of known CPU feature
> > > > +        names (e.g. 'vmx', 'cmt', et cetera) can be found in the same
> > > > +        file as CPU models -- <code>cpu_map.xml</code>. For example,
> > > > +        to explicitly specify the 'pcid' feature with Intel IvyBridge
> > > > +        CPU model:
> > > 
> > > Another paragraph above already says "The list of known feature
> > > names can be found in the same file as CPU models".   If you think the
> > > existing paragraph is not enough, I suggest rewriting it so the
> > > document won't repeat exactly the same thing.
> > 
> > True.  How about this rewrite: 
> > 
> >     "Once you choose a feature (e.g. 'pcid') from the `cpu_map.xml`, to
> >     specify it explicitly with the Intel IvyBridge CPU model [...]"
> 
> "Once you choose a feature (e.g. 'pcid') from the `cpu_map.xml`"
> doesn't seem to convey any additional information that wasn't
> mentioned before.  What about just "For example, to explicitly
> specify the 'pcid' feature with Intel IvyBridge CPU model:"?

I was just trying to be explicit.  But yours sound clearer too -- so can
update to yours.

> > 
> > I'll consider whether to also add a note that before specifying extra
> > CPU feature flags, one should check if the named CPU models provided by
> > libvirt already include the said flags.
> 
> Maybe this would be too much information.  It's harmless to set a feature
> explicitly to 'require' if the CPU model already contains the feature.

Okay, can remove it.  (I was just operating under the principle of
"explicit is better than implicit".)

Thanks for the review.

-- 
/kashyap

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list