qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Caio Carrara <ccarrara@redhat.com>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>,
	Gonglei <arei.gonglei@huawei.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Amit Shah" <amit@kernel.org>,
	"Andrea Bolognani" <abologna@redhat.com>,
	"Cleber Rosa" <crosa@redhat.com>,
	"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
	"Fam Zheng" <famz@redhat.com>,
	"Cornelia Huck" <cohuck@redhat.com>,
	"Kevin Wolf" <kwolf@redhat.com>, "Max Reitz" <mreitz@redhat.com>,
	libvir-list@redhat.com,
	"Wainer dos Santos Moschetta" <wainersm@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Laine Stump" <laine@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>
Subject: Re: [Qemu-devel] [PATCH for-4.0 v4 2/2] virtio: Provide version-specific variants of virtio PCI devices
Date: Fri, 7 Dec 2018 10:03:24 -0200	[thread overview]
Message-ID: <20181207120323.GA31088@localhost.localdomain> (raw)
In-Reply-To: <20181205195704.17605-3-ehabkost@redhat.com>

On Wed, Dec 05, 2018 at 05:57:04PM -0200, Eduardo Habkost wrote:
> 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 these multi-purpose device
> types, 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
>   - Configurable using `disable-legacy` and `disable-modern`
>     properties
>   - Legacy driver support is automatically enabled/disabled
>     depending on the bus where it is plugged
>   - Supports Conventional PCI and PCI Express buses
>     (but Conventional PCI is incompatible with
>     disable-legacy=off)
>   - Changes PCI vendor/device IDs at runtime
> - virtio-*-pci-transitional: virtio-1.0 device supporting legacy drivers
>   - Supports Conventional PCI buses only, because
>     it has a PIO BAR
> - virtio-*-pci-non-transitional: modern-only
>   - Supports both Conventional PCI and PCI Express buses
> 
> The existing TYPE_* macros for these types will point to an
> abstract base type, so existing casts in the code will keep
> working for all variants.
> 
> A simple test script (tests/acceptance/virtio_version.py) is
> included, to check if the new device types are equivalent to
> using the `disable-legacy` and `disable-modern` options.
> 
> Acked-by: Andrea Bolognani <abologna@redhat.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Caio Carrara <ccarrara@redhat.com>

> ---
> Changes v3 -> v4:
> * Only test code changes (Caio Carrara)
>   * Rename get_interfaces() to get_pci_interfaces()
>   * Use tuple instead of list at get_pci_interfaces()
>   * Spaces after commas
> 
> Changes v2 -> v3:
> * Split into two separate patches (type registration helper
>   and introduction of new types)
> * Include full type names as literals in the code instead
>   of generating type names automatically
> 
> 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()
> ---
>  hw/virtio/virtio-pci.h             |  24 ++--
>  hw/virtio/virtio-pci.c             |  60 ++++++++--
>  tests/acceptance/virtio_version.py | 176 +++++++++++++++++++++++++++++
>  3 files changed, 236 insertions(+), 24 deletions(-)
>  create mode 100644 tests/acceptance/virtio_version.py
> 
> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> index 8cd546608e..29b4216107 100644
> --- a/hw/virtio/virtio-pci.h
> +++ b/hw/virtio/virtio-pci.h
> @@ -216,7 +216,7 @@ static inline void virtio_pci_disable_modern(VirtIOPCIProxy *proxy)
>  /*
>   * virtio-scsi-pci: This extends VirtioPCIProxy.
>   */
> -#define TYPE_VIRTIO_SCSI_PCI "virtio-scsi-pci"
> +#define TYPE_VIRTIO_SCSI_PCI "virtio-scsi-pci-base"
>  #define VIRTIO_SCSI_PCI(obj) \
>          OBJECT_CHECK(VirtIOSCSIPCI, (obj), TYPE_VIRTIO_SCSI_PCI)
>  
> @@ -229,7 +229,7 @@ struct VirtIOSCSIPCI {
>  /*
>   * vhost-scsi-pci: This extends VirtioPCIProxy.
>   */
> -#define TYPE_VHOST_SCSI_PCI "vhost-scsi-pci"
> +#define TYPE_VHOST_SCSI_PCI "vhost-scsi-pci-base"
>  #define VHOST_SCSI_PCI(obj) \
>          OBJECT_CHECK(VHostSCSIPCI, (obj), TYPE_VHOST_SCSI_PCI)
>  
> @@ -239,7 +239,7 @@ struct VHostSCSIPCI {
>  };
>  #endif
>  
> -#define TYPE_VHOST_USER_SCSI_PCI "vhost-user-scsi-pci"
> +#define TYPE_VHOST_USER_SCSI_PCI "vhost-user-scsi-pci-base"
>  #define VHOST_USER_SCSI_PCI(obj) \
>          OBJECT_CHECK(VHostUserSCSIPCI, (obj), TYPE_VHOST_USER_SCSI_PCI)
>  
> @@ -252,7 +252,7 @@ struct VHostUserSCSIPCI {
>  /*
>   * vhost-user-blk-pci: This extends VirtioPCIProxy.
>   */
> -#define TYPE_VHOST_USER_BLK_PCI "vhost-user-blk-pci"
> +#define TYPE_VHOST_USER_BLK_PCI "vhost-user-blk-pci-base"
>  #define VHOST_USER_BLK_PCI(obj) \
>          OBJECT_CHECK(VHostUserBlkPCI, (obj), TYPE_VHOST_USER_BLK_PCI)
>  
> @@ -265,7 +265,7 @@ struct VHostUserBlkPCI {
>  /*
>   * virtio-blk-pci: This extends VirtioPCIProxy.
>   */
> -#define TYPE_VIRTIO_BLK_PCI "virtio-blk-pci"
> +#define TYPE_VIRTIO_BLK_PCI "virtio-blk-pci-base"
>  #define VIRTIO_BLK_PCI(obj) \
>          OBJECT_CHECK(VirtIOBlkPCI, (obj), TYPE_VIRTIO_BLK_PCI)
>  
> @@ -277,7 +277,7 @@ struct VirtIOBlkPCI {
>  /*
>   * virtio-balloon-pci: This extends VirtioPCIProxy.
>   */
> -#define TYPE_VIRTIO_BALLOON_PCI "virtio-balloon-pci"
> +#define TYPE_VIRTIO_BALLOON_PCI "virtio-balloon-pci-base"
>  #define VIRTIO_BALLOON_PCI(obj) \
>          OBJECT_CHECK(VirtIOBalloonPCI, (obj), TYPE_VIRTIO_BALLOON_PCI)
>  
> @@ -289,7 +289,7 @@ struct VirtIOBalloonPCI {
>  /*
>   * virtio-serial-pci: This extends VirtioPCIProxy.
>   */
> -#define TYPE_VIRTIO_SERIAL_PCI "virtio-serial-pci"
> +#define TYPE_VIRTIO_SERIAL_PCI "virtio-serial-pci-base"
>  #define VIRTIO_SERIAL_PCI(obj) \
>          OBJECT_CHECK(VirtIOSerialPCI, (obj), TYPE_VIRTIO_SERIAL_PCI)
>  
> @@ -301,7 +301,7 @@ struct VirtIOSerialPCI {
>  /*
>   * virtio-net-pci: This extends VirtioPCIProxy.
>   */
> -#define TYPE_VIRTIO_NET_PCI "virtio-net-pci"
> +#define TYPE_VIRTIO_NET_PCI "virtio-net-pci-base"
>  #define VIRTIO_NET_PCI(obj) \
>          OBJECT_CHECK(VirtIONetPCI, (obj), TYPE_VIRTIO_NET_PCI)
>  
> @@ -316,7 +316,7 @@ struct VirtIONetPCI {
>  
>  #ifdef CONFIG_VIRTFS
>  
> -#define TYPE_VIRTIO_9P_PCI "virtio-9p-pci"
> +#define TYPE_VIRTIO_9P_PCI "virtio-9p-pci-base"
>  #define VIRTIO_9P_PCI(obj) \
>          OBJECT_CHECK(V9fsPCIState, (obj), TYPE_VIRTIO_9P_PCI)
>  
> @@ -330,7 +330,7 @@ typedef struct V9fsPCIState {
>  /*
>   * virtio-rng-pci: This extends VirtioPCIProxy.
>   */
> -#define TYPE_VIRTIO_RNG_PCI "virtio-rng-pci"
> +#define TYPE_VIRTIO_RNG_PCI "virtio-rng-pci-base"
>  #define VIRTIO_RNG_PCI(obj) \
>          OBJECT_CHECK(VirtIORngPCI, (obj), TYPE_VIRTIO_RNG_PCI)
>  
> @@ -365,7 +365,7 @@ struct VirtIOInputHIDPCI {
>  
>  #ifdef CONFIG_LINUX
>  
> -#define TYPE_VIRTIO_INPUT_HOST_PCI "virtio-input-host-pci"
> +#define TYPE_VIRTIO_INPUT_HOST_PCI "virtio-input-host-pci-base"
>  #define VIRTIO_INPUT_HOST_PCI(obj) \
>          OBJECT_CHECK(VirtIOInputHostPCI, (obj), TYPE_VIRTIO_INPUT_HOST_PCI)
>  
> @@ -392,7 +392,7 @@ struct VirtIOGPUPCI {
>  /*
>   * vhost-vsock-pci: This extends VirtioPCIProxy.
>   */
> -#define TYPE_VHOST_VSOCK_PCI "vhost-vsock-pci"
> +#define TYPE_VHOST_VSOCK_PCI "vhost-vsock-pci-base"
>  #define VHOST_VSOCK_PCI(obj) \
>          OBJECT_CHECK(VHostVSockPCI, (obj), TYPE_VHOST_VSOCK_PCI)
>  
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index f07ec55c38..d05066deb8 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1120,7 +1120,10 @@ static void virtio_9p_pci_instance_init(Object *obj)
>  }
>  
>  static const VirtioPCIDeviceTypeInfo virtio_9p_pci_info = {
> -    .generic_name   = TYPE_VIRTIO_9P_PCI,
> +    .base_name              = TYPE_VIRTIO_9P_PCI,
> +    .generic_name           = "virtio-9p-pci",
> +    .transitional_name      = "virtio-9p-pci-transitional",
> +    .non_transitional_name  = "virtio-9p-pci-non-transitional",
>      .instance_size = sizeof(V9fsPCIState),
>      .instance_init = virtio_9p_pci_instance_init,
>      .class_init    = virtio_9p_pci_class_init,
> @@ -2102,7 +2105,10 @@ static void virtio_blk_pci_instance_init(Object *obj)
>  }
>  
>  static const VirtioPCIDeviceTypeInfo virtio_blk_pci_info = {
> -    .generic_name   = TYPE_VIRTIO_BLK_PCI,
> +    .base_name              = TYPE_VIRTIO_BLK_PCI,
> +    .generic_name           = "virtio-blk-pci",
> +    .transitional_name      = "virtio-blk-pci-transitional",
> +    .non_transitional_name  = "virtio-blk-pci-non-transitional",
>      .instance_size = sizeof(VirtIOBlkPCI),
>      .instance_init = virtio_blk_pci_instance_init,
>      .class_init    = virtio_blk_pci_class_init,
> @@ -2157,7 +2163,10 @@ static void vhost_user_blk_pci_instance_init(Object *obj)
>  }
>  
>  static const VirtioPCIDeviceTypeInfo vhost_user_blk_pci_info = {
> -    .generic_name    = TYPE_VHOST_USER_BLK_PCI,
> +    .base_name               = TYPE_VHOST_USER_BLK_PCI,
> +    .generic_name            = "vhost-user-blk-pci",
> +    .transitional_name       = "vhost-user-blk-pci-transitional",
> +    .non_transitional_name   = "vhost-user-blk-pci-non-transitional",
>      .instance_size  = sizeof(VHostUserBlkPCI),
>      .instance_init  = vhost_user_blk_pci_instance_init,
>      .class_init     = vhost_user_blk_pci_class_init,
> @@ -2224,7 +2233,10 @@ static void virtio_scsi_pci_instance_init(Object *obj)
>  }
>  
>  static const VirtioPCIDeviceTypeInfo virtio_scsi_pci_info = {
> -    .generic_name   = TYPE_VIRTIO_SCSI_PCI,
> +    .base_name              = TYPE_VIRTIO_SCSI_PCI,
> +    .generic_name           = "virtio-scsi-pci",
> +    .transitional_name      = "virtio-scsi-pci-transitional",
> +    .non_transitional_name  = "virtio-scsi-pci-non-transitional",
>      .instance_size = sizeof(VirtIOSCSIPCI),
>      .instance_init = virtio_scsi_pci_instance_init,
>      .class_init    = virtio_scsi_pci_class_init,
> @@ -2278,7 +2290,10 @@ static void vhost_scsi_pci_instance_init(Object *obj)
>  }
>  
>  static const VirtioPCIDeviceTypeInfo vhost_scsi_pci_info = {
> -    .generic_name  = TYPE_VHOST_SCSI_PCI,
> +    .base_name             = TYPE_VHOST_SCSI_PCI,
> +    .generic_name          = "vhost-scsi-pci",
> +    .transitional_name     = "vhost-scsi-pci-transitional",
> +    .non_transitional_name = "vhost-scsi-pci-non-transitional",
>      .instance_size = sizeof(VHostSCSIPCI),
>      .instance_init = vhost_scsi_pci_instance_init,
>      .class_init    = vhost_scsi_pci_class_init,
> @@ -2332,7 +2347,10 @@ static void vhost_user_scsi_pci_instance_init(Object *obj)
>  }
>  
>  static const VirtioPCIDeviceTypeInfo vhost_user_scsi_pci_info = {
> -    .generic_name  = TYPE_VHOST_USER_SCSI_PCI,
> +    .base_name             = TYPE_VHOST_USER_SCSI_PCI,
> +    .generic_name          = "vhost-user-scsi-pci",
> +    .transitional_name     = "vhost-user-scsi-pci-transitional",
> +    .non_transitional_name = "vhost-user-scsi-pci-non-transitional",
>      .instance_size = sizeof(VHostUserSCSIPCI),
>      .instance_init = vhost_user_scsi_pci_instance_init,
>      .class_init    = vhost_user_scsi_pci_class_init,
> @@ -2379,7 +2397,10 @@ static void vhost_vsock_pci_instance_init(Object *obj)
>  }
>  
>  static const VirtioPCIDeviceTypeInfo vhost_vsock_pci_info = {
> -    .generic_name  = TYPE_VHOST_VSOCK_PCI,
> +    .base_name             = TYPE_VHOST_VSOCK_PCI,
> +    .generic_name          = "vhost-vsock-pci",
> +    .transitional_name     = "vhost-vsock-pci-transitional",
> +    .non_transitional_name = "vhost-vsock-pci-non-transitional",
>      .instance_size = sizeof(VHostVSockPCI),
>      .instance_init = vhost_vsock_pci_instance_init,
>      .class_init    = vhost_vsock_pci_class_init,
> @@ -2435,7 +2456,10 @@ static void virtio_balloon_pci_instance_init(Object *obj)
>  }
>  
>  static const VirtioPCIDeviceTypeInfo virtio_balloon_pci_info = {
> -    .generic_name  = TYPE_VIRTIO_BALLOON_PCI,
> +    .base_name             = TYPE_VIRTIO_BALLOON_PCI,
> +    .generic_name          = "virtio-balloon-pci",
> +    .transitional_name     = "virtio-balloon-pci-transitional",
> +    .non_transitional_name = "virtio-balloon-pci-non-transitional",
>      .instance_size = sizeof(VirtIOBalloonPCI),
>      .instance_init = virtio_balloon_pci_instance_init,
>      .class_init    = virtio_balloon_pci_class_init,
> @@ -2507,7 +2531,10 @@ static void virtio_serial_pci_instance_init(Object *obj)
>  }
>  
>  static const VirtioPCIDeviceTypeInfo virtio_serial_pci_info = {
> -    .generic_name  = TYPE_VIRTIO_SERIAL_PCI,
> +    .base_name             = TYPE_VIRTIO_SERIAL_PCI,
> +    .generic_name          = "virtio-serial-pci",
> +    .transitional_name     = "virtio-serial-pci-transitional",
> +    .non_transitional_name = "virtio-serial-pci-non-transitional",
>      .instance_size = sizeof(VirtIOSerialPCI),
>      .instance_init = virtio_serial_pci_instance_init,
>      .class_init    = virtio_serial_pci_class_init,
> @@ -2561,7 +2588,10 @@ static void virtio_net_pci_instance_init(Object *obj)
>  }
>  
>  static const VirtioPCIDeviceTypeInfo virtio_net_pci_info = {
> -    .generic_name  = TYPE_VIRTIO_NET_PCI,
> +    .base_name             = TYPE_VIRTIO_NET_PCI,
> +    .generic_name          = "virtio-net-pci",
> +    .transitional_name     = "virtio-net-pci-transitional",
> +    .non_transitional_name = "virtio-net-pci-non-transitional",
>      .instance_size = sizeof(VirtIONetPCI),
>      .instance_init = virtio_net_pci_instance_init,
>      .class_init    = virtio_net_pci_class_init,
> @@ -2611,7 +2641,10 @@ static void virtio_rng_initfn(Object *obj)
>  }
>  
>  static const VirtioPCIDeviceTypeInfo virtio_rng_pci_info = {
> -    .generic_name  = TYPE_VIRTIO_RNG_PCI,
> +    .base_name             = TYPE_VIRTIO_RNG_PCI,
> +    .generic_name          = "virtio-rng-pci",
> +    .transitional_name     = "virtio-rng-pci-transitional",
> +    .non_transitional_name = "virtio-rng-pci-non-transitional",
>      .instance_size = sizeof(VirtIORngPCI),
>      .instance_init = virtio_rng_initfn,
>      .class_init    = virtio_rng_pci_class_init,
> @@ -2734,7 +2767,10 @@ static void virtio_host_initfn(Object *obj)
>  }
>  
>  static const VirtioPCIDeviceTypeInfo virtio_host_pci_info = {
> -    .generic_name  = TYPE_VIRTIO_INPUT_HOST_PCI,
> +    .base_name             = TYPE_VIRTIO_INPUT_HOST_PCI,
> +    .generic_name          = "virtio-input-host-pci",
> +    .transitional_name     = "virtio-input-host-pci-transitional",
> +    .non_transitional_name = "virtio-input-host-pci-non-transitional",
>      .parent        = TYPE_VIRTIO_INPUT_PCI,
>      .instance_size = sizeof(VirtIOInputHostPCI),
>      .instance_init = virtio_host_initfn,
> diff --git a/tests/acceptance/virtio_version.py b/tests/acceptance/virtio_version.py
> new file mode 100644
> index 0000000000..ce990250d8
> --- /dev/null
> +++ b/tests/acceptance/virtio_version.py
> @@ -0,0 +1,176 @@
> +"""
> +Check compatibility of virtio device types
> +"""
> +# Copyright (c) 2018 Red Hat, Inc.
> +#
> +# Author:
> +#  Eduardo Habkost <ehabkost@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or
> +# later.  See the COPYING file in the top-level directory.
> +import sys
> +import os
> +
> +sys.path.append(os.path.join(os.path.dirname(__file__), "..", "..", "scripts"))
> +from qemu import QEMUMachine
> +from avocado_qemu import Test
> +
> +# Virtio Device IDs:
> +VIRTIO_NET = 1
> +VIRTIO_BLOCK = 2
> +VIRTIO_CONSOLE = 3
> +VIRTIO_RNG = 4
> +VIRTIO_BALLOON = 5
> +VIRTIO_RPMSG = 7
> +VIRTIO_SCSI = 8
> +VIRTIO_9P = 9
> +VIRTIO_RPROC_SERIAL = 11
> +VIRTIO_CAIF = 12
> +VIRTIO_GPU = 16
> +VIRTIO_INPUT = 18
> +VIRTIO_VSOCK = 19
> +VIRTIO_CRYPTO = 20
> +
> +PCI_VENDOR_ID_REDHAT_QUMRANET = 0x1af4
> +
> +# Device IDs for legacy/transitional devices:
> +PCI_LEGACY_DEVICE_IDS = {
> +    VIRTIO_NET:     0x1000,
> +    VIRTIO_BLOCK:   0x1001,
> +    VIRTIO_BALLOON: 0x1002,
> +    VIRTIO_CONSOLE: 0x1003,
> +    VIRTIO_SCSI:    0x1004,
> +    VIRTIO_RNG:     0x1005,
> +    VIRTIO_9P:      0x1009,
> +    VIRTIO_VSOCK:   0x1012,
> +}
> +
> +def pci_modern_device_id(virtio_devid):
> +    return virtio_devid + 0x1040
> +
> +def devtype_implements(vm, devtype, implements):
> +    return devtype in [d['name'] for d in vm.command('qom-list-types', implements=implements)]
> +
> +def get_pci_interfaces(vm, devtype):
> +    interfaces = ('pci-express-device', 'conventional-pci-device')
> +    return [i for i in interfaces if devtype_implements(vm, devtype, i)]
> +
> +class VirtioVersionCheck(Test):
> +    """
> +    Check if virtio-version-specific device types result in the
> +    same device tree created by `disable-modern` and
> +    `disable-legacy`.
> +
> +    :avocado: enable
> +    :avocado: tags=x86_64
> +    """
> +
> +    # just in case there are failures, show larger diff:
> +    maxDiff = 4096
> +
> +    def run_device(self, devtype, opts=None, machine='pc'):
> +        """
> +        Run QEMU with `-device DEVTYPE`, return device info from `query-pci`
> +        """
> +        with QEMUMachine(self.qemu_bin) as vm:
> +            vm.set_machine(machine)
> +            if opts:
> +                devtype += ',' + opts
> +            vm.add_args('-device', '%s,id=devfortest' % (devtype))
> +            vm.add_args('-S')
> +            vm.launch()
> +
> +            pcibuses = vm.command('query-pci')
> +            alldevs = [dev for bus in pcibuses for dev in bus['devices']]
> +            devfortest = [dev for dev in alldevs
> +                          if dev['qdev_id'] == 'devfortest']
> +            return devfortest[0], get_pci_interfaces(vm, devtype)
> +
> +
> +    def assert_devids(self, dev, devid, non_transitional=False):
> +        self.assertEqual(dev['id']['vendor'], PCI_VENDOR_ID_REDHAT_QUMRANET)
> +        self.assertEqual(dev['id']['device'], devid)
> +        if non_transitional:
> +            self.assertTrue(0x1040 <= dev['id']['device'] <= 0x107f)
> +            self.assertGreaterEqual(dev['id']['subsystem'], 0x40)
> +
> +    def check_all_variants(self, qemu_devtype, virtio_devid):
> +        """Check if a virtio device type and its variants behave as expected"""
> +        # Force modern mode:
> +        dev_modern, _ = self.run_device(qemu_devtype,
> +                                       'disable-modern=off,disable-legacy=on')
> +        self.assert_devids(dev_modern, pci_modern_device_id(virtio_devid),
> +                           non_transitional=True)
> +
> +        # <prefix>-non-transitional device types should be 100% equivalent to
> +        # <prefix>,disable-modern=off,disable-legacy=on
> +        dev_1_0, nt_ifaces = self.run_device('%s-non-transitional' % (qemu_devtype))
> +        self.assertEqual(dev_modern, dev_1_0)
> +
> +        # Force transitional mode:
> +        dev_trans, _ = self.run_device(qemu_devtype,
> +                                      'disable-modern=off,disable-legacy=off')
> +        self.assert_devids(dev_trans, PCI_LEGACY_DEVICE_IDS[virtio_devid])
> +
> +        # Force legacy mode:
> +        dev_legacy, _ = self.run_device(qemu_devtype,
> +                                       'disable-modern=on,disable-legacy=off')
> +        self.assert_devids(dev_legacy, PCI_LEGACY_DEVICE_IDS[virtio_devid])
> +
> +        # No options: default to transitional on PC machine-type:
> +        no_opts_pc, generic_ifaces = self.run_device(qemu_devtype)
> +        self.assertEqual(dev_trans, no_opts_pc)
> +
> +        #TODO: check if plugging on a PCI Express bus will make the
> +        #      device non-transitional
> +        #no_opts_q35 = self.run_device(qemu_devtype, machine='q35')
> +        #self.assertEqual(dev_modern, no_opts_q35)
> +
> +        # <prefix>-transitional device types should be 100% equivalent to
> +        # <prefix>,disable-modern=off,disable-legacy=off
> +        dev_trans, trans_ifaces = self.run_device('%s-transitional' % (qemu_devtype))
> +        self.assertEqual(dev_trans, dev_trans)
> +
> +        # ensure the interface information is correct:
> +        self.assertIn('conventional-pci-device', generic_ifaces)
> +        self.assertIn('pci-express-device', generic_ifaces)
> +
> +        self.assertIn('conventional-pci-device', nt_ifaces)
> +        self.assertIn('pci-express-device', nt_ifaces)
> +
> +        self.assertIn('conventional-pci-device', trans_ifaces)
> +        self.assertNotIn('pci-express-device', trans_ifaces)
> +
> +
> +    def test_conventional_devs(self):
> +        self.check_all_variants('virtio-net-pci', VIRTIO_NET)
> +        # virtio-blk requires 'driver' parameter
> +        #self.check_all_variants('virtio-blk-pci', VIRTIO_BLOCK)
> +        self.check_all_variants('virtio-serial-pci', VIRTIO_CONSOLE)
> +        self.check_all_variants('virtio-rng-pci', VIRTIO_RNG)
> +        self.check_all_variants('virtio-balloon-pci', VIRTIO_BALLOON)
> +        self.check_all_variants('virtio-scsi-pci', VIRTIO_SCSI)
> +        # virtio-9p requires 'fsdev' parameter
> +        #self.check_all_variants('virtio-9p-pci', VIRTIO_9P)
> +
> +    def check_modern_only(self, qemu_devtype, virtio_devid):
> +        """Check if a modern-only virtio device type behaves as expected"""
> +        # Force modern mode:
> +        dev_modern, _ = self.run_device(qemu_devtype,
> +                                       'disable-modern=off,disable-legacy=on')
> +        self.assert_devids(dev_modern, pci_modern_device_id(virtio_devid),
> +                           non_transitional=True)
> +
> +        # No options: should be modern anyway
> +        dev_no_opts, ifaces = self.run_device(qemu_devtype)
> +        self.assertEqual(dev_modern, dev_no_opts)
> +
> +        self.assertIn('conventional-pci-device', ifaces)
> +        self.assertIn('pci-express-device', ifaces)
> +
> +    def test_modern_only_devs(self):
> +        self.check_modern_only('virtio-vga', VIRTIO_GPU)
> +        self.check_modern_only('virtio-gpu-pci', VIRTIO_GPU)
> +        self.check_modern_only('virtio-mouse-pci', VIRTIO_INPUT)
> +        self.check_modern_only('virtio-tablet-pci', VIRTIO_INPUT)
> +        self.check_modern_only('virtio-keyboard-pci', VIRTIO_INPUT)
> -- 
> 2.18.0.rc1.1.g3f1ff2140
> 

-- 
Caio Carrara
Software Engineer, Virt Team - Red Hat
ccarrara@redhat.com

  reply	other threads:[~2018-12-07 12:04 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-05 19:57 [Qemu-devel] [PATCH for-4.0 v4 0/2] virtio: Provide version-specific variants of virtio PCI devices Eduardo Habkost
2018-12-05 19:57 ` [Qemu-devel] [PATCH for-4.0 v4 1/2] virtio: Helper for registering virtio device types Eduardo Habkost
2018-12-05 19:57 ` [Qemu-devel] [PATCH for-4.0 v4 2/2] virtio: Provide version-specific variants of virtio PCI devices Eduardo Habkost
2018-12-07 12:03   ` Caio Carrara [this message]
2019-01-03  9:38   ` Thomas Huth
2019-01-03 10:14     ` Thomas Huth
2019-01-03 10:48       ` Cornelia Huck
2019-01-03 18:32         ` Eduardo Habkost
2019-01-04  4:27           ` Michael S. Tsirkin
2019-01-04  8:27             ` Cornelia Huck
2018-12-05 21:42 ` [Qemu-devel] [libvirt] [PATCH for-4.0 v4 0/2] " no-reply
2018-12-12  1:18 ` [Qemu-devel] " Eduardo Habkost
2018-12-12  1:22   ` Michael S. Tsirkin
2019-03-05 12:09 ` Andrea Bolognani
2019-03-05 14:38   ` Gerd Hoffmann
2019-03-05 15:56     ` Andrea Bolognani
2019-03-06  7:41       ` [Qemu-devel] [libvirt] " Peter Krempa
2019-03-06  8:30         ` Ján Tomko
2019-03-06  9:08           ` Andrea Bolognani
2019-03-06  9:10           ` Daniel P. Berrangé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181207120323.GA31088@localhost.localdomain \
    --to=ccarrara@redhat.com \
    --cc=abologna@redhat.com \
    --cc=amit@kernel.org \
    --cc=arei.gonglei@huawei.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=crosa@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=famz@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=laine@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=mreitz@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=wainersm@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).