[libvirt] [PATCH for-4.0 v4 0/2] virtio: Provide version-specific variants of virtio PCI devices

Eduardo Habkost posted 2 patches 6 days ago
Failed in applying to current master (apply log)
hw/virtio/virtio-pci.h             |  78 +++++++--
hw/display/virtio-gpu-pci.c        |   7 +-
hw/display/virtio-vga.c            |   7 +-
hw/virtio/virtio-crypto-pci.c      |   7 +-
hw/virtio/virtio-pci.c             | 267 ++++++++++++++++++++++-------
tests/acceptance/virtio_version.py | 176 +++++++++++++++++++
6 files changed, 452 insertions(+), 90 deletions(-)
create mode 100644 tests/acceptance/virtio_version.py

[libvirt] [PATCH for-4.0 v4 0/2] virtio: Provide version-specific variants of virtio PCI devices

Posted by Eduardo Habkost 6 days ago
Existing modern-only device types are not being touched by v3, as
they don't need separate variants.  However, I plan to implement
separate cleanups in the code that calls virtio_pci_force_virtio_1(),
first, and then propose additional changes (e.g. deprecating
disable-legacy and disable-modern in those device types).

Changes v3 -> v4:
* Trivial comment fixes (Cornelia Huck)
* Test code update (Caio Carrara)

Changes v2 -> v3:
* Split into two separate patches (type registration helper
  and introduction of new types)
* Rewrote virtio_pci_types_register() completely:
  * Replaced magic generation of type names with explicit fields in
    VirtioPCIDeviceTypeInfo
  * Removed modern_only field (not necessary anymore)
  * Don't register a separate base type unless necessary

Changes v1 -> v2:
* Removed *-0.9 devices.  Nobody will want to use them, if
  transitional devices work with legacy drivers
  (Gerd Hoffmann, Michael S. Tsirkin)
* Drop virtio version from name: rename -1.0-transitional to
  -transitional (Michael S. Tsirkin)
* Renamed -1.0 to -non-transitional
* Don't add any extra variants to modern-only device types
  (they don't need it)
* Fix typo on TYPE_VIRTIO_INPUT_HOST_PCI (crash reported by
  Cornelia Huck)
* No need to change cast macros for modern-only devices
* Rename virtio_register_types() to virtio_pci_types_register()

Original patch description:

Many of the current virtio-*-pci device types actually represent
3 different types of devices:
* virtio 1.0 non-transitional devices
* virtio 1.0 transitional devices
* virtio 0.9 ("legacy device" in virtio 1.0 terminology)

That would be just an annoyance if it didn't break our device/bus
compatibility QMP interfaces.  With this multi-purpose device
type, there's no way to tell management software that
transitional devices and legacy devices require a Conventional
PCI bus.

The multi-purpose device types would also prevent us from telling
management software what's the PCI vendor/device ID for them,
because their PCI IDs change at runtime depending on the bus
where they were plugged.

This patch adds separate device types for each of those virtio
device flavors:

* virtio-*-pci: the existing multi-purpose device types
* virtio-*-pci-transitional: virtio-1.0 device supporting legacy drivers
* virtio-*-pci-non-transitional: modern-only

Reference to previous discussion that originated this idea:
  https://www.mail-archive.com/qemu-devel@nongnu.org/msg558389.html

Eduardo Habkost (2):
  virtio: Helper for registering virtio device types
  virtio: Provide version-specific variants of virtio PCI devices

 hw/virtio/virtio-pci.h             |  78 +++++++--
 hw/display/virtio-gpu-pci.c        |   7 +-
 hw/display/virtio-vga.c            |   7 +-
 hw/virtio/virtio-crypto-pci.c      |   7 +-
 hw/virtio/virtio-pci.c             | 267 ++++++++++++++++++++++-------
 tests/acceptance/virtio_version.py | 176 +++++++++++++++++++
 6 files changed, 452 insertions(+), 90 deletions(-)
 create mode 100644 tests/acceptance/virtio_version.py

-- 
2.18.0.rc1.1.g3f1ff2140

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH for-4.0 v4 0/2] virtio: Provide version-specific variants of virtio PCI devices

Posted by Eduardo Habkost 9 hours ago
Friendly ping.  3.1.0 is tagged now, so there's anything else
blocking this series?


On Wed, Dec 05, 2018 at 05:57:02PM -0200, Eduardo Habkost wrote:
> Existing modern-only device types are not being touched by v3, as
> they don't need separate variants.  However, I plan to implement
> separate cleanups in the code that calls virtio_pci_force_virtio_1(),
> first, and then propose additional changes (e.g. deprecating
> disable-legacy and disable-modern in those device types).
> 
> Changes v3 -> v4:
> * Trivial comment fixes (Cornelia Huck)
> * Test code update (Caio Carrara)
> 
> Changes v2 -> v3:
> * Split into two separate patches (type registration helper
>   and introduction of new types)
> * Rewrote virtio_pci_types_register() completely:
>   * Replaced magic generation of type names with explicit fields in
>     VirtioPCIDeviceTypeInfo
>   * Removed modern_only field (not necessary anymore)
>   * Don't register a separate base type unless necessary
> 
> Changes v1 -> v2:
> * Removed *-0.9 devices.  Nobody will want to use them, if
>   transitional devices work with legacy drivers
>   (Gerd Hoffmann, Michael S. Tsirkin)
> * Drop virtio version from name: rename -1.0-transitional to
>   -transitional (Michael S. Tsirkin)
> * Renamed -1.0 to -non-transitional
> * Don't add any extra variants to modern-only device types
>   (they don't need it)
> * Fix typo on TYPE_VIRTIO_INPUT_HOST_PCI (crash reported by
>   Cornelia Huck)
> * No need to change cast macros for modern-only devices
> * Rename virtio_register_types() to virtio_pci_types_register()
> 
> Original patch description:
> 
> Many of the current virtio-*-pci device types actually represent
> 3 different types of devices:
> * virtio 1.0 non-transitional devices
> * virtio 1.0 transitional devices
> * virtio 0.9 ("legacy device" in virtio 1.0 terminology)
> 
> That would be just an annoyance if it didn't break our device/bus
> compatibility QMP interfaces.  With this multi-purpose device
> type, there's no way to tell management software that
> transitional devices and legacy devices require a Conventional
> PCI bus.
> 
> The multi-purpose device types would also prevent us from telling
> management software what's the PCI vendor/device ID for them,
> because their PCI IDs change at runtime depending on the bus
> where they were plugged.
> 
> This patch adds separate device types for each of those virtio
> device flavors:
> 
> * virtio-*-pci: the existing multi-purpose device types
> * virtio-*-pci-transitional: virtio-1.0 device supporting legacy drivers
> * virtio-*-pci-non-transitional: modern-only
> 
> Reference to previous discussion that originated this idea:
>   https://www.mail-archive.com/qemu-devel@nongnu.org/msg558389.html
> 
> Eduardo Habkost (2):
>   virtio: Helper for registering virtio device types
>   virtio: Provide version-specific variants of virtio PCI devices
> 
>  hw/virtio/virtio-pci.h             |  78 +++++++--
>  hw/display/virtio-gpu-pci.c        |   7 +-
>  hw/display/virtio-vga.c            |   7 +-
>  hw/virtio/virtio-crypto-pci.c      |   7 +-
>  hw/virtio/virtio-pci.c             | 267 ++++++++++++++++++++++-------
>  tests/acceptance/virtio_version.py | 176 +++++++++++++++++++
>  6 files changed, 452 insertions(+), 90 deletions(-)
>  create mode 100644 tests/acceptance/virtio_version.py
> 
> -- 
> 2.18.0.rc1.1.g3f1ff2140
> 

-- 
Eduardo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH for-4.0 v4 0/2] virtio: Provide version-specific variants of virtio PCI devices

Posted by Michael S. Tsirkin 8 hours ago
Nothing, I'm packing up the 1st pull request.

On Tue, Dec 11, 2018 at 11:18:51PM -0200, Eduardo Habkost wrote:
> Friendly ping.  3.1.0 is tagged now, so there's anything else
> blocking this series?
> 
> 
> On Wed, Dec 05, 2018 at 05:57:02PM -0200, Eduardo Habkost wrote:
> > Existing modern-only device types are not being touched by v3, as
> > they don't need separate variants.  However, I plan to implement
> > separate cleanups in the code that calls virtio_pci_force_virtio_1(),
> > first, and then propose additional changes (e.g. deprecating
> > disable-legacy and disable-modern in those device types).
> > 
> > Changes v3 -> v4:
> > * Trivial comment fixes (Cornelia Huck)
> > * Test code update (Caio Carrara)
> > 
> > Changes v2 -> v3:
> > * Split into two separate patches (type registration helper
> >   and introduction of new types)
> > * Rewrote virtio_pci_types_register() completely:
> >   * Replaced magic generation of type names with explicit fields in
> >     VirtioPCIDeviceTypeInfo
> >   * Removed modern_only field (not necessary anymore)
> >   * Don't register a separate base type unless necessary
> > 
> > Changes v1 -> v2:
> > * Removed *-0.9 devices.  Nobody will want to use them, if
> >   transitional devices work with legacy drivers
> >   (Gerd Hoffmann, Michael S. Tsirkin)
> > * Drop virtio version from name: rename -1.0-transitional to
> >   -transitional (Michael S. Tsirkin)
> > * Renamed -1.0 to -non-transitional
> > * Don't add any extra variants to modern-only device types
> >   (they don't need it)
> > * Fix typo on TYPE_VIRTIO_INPUT_HOST_PCI (crash reported by
> >   Cornelia Huck)
> > * No need to change cast macros for modern-only devices
> > * Rename virtio_register_types() to virtio_pci_types_register()
> > 
> > Original patch description:
> > 
> > Many of the current virtio-*-pci device types actually represent
> > 3 different types of devices:
> > * virtio 1.0 non-transitional devices
> > * virtio 1.0 transitional devices
> > * virtio 0.9 ("legacy device" in virtio 1.0 terminology)
> > 
> > That would be just an annoyance if it didn't break our device/bus
> > compatibility QMP interfaces.  With this multi-purpose device
> > type, there's no way to tell management software that
> > transitional devices and legacy devices require a Conventional
> > PCI bus.
> > 
> > The multi-purpose device types would also prevent us from telling
> > management software what's the PCI vendor/device ID for them,
> > because their PCI IDs change at runtime depending on the bus
> > where they were plugged.
> > 
> > This patch adds separate device types for each of those virtio
> > device flavors:
> > 
> > * virtio-*-pci: the existing multi-purpose device types
> > * virtio-*-pci-transitional: virtio-1.0 device supporting legacy drivers
> > * virtio-*-pci-non-transitional: modern-only
> > 
> > Reference to previous discussion that originated this idea:
> >   https://www.mail-archive.com/qemu-devel@nongnu.org/msg558389.html
> > 
> > Eduardo Habkost (2):
> >   virtio: Helper for registering virtio device types
> >   virtio: Provide version-specific variants of virtio PCI devices
> > 
> >  hw/virtio/virtio-pci.h             |  78 +++++++--
> >  hw/display/virtio-gpu-pci.c        |   7 +-
> >  hw/display/virtio-vga.c            |   7 +-
> >  hw/virtio/virtio-crypto-pci.c      |   7 +-
> >  hw/virtio/virtio-pci.c             | 267 ++++++++++++++++++++++-------
> >  tests/acceptance/virtio_version.py | 176 +++++++++++++++++++
> >  6 files changed, 452 insertions(+), 90 deletions(-)
> >  create mode 100644 tests/acceptance/virtio_version.py
> > 
> > -- 
> > 2.18.0.rc1.1.g3f1ff2140
> > 
> 
> -- 
> Eduardo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH for-4.0 v4 0/2] virtio: Provide version-specific variants of virtio PCI devices

Posted by no-reply@patchew.org 6 days ago
Patchew URL: https://patchew.org/QEMU/20181205195704.17605-1-ehabkost@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20181205195704.17605-1-ehabkost@redhat.com
Subject: [libvirt] [PATCH for-4.0 v4 0/2] virtio: Provide version-specific variants of virtio PCI devices
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
486a758 virtio: Provide version-specific variants of virtio PCI devices
85361d9 virtio: Helper for registering virtio device types

=== OUTPUT BEGIN ===
Checking PATCH 1/2: virtio: Helper for registering virtio device types...
WARNING: line over 80 characters
#496: FILE: hw/virtio/virtio-pci.h:443:
+     * Implements both INTERFACE_PCIE_DEVICE and INTERFACE_CONVENTIONAL_PCI_DEVICE,

WARNING: line over 80 characters
#505: FILE: hw/virtio/virtio-pci.h:452:
+     * Implements both INTERFACE_PCIE_DEVICE and INTERFACE_CONVENTIONAL_PCI_DEVICE.

total: 0 errors, 2 warnings, 469 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/2: virtio: Provide version-specific variants of virtio PCI devices...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#316: 
new file mode 100644

ERROR: line over 90 characters
#372: FILE: tests/acceptance/virtio_version.py:52:
+    return devtype in [d['name'] for d in vm.command('qom-list-types', implements=implements)]

WARNING: line over 80 characters
#427: FILE: tests/acceptance/virtio_version.py:107:
+        dev_1_0, nt_ifaces = self.run_device('%s-non-transitional' % (qemu_devtype))

WARNING: line over 80 characters
#451: FILE: tests/acceptance/virtio_version.py:131:
+        dev_trans, trans_ifaces = self.run_device('%s-transitional' % (qemu_devtype))

total: 1 errors, 3 warnings, 404 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20181205195704.17605-1-ehabkost@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list