When virtio-(non-)transitional models were introduced, the
documentation was updated to include them; at the same time,
language was introduced indicating that using the existing
virtio model is no longer recommended.

This is unnecessarily harsh, and has resulted in people
incorrectly believing (through no fault of their own) that the
virtio model has been deprecated.

In reality, it's perfectly fine to use the virtio model as the
stress-free option that, while often not producing the ideal
PCI topology, will generally get the job done and work reliably
across libvirt versions and machine types.

Tweak the documentation so that it hopefully carries the
desired message across.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 docs/formatdomain.rst | 55 ++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 27 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index c50744b57b..75dff5a153 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2756,8 +2756,8 @@ paravirtualized driver is specified via the ``disk`` element.
    ``model``
       Indicates the emulated device model of the disk. Typically this is
       indicated solely by the ``bus`` property but for ``bus`` "virtio" the
-      model can be specified further with "virtio-transitional",
-      "virtio-non-transitional", or "virtio". See `Virtio transitional devices`_
+      model can be specified further with "virtio", "virtio-transitional" or
+      "virtio-non-transitional". See `virtio device models`_
       for more details. :since:`Since 5.2.0`
    ``rawio``
       Indicates whether the disk needs rawio capability. Valid settings are
@@ -3680,9 +3680,8 @@ A directory on the host that can be accessed directly from the guest.
       info <https://lists.gnu.org/archive/html/qemu-devel/2010-09/msg00121.html>`__
 
    :since:`Since 5.2.0`, the filesystem element has an optional attribute
-   ``model`` with supported values "virtio-transitional",
-   "virtio-non-transitional", or "virtio". See `Virtio transitional devices`_
-   for more details.
+   ``model`` with supported values "virtio", "virtio-transitional" or
+   "virtio-non-transitional". See `virtio device models`_ for more details.
 
    The filesystem element has optional attributes ``fmode`` and ``dmode``.
    These two attributes control the creation mode for files and directories
@@ -3910,11 +3909,20 @@ Note: In general you should leave this option alone, unless you are very certain
 you know what you are doing.
 
 
-Virtio transitional devices
-~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Virtio device models
+~~~~~~~~~~~~~~~~~~~~
+
+Virtio devices come in several variants, some of which are only applicable to
+certain machine types or scenarios. The variant can be chosen via the ``model``
+attribute, which supports the following values:
 
-:since:`Since 5.2.0`, some of QEMU's virtio devices, when used with PCI/PCIe
-machine types, accept the following ``model`` values:
+``virtio``
+   This is the recommended choice in the absence of guest OS specific
+   constraints, as it will will generally work correctly across a large range
+   of architectures, machine types and libvirt versions.
+
+:since:`Since 5.2.0`, the following values can additionally be used with machine
+types based on PCI (either conventional PCI or PCI Express):
 
 ``virtio-transitional``
    This device can work both with virtio 0.9 and virtio 1.0 guest drivers, so
@@ -3926,12 +3934,6 @@ machine types, accept the following ``model`` values:
    necessary. libvirt will plug the device into either a PCI Express slot or a
    conventional PCI slot based on the machine type, resulting in a more
    optimized PCI topology.
-``virtio``
-   This device will work like a ``virtio-non-transitional`` device when plugged
-   into a PCI Express slot, and like a ``virtio-transitional`` device otherwise;
-   libvirt will pick one or the other based on the machine type. This is the
-   best choice when compatibility with libvirt versions older than 5.2.0 is
-   necessary, but it's otherwise not recommended to use it.
 
 While the information outlined above applies to most virtio devices, there are a
 few exceptions:
@@ -3992,14 +3994,14 @@ specific features, such as:
    The ``virtio-serial`` controller has two additional optional attributes
    ``ports`` and ``vectors``, which control how many devices can be connected
    through the controller. :since:`Since 5.2.0`, it supports an optional
-   attribute ``model`` which can be 'virtio', 'virtio-transitional', or
-   'virtio-non-transitional'. See `Virtio transitional devices`_ for more details.
+   attribute ``model`` which can be 'virtio', 'virtio-transitional' or
+   'virtio-non-transitional'. See `virtio device models`_ for more details.
 ``scsi``
    A ``scsi`` controller has an optional attribute ``model``, which is one of
    'auto', 'buslogic', 'ibmvscsi', 'lsilogic', 'lsisas1068', 'lsisas1078',
    'virtio-scsi', 'vmpvscsi', 'virtio-transitional', 'virtio-non-transitional',
    'ncr53c90' (as builtin implicit controller only), 'am53c974', 'dc390'.
-   See `Virtio transitional devices`_ for more details.
+   See `virtio device models`_ for more details.
 ``usb``
    A ``usb`` controller has an optional attribute ``model``, which is one of
    "piix3-uhci", "piix4-uhci", "ehci", "ich9-ehci1", "ich9-uhci1", "ich9-uhci2",
@@ -4452,9 +4454,8 @@ or:
       :since:`since 2.5.0` For SCSI devices, user is responsible to make sure
       the device is not used by host. This ``type`` passes all LUNs presented by
       a single HBA to the guest. :since:`Since 5.2.0`, the ``model`` attribute
-      can be specified further with "virtio-transitional",
-      "virtio-non-transitional", or "virtio". `Virtio transitional devices`_
-      for more details.
+      can be specified further with "virtio", "virtio-transitional" or
+      "virtio-non-transitional". See `virtio device models`_ for more details.
    ``mdev``
       For mediated devices ( :since:`Since 3.2.0` ) the ``model`` attribute
       specifies the device API which determines how the host's vfio driver will
@@ -6300,8 +6301,8 @@ value 'all' which when enabled grabs all input devices instead of just one,
 change the grab key combination.
 ``input`` type ``evdev`` is currently supported only on linux devices.
 (KVM only) :since:`Since 5.2.0`, the ``input`` element accepts a
-``model`` attribute which has the values 'virtio', 'virtio-transitional' and
-'virtio-non-transitional'. See `Virtio transitional devices`_ for more details.
+``model`` attribute which has the values 'virtio', 'virtio-transitional' or
+'virtio-non-transitional'. See `virtio device models`_ for more details.
 
 The subelement ``driver`` can be used to tune the virtio options of the device:
 `Virtio-related options`_ can also be set. ( :since:`Since 3.5.0` )
@@ -7982,7 +7983,7 @@ Example: manually added device with static PCI slot 2 requested
    -  'virtio-non-transitional' :since:`Since 5.2.0`
    -  'xen' - default with Xen
 
-   See `Virtio transitional devices`_ for more details.
+   See `virtio device models`_ for more details.
 
 ``autodeflate``
    The optional ``autodeflate`` attribute allows to enable/disable (values
@@ -8048,7 +8049,7 @@ Example: usage of the RNG device:
    -  'virtio-transitional' :since:`Since 5.2.0`
    -  'virtio-non-transitional' :since:`Since 5.2.0`
 
-   See `Virtio transitional devices`_ for more details.
+   See `virtio device models`_ for more details.
 
 ``rate``
    The optional ``rate`` element allows limiting the rate at which entropy can
@@ -8673,8 +8674,8 @@ Vsock
 ~~~~~
 
 A vsock host/guest interface. The ``model`` attribute defaults to ``virtio``.
-:since:`Since 5.2.0` ``model`` can also be 'virtio-transitional' and
-'virtio-non-transitional', see `Virtio transitional devices`_  for more details.
+:since:`Since 5.2.0` ``model`` can also be 'virtio-transitional' or
+'virtio-non-transitional', see `virtio device models`_ for more details.
 The optional attribute ``address`` of the ``cid`` element specifies the CID
 assigned to the guest. If the attribute ``auto`` is set to ``yes``, libvirt will
 assign a free CID automatically on domain startup. :since:`Since 4.4.0`
-- 
2.47.0

On Thu, Nov 07, 2024 at 06:34:49PM +0100, Andrea Bolognani wrote:
>When virtio-(non-)transitional models were introduced, the
>documentation was updated to include them; at the same time,
>language was introduced indicating that using the existing
>virtio model is no longer recommended.
>
>This is unnecessarily harsh, and has resulted in people
>incorrectly believing (through no fault of their own) that the
>virtio model has been deprecated.
>
>In reality, it's perfectly fine to use the virtio model as the
>stress-free option that, while often not producing the ideal
>PCI topology, will generally get the job done and work reliably
>across libvirt versions and machine types.
>

I wonder who reviewed the harsh language back then 🤔 =D

>Tweak the documentation so that it hopefully carries the
>desired message across.
>
>Signed-off-by: Andrea Bolognani <abologna@redhat.com>
>---
> docs/formatdomain.rst | 55 ++++++++++++++++++++++---------------------
> 1 file changed, 28 insertions(+), 27 deletions(-)
>
>diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
>index c50744b57b..75dff5a153 100644
>--- a/docs/formatdomain.rst
>+++ b/docs/formatdomain.rst
[...]
>@@ -3910,11 +3909,20 @@ Note: In general you should leave this option alone, unless you are very certain
> you know what you are doing.
>
>
>-Virtio transitional devices
>-~~~~~~~~~~~~~~~~~~~~~~~~~~~
>+Virtio device models
>+~~~~~~~~~~~~~~~~~~~~
>+
>+Virtio devices come in several variants, some of which are only applicable to
>+certain machine types or scenarios. The variant can be chosen via the ``model``
>+attribute, which supports the following values:
>
>-:since:`Since 5.2.0`, some of QEMU's virtio devices, when used with PCI/PCIe
>-machine types, accept the following ``model`` values:
>+``virtio``
>+   This is the recommended choice in the absence of guest OS specific
>+   constraints, as it will will generally work correctly across a large range
>+   of architectures, machine types and libvirt versions.
>+
>+:since:`Since 5.2.0`, the following values can additionally be used with machine
>+types based on PCI (either conventional PCI or PCI Express):
>

I think that you cannot specify model='virtio' before 5.2.0.  Maybe what
Daniel suggested, keeping the mention of compatibility with older
versions is the better way to go.  From the above one might think that
model='virtio' will work on older libvirt, and it will look like it
does, but it won't get parsed.

Well, that's pretty minor in the end, so either way you decide, unless
someone objects

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

On Mon, Nov 11, 2024 at 09:27:00AM +0100, Martin Kletzander wrote:
> On Thu, Nov 07, 2024 at 06:34:49PM +0100, Andrea Bolognani wrote:
> > When virtio-(non-)transitional models were introduced, the
> > documentation was updated to include them; at the same time,
> > language was introduced indicating that using the existing
> > virtio model is no longer recommended.
> > 
> > This is unnecessarily harsh, and has resulted in people
> > incorrectly believing (through no fault of their own) that the
> > virtio model has been deprecated.
> > 
> > In reality, it's perfectly fine to use the virtio model as the
> > stress-free option that, while often not producing the ideal
> > PCI topology, will generally get the job done and work reliably
> > across libvirt versions and machine types.
> > 
> 
> I wonder who reviewed the harsh language back then 🤔 =D
> 
> > Tweak the documentation so that it hopefully carries the
> > desired message across.
> > 
> > Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> > ---
> > docs/formatdomain.rst | 55 ++++++++++++++++++++++---------------------
> > 1 file changed, 28 insertions(+), 27 deletions(-)
> > 
> > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> > index c50744b57b..75dff5a153 100644
> > --- a/docs/formatdomain.rst
> > +++ b/docs/formatdomain.rst
> [...]
> > @@ -3910,11 +3909,20 @@ Note: In general you should leave this option alone, unless you are very certain
> > you know what you are doing.
> > 
> > 
> > -Virtio transitional devices
> > -~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +Virtio device models
> > +~~~~~~~~~~~~~~~~~~~~
> > +
> > +Virtio devices come in several variants, some of which are only applicable to
> > +certain machine types or scenarios. The variant can be chosen via the ``model``
> > +attribute, which supports the following values:
> > 
> > -:since:`Since 5.2.0`, some of QEMU's virtio devices, when used with PCI/PCIe
> > -machine types, accept the following ``model`` values:
> > +``virtio``
> > +   This is the recommended choice in the absence of guest OS specific
> > +   constraints, as it will will generally work correctly across a large range
> > +   of architectures, machine types and libvirt versions.
> > +
> > +:since:`Since 5.2.0`, the following values can additionally be used with machine
> > +types based on PCI (either conventional PCI or PCI Express):
> > 
> 
> I think that you cannot specify model='virtio' before 5.2.0.  Maybe what
> Daniel suggested, keeping the mention of compatibility with older
> versions is the better way to go.  From the above one might think that
> model='virtio' will work on older libvirt, and it will look like it
> does, but it won't get parsed.

It is perhaps more accurate to say

   "virtio" is equivalent to omitting the attribute entirely, reflecting
   the historical default behaviour of libvirt prior to 5.2.0.


With 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 :|

On Tue, Nov 12, 2024 at 09:12:06AM +0000, Daniel P. Berrangé wrote:
> On Mon, Nov 11, 2024 at 09:27:00AM +0100, Martin Kletzander wrote:
> > > +Virtio device models
> > > +~~~~~~~~~~~~~~~~~~~~
> > > +
> > > +Virtio devices come in several variants, some of which are only applicable to
> > > +certain machine types or scenarios. The variant can be chosen via the ``model``
> > > +attribute, which supports the following values:
> > >
> > > -:since:`Since 5.2.0`, some of QEMU's virtio devices, when used with PCI/PCIe
> > > -machine types, accept the following ``model`` values:
> > > +``virtio``
> > > +   This is the recommended choice in the absence of guest OS specific
> > > +   constraints, as it will will generally work correctly across a large range
> > > +   of architectures, machine types and libvirt versions.
> > > +
> > > +:since:`Since 5.2.0`, the following values can additionally be used with machine
> > > +types based on PCI (either conventional PCI or PCI Express):
> >
> > I think that you cannot specify model='virtio' before 5.2.0.  Maybe what
> > Daniel suggested, keeping the mention of compatibility with older
> > versions is the better way to go.  From the above one might think that
> > model='virtio' will work on older libvirt, and it will look like it
> > does, but it won't get parsed.
>
> It is perhaps more accurate to say
>
>    "virtio" is equivalent to omitting the attribute entirely, reflecting
>    the historical default behaviour of libvirt prior to 5.2.0.

That's only true for the devices that didn't accept the model
attribute before 5.2.0. <interface> is the obvious counter-example,
where specifying model=virtio and omitting the attribute have always
resulted in different behaviors.

As explained in my other reply, I think it's okay to omit the fine
details here and just provide an overview of the possible values and
the scenarios to which each is applicable, in order to help the user
make an informed choice. The documentation for each element is the
source of truth when it comes to when it started supporting the
attribute and its various values.

-- 
Andrea Bolognani / Red Hat / Virtualization

On Mon, Nov 11, 2024 at 09:27:00AM +0100, Martin Kletzander wrote:
> On Thu, Nov 07, 2024 at 06:34:49PM +0100, Andrea Bolognani wrote:
> > When virtio-(non-)transitional models were introduced, the
> > documentation was updated to include them; at the same time,
> > language was introduced indicating that using the existing
> > virtio model is no longer recommended.
> >
> > This is unnecessarily harsh, and has resulted in people
> > incorrectly believing (through no fault of their own) that the
> > virtio model has been deprecated.
> >
> > In reality, it's perfectly fine to use the virtio model as the
> > stress-free option that, while often not producing the ideal
> > PCI topology, will generally get the job done and work reliably
> > across libvirt versions and machine types.
>
> I wonder who reviewed the harsh language back then 🤔 =D

Whoever that might have been, they should probably be ashamed of
themselves right now, and might want to make amends through concrete
actions such as improving the documentation O:-)

> > +Virtio device models
> > +~~~~~~~~~~~~~~~~~~~~
> > +
> > +Virtio devices come in several variants, some of which are only applicable to
> > +certain machine types or scenarios. The variant can be chosen via the ``model``
> > +attribute, which supports the following values:
> >
> > -:since:`Since 5.2.0`, some of QEMU's virtio devices, when used with PCI/PCIe
> > -machine types, accept the following ``model`` values:
> > +``virtio``
> > +   This is the recommended choice in the absence of guest OS specific
> > +   constraints, as it will will generally work correctly across a large range
> > +   of architectures, machine types and libvirt versions.
> > +
> > +:since:`Since 5.2.0`, the following values can additionally be used with machine
> > +types based on PCI (either conventional PCI or PCI Express):
>
> I think that you cannot specify model='virtio' before 5.2.0.  Maybe what
> Daniel suggested, keeping the mention of compatibility with older
> versions is the better way to go.  From the above one might think that
> model='virtio' will work on older libvirt, and it will look like it
> does, but it won't get parsed.

It will work with many devices (net, balloon, rng, ...) but not with
others (disk, fs, input, ...) so blanket statements in either
direction will end up being partially incorrect. Each device's
documentation contains the complete and accurate version history for
when the attribute and its possible values were introduced. So in my
opinion it's fine to be a bit more hand-wavy here. The goal of this
section is to give an overview of how virtio device models work
overall, not replicate information already found elsewhere.

> Well, that's pretty minor in the end, so either way you decide, unless
> someone objects
>
> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

Thanks. I'll give Daniel some time to weigh in before pushing.

-- 
Andrea Bolognani / Red Hat / Virtualization