Hi, On 12/09/2016 22:05, Alex Williamson wrote: > On Mon, 12 Sep 2016 21:39:22 +0200 > Auger Eric <eric.au...@redhat.com> wrote: > >> Hi, >> On 12/09/2016 18:17, Alex Williamson wrote: >>> On Mon, 12 Sep 2016 16:00:18 +0200 >>> Auger Eric <eric.au...@redhat.com> wrote: >>> >>>> Hi Markus, >>>> >>>> On 12/09/2016 14:45, Markus Armbruster wrote: >>>>> Eric Auger <eric.au...@redhat.com> 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 >