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

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(a)redhat.com&gt; >> --- >> >> 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 @@ >> &lt;vapic state='on'/&gt; >> &lt;spinlocks state='on' retries='4096'/&gt; >> &lt;/hyperv&gt; >> + &lt;kvm&gt; >> + &lt;hidden state='on'/&gt; >> + &lt;/kvm&gt; >> &lt;pvspinlock/&gt; >> >> &lt;/features&gt; >> @@ -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?