From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35575) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bjhAS-0007tF-4n for qemu-devel@nongnu.org; Tue, 13 Sep 2016 02:26:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bjhAN-0004yW-Qx for qemu-devel@nongnu.org; Tue, 13 Sep 2016 02:26:03 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59228) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bjhAN-0004yM-J3 for qemu-devel@nongnu.org; Tue, 13 Sep 2016 02:25:59 -0400 From: Markus Armbruster References: <1473145893-17088-1-git-send-email-eric.auger@redhat.com> <1473145893-17088-2-git-send-email-eric.auger@redhat.com> <87y42xxp5m.fsf@dusky.pond.sub.org> <20160912101722.149f3a8a@t450s.home> Date: Tue, 13 Sep 2016 08:25:56 +0200 In-Reply-To: <20160912101722.149f3a8a@t450s.home> (Alex Williamson's message of "Mon, 12 Sep 2016 10:17:22 -0600") Message-ID: <878tuwiadn.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: Auger Eric , eric.auger.pro@gmail.com, qemu-devel@nongnu.org Alex Williamson writes: > On Mon, 12 Sep 2016 16:00:18 +0200 > Auger Eric wrote: > >> Hi Markus, >> >> On 12/09/2016 14:45, Markus Armbruster wrote: >> > Eric Auger 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. [...]