Re: [Qemu-devel] [PATCH 00/16] PCI device assignment: improve error reporting over QMP

[Luiz Capitulino <lcapitulino@redhat.com>]

On Wed, 30 Apr 2014 15:48:13 +0200
Laszlo Ersek <lersek@redhat.com> wrote:

> Hi Luiz,
> 
> On 04/10/14 10:24, Laszlo Ersek wrote:
> > This is for <https://bugzilla.redhat.com/show_bug.cgi?id=616415>.
> > 
> > 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
>