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

Re: [libvirt] [PATCH 3/3] conf: formatter/parser/RNG/docs for hostdev <driver name='kvm|vfio'/>



On 04/23/2013 10:45 AM, Laine Stump wrote:
> A domain's <interface> or <hostdev>, as well as a <network>'s
> <forward>, can now have an optional <driver name='kvm|vfio'/>
> element. As of this patch, there is no functionality behind this new
> knob - this patch adds support to the domain and network
> formatter/parser, and to the RNG and documentation.
> 
> When then backend is added, legacy KVM PCI device assignment will

s/then/the/

> continue to be used when no driver name is specified (or if <driver
> name='kvm'/> is specified), but if driver name is 'vfio', the new UEFI
> Secure Boot compatible VFIO device assignment will be used.
> 
> Note that the parser doesn't automatically insert the current default
> value of this setting. This is done on purpose because the two
> possibilities are functionally equivalent from the guest's point of
> view, and we want to be able to automatically start using vfio as the
> default (even for existing domains) at some time in the future. This
> is similar to what was done with the "vhost" driver option in
> <interface>.

Yes, leaving an unspecified element as hypervisor default has worked for
us in the past, especially when the choice of host driver should not be
guest-visible.

> ---
>  docs/formatdomain.html.in     | 42 ++++++++++++++++++++++-
>  docs/formatnetwork.html.in    | 15 ++++++++
>  docs/schemas/domaincommon.rng | 79 ++++++++++++++++++++++++++++---------------
>  docs/schemas/network.rng      | 11 ++++++
>  src/conf/domain_conf.c        | 36 ++++++++++++++++++++
>  src/conf/domain_conf.h        | 11 ++++++
>  6 files changed, 166 insertions(+), 28 deletions(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 888c005..0e5b00c 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2340,7 +2340,22 @@
>        the device as can be found with the <code>lspci</code> or
>        with <code>virsh
>        nodedev-list</code>. <a href="#elementsAddress">See above</a> for
> -      more details on the address element.
> +      more details on the address element.</dd>
> +      <dt><code>driver</code></dt>
> +      <dd>
> +        PCI devices can have an optional <code>driver</code>
> +        subelement that specifies which backend driver to use for PCI
> +        device assignment. Use the <code>name</code> attribute to
> +        select either "vfio" (for the new VFIO device assignment
> +        backend, which is compatible with UEFI SecureBoot) or "kvm"
> +        (for the legacy device assignment handled directly by the KVM
> +        kernel module)<span class="since">Since 1.0.5 (QEMU and KVM
> +        only, requires kernel 3.6 or newer)</span>. Currently, "kvm"
> +        is the default used by libvirt when not explicitly provided,
> +        but since the two are functionally equivalent, this default
> +        could be changed in the future with no impact to domains that
> +        don't specify anything.

IIRC, we have talked about managing host devices across all hypervisors,
since it is a shared host resource. If that's true, then what happens if
LXC wants to start using a driver?  Or should LXC only use vfio and
never use kvm?  Also, didn't you say that xen used yet another driver,
in which case we should consider listing that in the RNG.


> +        device assignment performed directly by the kvm kernel module
> +        (the default is currently "kvm", but is subject to change).
> +        <span class="since">Since 1.0.5 (QEMU and KVM only, requires kernel 3.6 or newer)</span>

Long line (several times).


> +++ b/docs/schemas/domaincommon.rng
> @@ -1923,28 +1923,40 @@
>        </optional>
>        <optional>
>          <element name="driver">
> -          <optional>
> -            <attribute name="name">
> -              <choice>
> -                <value>qemu</value>
> -                <value>vhost</value>
> -              </choice>
> -            </attribute>
> -          </optional>
> -          <optional>
> -            <attribute name="txmode">
> -              <choice>
> -                <value>iothread</value>
> -                <value>timer</value>
> -              </choice>
> -            </attribute>
> -          </optional>
> -          <optional>
> -            <ref name="ioeventfd"/>
> -          </optional>
> -          <optional>
> -            <ref name="event_idx"/>
> -          </optional>
> +          <choice>
> +            <group>
> +              <attribute name="name">
> +                <choice>
> +                  <value>kvm</value>
> +                  <value>vfio</value>
> +                </choice>
> +              </attribute>
> +            </group>
> +            <group>
> +              <optional>
> +                <attribute name="name">
> +                  <choice>
> +                    <value>qemu</value>
> +                    <value>vhost</value>

So for define='interface-options', we are extending the exiting <driver>
so that there are now four values for <driver name=.../>, but only the
two older options allow other attributes.

Can the two ever overlap, or are we safe in that kvm/vfio are for
hostdev interface passthrough, while qemu/vhost is for emulation, so the
two types of driver choices never make sense at once?

> +                  </choice>
> +                </attribute>
> +              </optional>
> +              <optional>
> +                <attribute name="txmode">
> +                  <choice>
> +                    <value>iothread</value>
> +                    <value>timer</value>
> +                  </choice>
> +                </attribute>
> +              </optional>
> +              <optional>
> +                <ref name="ioeventfd"/>
> +              </optional>
> +              <optional>
> +                <ref name="event_idx"/>
> +              </optional>
> +            </group>
> +          </choice>
>            <empty/>
>          </element>
>        </optional>
> @@ -3084,14 +3096,27 @@
>      <attribute name="type">
>        <value>pci</value>
>      </attribute>
> -    <element name="source">
> +    <interleave>
>        <optional>
> -        <ref name="startupPolicy"/>
> +        <element name="driver">
> +          <attribute name="name">
> +            <choice>
> +              <value>kvm</value>
> +              <value>vfio</value>
> +            </choice>
> +          </attribute>
> +          <empty/>

Whereas for define='hostdevsubsyspci', we are completely adding a new
subelement, with only two choices for the new attribute.

> +        </element>
>        </optional>
> -      <element name="address">
> -        <ref name="pciaddress"/>
> +      <element name="source">
> +        <optional>
> +          <ref name="startupPolicy"/>
> +        </optional>
> +        <element name="address">
> +          <ref name="pciaddress"/>
> +        </element>
>        </element>
> -    </element>
> +    </interleave>
>    </define>
>  
>    <define name="hostdevsubsysusb">
> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
> index 6c3fae2..493edae 100644
> --- a/docs/schemas/network.rng
> +++ b/docs/schemas/network.rng
> @@ -149,6 +149,17 @@
>                    </attribute>
>                  </element>
>                </optional>
> +              <optional>
> +                <element name="driver">
> +                  <attribute name="name">
> +                    <choice>
> +                      <value>kvm</value>
> +                      <value>vfio</value>
> +                    </choice>
> +                  </attribute>
> +                  <empty/>
> +                </element>
> +              </optional>
>              </interleave>

Looks okay to me.

I'm not sure if we are ready to push this without more patches in the
series, but at least I think the XML is on the right track.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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