[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize
From: |
Auger Eric |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize |
Date: |
Mon, 12 Sep 2016 22:51:57 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 |
Hi,
On 12/09/2016 22:05, Alex Williamson wrote:
> On Mon, 12 Sep 2016 21:39:22 +0200
> Auger Eric <address@hidden> wrote:
>
>> Hi,
>> On 12/09/2016 18:17, Alex Williamson wrote:
>>> 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.
>> On my side I would be inclined to keep the "vfio: BDF" prefix. Maybe the
>> PCI domain may be omitted?
>
> I don't really see the point of making the device name smaller. If a
> user happens to have multiple domains, they're going to care about that
> component of the address. Is QEMU going to probe the host system to
> see if multiple domains are available and update if a new PCI chassis
> handled as a separate domain is hot attached? Even an approach like
> only printing the domain if it's non-zero devolves into needing logic
> to know that basename is a PCI path and not a random sysfs device
> path. And then if we only print the domain when non-zero, what about
> the bus number or slot number. It's a lot of logic for a problem that
> I'm not even convinced is a problem. Thanks,
I tend to agree. So I will keep the prefix as is and take into account
Markus' other comments.
Thanks!
Eric
>
> Alex
>
- Re: [Qemu-devel] [PATCH 2/3] vfio/pci: pass an error object to vfio_populate_device, (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/13
- 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