Re: [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

> Markus Armbruster <address@hidden> writes:
>
>> Andreas Färber <address@hidden> writes:
>>
>>> Hi Markus,
>>>
>>> Am 28.10.2014 um 08:35 schrieb Markus Armbruster:
>>>> While discussing Gonglei's "[PATCH v2 00/19] usb: convert device init
>>>> to realize", Paolo called the PCI conversion job "Gargantuan".  This
>>>> series attempts to crack it into manageable jobs.
>>>
>>> Thanks for giving this a stab! What kept me from diving into the PCI
>>> converstion was that I first invested into qtests for the non-default
>>> PCI devices. How many of the converted devices are actually covered in
>>> qtest?
>>
>> Can't tell offhand, but I can find out.
>
> The vast majority of my conversions are utterly trivial [PATCH 03]:
> change return type to void, rename, put into ->realize instead of
> ->init.  I could enumerate the devices so changed and look for qtests,
> but I feel it's rather pointless busywork.  But if you should insist...
>
> The remaining conversions are all very, very simple:
>
> * PATCH 05: "pcnet"
>
>   Utterly trivial after trivial PATCH 04 changed a helper's return type
>   to void.
>
>   There's pcnet-test.c, which basically tests "-device pcnet doesn't
>   explode right away".
>
> * PATCH 06: "pci-serial", "pci-serial-2x", "pci-serial-4x"
>
>   Two error paths trivially converted from qerror_report_err() to
>   error_propagate().
>
>   Not covered in qtest as far as I can tell.
>
> * PATCH 07: "ich9-ahci"
>
>   One pci_add_capability() replaced by pci_add_capability2().  The
>   former is a wrapper around the latter which additionally passes any
>   error to error_report(), then frees it.  Straightforward conversion of
>   the error path to error_setg().
>
>   Not covered in qtest as far as I can tell.
>
> * PATCH 08: "cirrus-vga"
>
>   One error path trivially converted from error_report() to
>   error_setg().
>
>   There's display-vga-test.c, which tests "-device cirrus-vga doesn't
>   explode right away".
>
> * PATCH 09: "qxl-vga", "qxl"
>
>   Two error paths trivially converted from error_report() to
>   error_setg().
>
>   Not covered in qtest as far as I can tell.
>
> * PATCH 10: "kvm-pci-assign"
>
>   Two error paths trivially converted from qerror_report_err() to
>   error_propagate().
>
>   Not covered in qtest as far as I can tell.
>
> I'm prepared to drop conversions you consider risky without test
> coverage (although I have a hard time seeing risks, considering how
> silly-simple my conversions are).  But I'd really like to get the core
> changes plus some examples in.

This series passes all tests added by my "qtest: Generic PCI device
test" series.  That series covers realizing every PCI device, except for

* blacklisted devices, and

* devices not included in the x86 targets (because the test runs only
  for the x86 targets, just like all of $(check-qtest-pci-y)).

Devices in the above list of not entirely trivial conversions that
aren't covered:

* "kvm-pci-assign" is blacklisted because it requires a suitable host
  device to pass through.

* "qxl" is blacklisted because "-device qxl" crashes.  However,
  "qxl-vga" executes the patched code as well, and *is* covered.

Andreas, if you object to any of my conversions without additional test
coverage, please enumerate the devices whose conversion you want me to
drop.