Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize

[Auger Eric]

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
>