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

[Alex Williamson]

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,

Alex