[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH RFC 00/10] pci: Partial conversion to realize |
Date: |
Fri, 30 Jan 2015 10:17:51 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
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.