Re: [Qemu-devel] [PATCH 15/16] pci-assign: propagate errors from assign_intx()

[Eric Blake]

On 04/10/2014 02:24 AM, Laszlo Ersek wrote:
> Among the callers, only assigned_initfn() should set the  monitor's stored

unintentional double space

> error. Other callers may run in contexts where the monitor's stored error
> makes no sense. For example:
> 
> assigned_dev_pci_write_config()
>   assigned_dev_update_msix()
>     assign_intx()
> 
> Signed-off-by: Laszlo Ersek <address@hidden>
> ---
>  hw/i386/kvm/pci-assign.c | 39 ++++++++++++++++++++++++++++-----------
>  1 file changed, 28 insertions(+), 11 deletions(-)
> 

> @@ -954,12 +954,15 @@ static void assigned_dev_update_irq_routing(PCIDevice 
> *dev)
>  {
>      AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, dev);
>      Error *err = NULL;
>      int r;
>  
> -    r = assign_intx(assigned_dev);
> +    r = assign_intx(assigned_dev, &err);
>      if (r < 0) {
> +        error_report("%s", error_get_pretty(err));
> +        error_free(err);
> +        err = NULL;

Rather than assigning err = NULL,

>          qdev_unplug(&dev->qdev, &err);
>          assert(!err);

why not also replace these two lines with the one-liner
 qdev_unplug(&dev->qdev, &error_abort);

I'm also a bit surprised this used 'err' instead of 'local_err'; but it
was pre-existing; I'm also not sure if the recent error cleanup patches
from Markus will impact this code.

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature