[libvirt] [PATCH 4/6] qemu: Wire up disk model=virtio-{non-}transitional

[Daniel P. Berrangé]

On Wed, Jan 16, 2019 at 12:31:04PM +0100, Andrea Bolognani wrote:
> On Tue, 2019-01-15 at 16:56 +0000, Daniel P. Berrangé wrote:
> > On Sun, Jan 13, 2019 at 06:12:06PM -0500, Cole Robinson wrote:
> > > Add new <disk> model values for virtio transitional devices. When
> > > combined with bus='virtio':
> > > 
> > > * "virtio-transitional" maps to qemu "virtio-blk-pci-transitional"
> > > * "virtio-non-transitional" maps to qemu "virtio-blk-pci-non-transitional"
> > > 
> > > Signed-off-by: Cole Robinson <crobinso at redhat.com>
> > > ---
> > >  src/qemu/qemu_command.c                       | 31 ++++++++++++++++++-
> > >  src/qemu/qemu_domain_address.c                |  2 ++
> > >  ...virtio-non-transitional.x86_64-latest.args |  7 +++--
> > >  .../virtio-transitional.x86_64-latest.args    |  4 +--
> > >  .../virtio-non-transitional.xml               | 10 ++++--
> > >  5 files changed, 45 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > > index 822d5f8669..ca6abea227 100644
> > > --- a/src/qemu/qemu_command.c
> > > +++ b/src/qemu/qemu_command.c
> > > @@ -443,6 +443,33 @@ qemuBuildVirtioDevStr(virBufferPtr buf,
> > >      return 0;
> > >  }
> > >  
> > > +static int
> > > +qemuBuildVirtioTransitional(virBufferPtr buf,
> > > +                            const char *baseName,
> > > +                            virDomainDeviceAddressType type,
> > > +                            bool transitional,
> > > +                            bool nontransitional)
> > > +{
> > > +    if (qemuBuildVirtioDevStr(buf, baseName, type) < 0)
> > > +        return -1;
> > > +
> > > +    if (type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
> > > +        (transitional || nontransitional)) {
> > > +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > > +                       _("virtio transitional models are not supported "
> > > +                         "for address type=%s"),
> > > +                       virDomainDeviceAddressTypeToString(type));
> > > +        return -1;
> > > +    }
> > > +
> > > +    if (transitional) {
> > > +        virBufferAddLit(buf, "-transitional");
> > > +    } else if (nontransitional) {
> > > +        virBufferAddLit(buf, "-non-transitional");
> > > +    }
> > > +    return 0;
> > 
> > So this only works on QEMU >= 4.0.0 - earlier versions will
> > fail to start.
> > 
> > We can, however, make it work correctly with old QEMU.
> > 
> > A transitional device is 100% identical to the existing device
> > types, so we can simply not add the "-transitional" suffix for
> > old QEMU. The only difference is the way libvirt does PCI bus
> > placement of the transitional device - we'd never use PCIe.
> > 
> > A non-transitional device is identical to the existing device
> > types, but with  disable-legacy=true set.
> 
> Again, the relationship between existing and new devices is not
> quite this straighforward because of the reasons I outlined in
> 
>   https://www.redhat.com/archives/libvir-list/2019-January/msg00514.html

When told to use virtio-transitional for a device, libvirt would
only plug it into a PCI slot, never a PCI-X slot. Given this
constraint, it is functionally identical / interchangable with
the existing device.

> But the idea of using disable-{legacy,modern} instead of the new
> virtio-*-{non,}-transitional devices is one I had already suggested
> in
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=1614127
> 
> so I'm obviously on board with it :)
> 
> > QEMU guarantees this compatibility of the different devices,
> > but only for machine types < pc-i440fx-4.0.0 / pc-q35-4.0.0.
> > So we should none the less make sure we use the modern device
> > names for any QEMU which genuinely supports them.
> 
> As I already mentioned in the bug report linked above, I'm not
> quite convinced that's the case, and I don't see why we wouldn't
> just use the options and basically ignore the QEMU-level devices,
> as the former approach would work on old QEMU releases as well as
> recent ones with no drawback I can think of.

The QEMU maintainers were against the idea of us doing that. In the
future they may add properties to, or change the defaults on, the
-transitional or -non-transitional devices only, associated with
new machine type versions. If libvirt forever uses the old devices,
then we loose ability to take advantage of that.

Indeed if QEMU maintainers wanted us to use the disable-legacy/modern
features long term, there would be no point in them even adding the
new device types in the first place.

We should only ever use the disable- flags if the new devices do
not exist in QEMU.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|