From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34870) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wi3bx-0000I7-Qb for qemu-devel@nongnu.org; Wed, 07 May 2014 11:18:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wi3br-0003rg-Gy for qemu-devel@nongnu.org; Wed, 07 May 2014 11:18:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49865) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wi3br-0003qx-3V for qemu-devel@nongnu.org; Wed, 07 May 2014 11:18:15 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s47FID16004514 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Wed, 7 May 2014 11:18:14 -0400 Message-ID: <536A4EB2.7010906@redhat.com> Date: Wed, 07 May 2014 17:18:10 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <1397118285-11715-1-git-send-email-lersek@redhat.com> <5360FF1D.6020300@redhat.com> <20140507095209.02fc5377@redhat.com> In-Reply-To: <20140507095209.02fc5377@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 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: Luiz Capitulino Cc: qemu-devel@nongnu.org On 05/07/14 15:52, Luiz Capitulino wrote: > 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. Awesome, thank you. Laszlo