From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52005) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wi2wG-00069Q-2z for qemu-devel@nongnu.org; Wed, 07 May 2014 10:35:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wi2vx-00052e-63 for qemu-devel@nongnu.org; Wed, 07 May 2014 10:35:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:10304) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wi2vw-00051z-Tv for qemu-devel@nongnu.org; Wed, 07 May 2014 10:34:57 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s47EYslI030712 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Wed, 7 May 2014 10:34:54 -0400 Date: Wed, 7 May 2014 09:52:09 -0400 From: Luiz Capitulino Message-ID: <20140507095209.02fc5377@redhat.com> In-Reply-To: <5360FF1D.6020300@redhat.com> References: <1397118285-11715-1-git-send-email-lersek@redhat.com> <5360FF1D.6020300@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 00/16] PCI device assignment: improve error reporting over QMP List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek Cc: qemu-devel@nongnu.org On Wed, 30 Apr 2014 15:48:13 +0200 Laszlo Ersek wrote: > Hi Luiz, > > On 04/10/14 10:24, Laszlo Ersek wrote: > > This is for . > > > > In general, we want to propagate non-fatal (ie. non-exit()ing, > > non-abort()ing) errors to the QMP caller, rather than printing them > > on-site. The series focuses on errors raised at PCI assignment time (ie. > > reachable from assigned_initfn()), other errors are not converted. > > > > Errors are not propagated through assigned_initfn(); let's wait for > > someone else to convert "kvm-pci-assign" from qdev to QOM. The series is > > nonetheless an improvement, because the forwarding of initialization > > errors now stops just before device_realize(). We set the > > stored/persistent monitor error there. > > > > Informative and warning messages (that report about circumstances that > > don't break the assignment operation) cannot terminate in > > qerror_report_err(), because that would set the persistent monitor > > error, breaking the high level (QMP) operation. > > > > A call graph rooted in assigned_initfn() was generated with cflow. > > Leaves that could never fail were removed from the graph (and this > > property similarly propagated upwards as far as possible). > > > > The patchset loosely follows a bottom-up algorithm on this calltree. Any > > leaf that reports an error internally and returns a related failure is > > converted to "throw" an Error structure instead. All direct callers of > > the converted leaf are addressed at once, in the same patch, to consume > > the error (and they become the new leaves gradually). > > > > When the leaf to be converted is also called outside of > > "hw/i386/kvm/pci-assign.c", the conversion keeps a compatibility > > function under the original name, so that conversion of callers > > unrelated to PCI assignment can be deferred. > > > > Reviewers should copy the call graph to a text file, and mark, as the > > series progresses, fully converted functions. (Ie. functions that now > > report their terminating error messages with Error objects only.) > > > > assigned_initfn() > > error_report() > > get_real_device() > > monitor_handle_fd_param() > > error_report() > > get_real_vendor_id() > > get_real_id() > > error_report() > > get_real_device_id() > > get_real_id() > > error_report() > > assigned_device_pci_cap_init() > > check_irqchip_in_kernel() > > error_report() > > pci_add_capability() > > error_report() > > assigned_dev_register_msix_mmio() > > error_report() > > assigned_dev_register_regions() > > error_report() > > assign_device() > > error_report() > > assign_failed_examine() > > get_real_vendor_id() > > get_real_id() > > error_report() > > get_real_device_id() > > get_real_id() > > error_report() > > error_printf() > > error_report() > > assign_intx() > > check_irqchip_in_kernel() > > error_report() > > error_report() > > error_printf() > > > > Laszlo Ersek (16): > > cutils: tighten qemu_parse_fd() > > monitor: add Error-propagating monitor_handle_fd_param2() > > pci-assign: accept Error from monitor_handle_fd_param2() > > pci-assign: make assign_failed_examine() just format the cause > > pci-assign: propagate errors from get_real_id() > > pci-assign: propagate Error from check_irqchip_in_kernel() > > pci: add Error-propagating pci_add_capability2() > > pci-assign: accept Error from pci_add_capability2() > > pci-assign: assignment should fail if we can't read config space > > pci-assign: propagate errors from get_real_device() > > pci-assign: propagate errors from assigned_device_pci_cap_init() > > pci-assign: propagate errors from assigned_dev_register_msix_mmio() > > pci-assign: propagate errors from assigned_dev_register_regions() > > pci-assign: propagate errors from assign_device() > > pci-assign: propagate errors from assign_intx() > > pci-assign: assigned_initfn(): set monitor error in common error > > handler > > > > include/hw/pci/pci.h | 4 + > > include/monitor/monitor.h | 1 + > > hw/i386/kvm/pci-assign.c | 273 ++++++++++++++++++++++++++++------------------ > > hw/pci/pci.c | 32 +++++- > > monitor.c | 29 ++++- > > util/cutils.c | 13 ++- > > 6 files changed, 232 insertions(+), 120 deletions(-) > > > > I forgot to CC you on this. Applied to the qmp branch, thanks. > > Thanks > Laszlo >