[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize |
Date: |
Tue, 13 Sep 2016 08:25:56 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Alex Williamson <address@hidden> writes:
> On Mon, 12 Sep 2016 16:00:18 +0200
> Auger Eric <address@hidden> wrote:
>
>> Hi Markus,
>>
>> On 12/09/2016 14:45, Markus Armbruster wrote:
>> > Eric Auger <address@hidden> writes:
>> >
>> >> This patch converts VFIO PCI to realize function.
>> >>
>> >> Also original initfn errors now are propagated using QEMU
>> >> error objects. All errors are formatted with the same pattern:
>> >> "vfio: %s: the error description"
>> >
>> > Example:
>> >
>> > $ upstream-qemu -device vfio-pci
>> > upstream-qemu: -device vfio-pci: vfio: 0000:00:00.0: no iommu_group found:
>> > Unknown error -2
>> >
>> > Two remarks:
>> >
>> > * "Unknown error -2" should be "No such file or directory". See below.
>> Hum. I noticed that but I didn't have the presence of mind to get it was
>> due to -errno!
>> >
>> > * Five colons, not counting the ones in the PCI address. Do we need the
>> > "vfio: 0000:00:00.0:" part? If yes, could we find a nicer way to
>> > print it? Up to you.
>> Well we have quite a lot of traces with such format, hence my choice.
>> Alex do you want to change this?
>
> Well, we need to identify the component with the error, it's not
> uncommon to have multiple assigned devices. The PCI address is just
> the basename in vfio code, it might also be the name of a device node
> in sysfs, such as a uuid of an mdev devices. AFAIK we cannot count on
> a id and even if we could libvirt assigns them based on order in the
> xml, making them a bit magic. Maybe I'm not understanding the
> question. Regarding trace vs error message, I expect that it's going
> to be a more advanced user/developer enabling tracing, error reports
> should try a little harder to be comprehensible to an average user.
Yes, the error message needs to identify the part that's wrong.
However, we need to consider the *complete* error message to judge it.
Consider:
$ upstream-qemu -device vfio-pci
upstream-qemu: -device vfio-pci: vfio: 0000:00:00.0: no iommu_group found:
No such file or directory
Which parts are actually useful for the user? "-device vfio-pci"
identifies the part that's wrong. "vfio: 0000:00:00.0" is gobbledygook
unless you're somewhat familiar with vfio, and then it's redundant.
The "vfio: 0000:00:00.0" *is* useful in messages outside realize()
context, because then the system can't prefix the "-device vfio-pci"
part.
>> > Always, always, always test your error messages :)
Because what you think you see in the code may differ from what the user
will see.
Anyway, your choice, I'm just giving feedback on the error messages I
observe in testing.
[...]
- [Qemu-devel] [PATCH 3/3] vfio/pci: pass an error object to vfio_msix_early_setup, (continued)
- [Qemu-devel] [PATCH 3/3] vfio/pci: pass an error object to vfio_msix_early_setup, Eric Auger, 2016/09/06
- [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize, Eric Auger, 2016/09/06
- Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize, Markus Armbruster, 2016/09/12
- Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize, Auger Eric, 2016/09/12
- Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize, Alex Williamson, 2016/09/12
- Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize, Auger Eric, 2016/09/12
- Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize, Alex Williamson, 2016/09/12
- Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize, Auger Eric, 2016/09/12
- Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize, Auger Eric, 2016/09/13
- Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize, Alex Williamson, 2016/09/13