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

Eduardo Habkost posted 2 patches 5 years, 4 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
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 | 177 +++++++++++++++++++
6 files changed, 453 insertions(+), 90 deletions(-)
create mode 100644 tests/acceptance/virtio_version.py
[libvirt] [PATCH for-4.0 v3 0/2] virtio: Provide version-specific variants of virtio PCI devices
Posted by Eduardo Habkost 5 years, 4 months 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 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 | 177 +++++++++++++++++++
 6 files changed, 453 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 v3 0/2] virtio: Provide version-specific variants of virtio PCI devices
Posted by no-reply@patchew.org 5 years, 4 months ago
Hi,

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

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

=== 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'
421d93d virtio: Provide version-specific variants of virtio PCI devices
bef1223 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
#495: FILE: hw/virtio/virtio-pci.h:443:
+     * Implements both INTERFACE_PCIE_DEVICE and INTERFACE_CONVENTIONAL_PCI_DEVICE,

WARNING: line over 80 characters
#504: 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?
#315: 
new file mode 100644

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

ERROR: line over 90 characters
#373: FILE: tests/acceptance/virtio_version.py:54:
+def get_interfaces(vm, devtype, interfaces=['pci-express-device', 'conventional-pci-device']):

WARNING: line over 80 characters
#427: FILE: tests/acceptance/virtio_version.py:108:
+        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:132:
+        dev_trans,trans_ifaces = self.run_device('%s-transitional' % (qemu_devtype))

total: 2 errors, 3 warnings, 405 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


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