[libvirt] [PATCH] Add new 'kvm' domain feature and ability to hide KVM signature

[Cole Robinson]

On 08/19/2014 03:36 PM, Alex Williamson wrote:
> On Tue, 2014-08-19 at 12:40 -0400, Cole Robinson wrote:
>> On 08/15/2014 12:32 PM, Alex Williamson wrote:
>>> QEMU 2.1 added support for the kvm=off option to the -cpu command,
>>> allowing the KVM hypervisor signature to be hidden from the guest.
>>> This enables disabling of some paravirualization features in the
>>> guest as well as allowing certain drivers which test for the
>>> hypervisor to load.  Domain XML syntax is as follows:
>>>
>>> <domain type='kvm>
>>>   ...
>>>   <features>
>>>     ...
>>>     <kvm>
>>>       <hidden state='on'/>
>>>     </kvm>
>>>   </features>
>>>   ...
>>>
>>> Signed-off-by: Alex Williamson <alex.williamson at redhat.com>
>>> ---
>>>
>>> If it's not obvious, this patch is derived from copying and modifying
>>> the similar hyperv feature support.  Hopefully I've found all the
>>> right pieces.
>>>
>>
>> Seems to cover all the bases. couple minor bits:
>>
>>>  docs/formatdomain.html.in                          |   21 ++++
>>>  docs/schemas/domaincommon.rng                      |   18 +++-
>>>  src/conf/domain_conf.c                             |  100 ++++++++++++++++++++
>>>  src/conf/domain_conf.h                             |    9 ++
>>>  src/qemu/qemu_command.c                            |   22 ++++
>>>  tests/qemuargv2xmltest.c                           |    2 
>>>  .../qemuxml2argv-kvm-features-off.args             |    5 +
>>>  .../qemuxml2argv-kvm-features-off.xml              |   27 +++++
>>>  .../qemuxml2argv-kvm-features.args                 |    5 +
>>>  .../qemuxml2argvdata/qemuxml2argv-kvm-features.xml |   27 +++++
>>>  tests/qemuxml2argvtest.c                           |    3 +
>>>  tests/qemuxml2xmltest.c                            |    3 +
>>>  12 files changed, 240 insertions(+), 2 deletions(-)
>>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm-features-off.args
>>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm-features-off.xml
>>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm-features.args
>>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm-features.xml
>>>
>>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>>> index bd99ae0..32cc381 100644
>>> --- a/docs/formatdomain.html.in
>>> +++ b/docs/formatdomain.html.in
>>> @@ -1232,6 +1232,9 @@
>>>        <vapic state='on'/>
>>>        <spinlocks state='on' retries='4096'/>
>>>      </hyperv>
>>> +    <kvm>
>>> +      <hidden state='on'/>
>>> +    </kvm>
>>>      <pvspinlock/>
>>>  
>>>    </features>
>>> @@ -1310,7 +1313,23 @@
>>>            can be explicitly disabled by using <code>state='off'</code>
>>>            attribute.
>>>        </dd>
>>> -
>>> +      <dt><code>kvm</code></dt>
>>> +      <dd>Various features to change the behavior of the KVM hypervisor.
>>> +      <table class="top_table">
>>> +        <tr>
>>> +          <th>Feature</th>
>>> +          <th>Description</th>
>>> +          <th>Value</th>
>>> +          <th>Since</th>
>>> +        </tr>
>>> +        <tr>
>>> +          <td>hidden</td>
>>> +          <td>Hide the KVM hypervisor from standard MSR based discovery</td>
>>> +          <td> on, off</td>
>>> +          <td><span class="since">2.1.0 (QEMU only)</span></td>
>>> +        </tr>
>>> +      </table>
>>> +      </dd>
>>>      </dl>
>>>  
>>
>> I'd specify that the default value is 'off' if using KVM, 'on' otherwise, and
>> can be explicitly disabled with 'on'.
> 
> At the beginning of this section we specify:
> 
>         All features are listed within the features element, omitting a
>         togglable feature tag turns it off.
> 
> Our feature is whether to hide or not hide the KVM hypervisor signature.
> Note that 'not hide' does not imply expose, IMO.  The default per the
> section scope is 'off'.  Without KVM acceleration, there is no KVM
> hypervisor signature to hide, so the value is meaningless and QEMU
> accepts, but does not change behavior based on this.  With KVM, there is
> of course a signature that can be hidden.
> 
> Do you think there's still something to change here?

That's sufficient for me, thanks.

- Cole