Date: Tue, 28 Apr 2015 17:55:21 +0200
From: "Michael S. Tsirkin" <mst@redhat.com>
Message-ID: <20150428175513-mutt-send-email-mst@redhat.com>
References: <1430218207-7079-1-git-send-email-thuth@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1430218207-7079-1-git-send-email-thuth@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] pci: Merge pci_nic_init() into
	pci_nic_init_nofail()
To: Thomas Huth <thuth@redhat.com>
Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>, qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>

On Tue, Apr 28, 2015 at 12:50:07PM +0200, Thomas Huth wrote:
> The error reporting in pci_nic_init() is quite erratic: Some errors
> are printed directly with error_report(), and some are passed back
> to the caller pci_nic_init_nofail() via an Error pointer.
> Since pci_nic_init() is only used by pci_nic_init_nofail(), the
> functions can be simply merged to clean up this inconsistency.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Applied, thanks everyone.

> ---
>  hw/pci/pci.c | 43 ++++++++++++++-----------------------------
>  1 file changed, 14 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index b3d5100..56947ae 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1611,28 +1611,32 @@ static const char * const pci_nic_names[] = {
>  };
>  
>  /* Initialize a PCI NIC.  */
> -static PCIDevice *pci_nic_init(NICInfo *nd, PCIBus *rootbus,
> +PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
>                                 const char *default_model,
> -                               const char *default_devaddr,
> -                               Error **errp)
> +                               const char *default_devaddr)
>  {
>      const char *devaddr = nd->devaddr ? nd->devaddr : default_devaddr;
>      Error *err = NULL;
>      PCIBus *bus;
> -    int devfn;
>      PCIDevice *pci_dev;
>      DeviceState *dev;
> +    int devfn;
>      int i;
>  
> +    if (qemu_show_nic_models(nd->model, pci_nic_models)) {
> +        exit(0);
> +    }
> +
>      i = qemu_find_nic_model(nd, pci_nic_models, default_model);
> -    if (i < 0)
> -        return NULL;
> +    if (i < 0) {
> +        exit(1);
> +    }
>  
>      bus = pci_get_bus_devfn(&devfn, rootbus, devaddr);
>      if (!bus) {
>          error_report("Invalid PCI device address %s for device %s",
>                       devaddr, pci_nic_names[i]);
> -        return NULL;
> +        exit(1);
>      }
>  
>      pci_dev = pci_create(bus, devfn, pci_nic_names[i]);
> @@ -1641,31 +1645,12 @@ static PCIDevice *pci_nic_init(NICInfo *nd, PCIBus *rootbus,
>  
>      object_property_set_bool(OBJECT(dev), true, "realized", &err);
>      if (err) {
> -        error_propagate(errp, err);
> +        error_report_err(err);
>          object_unparent(OBJECT(dev));
> -        return NULL;
> -    }
> -    return pci_dev;
> -}
> -
> -PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
> -                               const char *default_model,
> -                               const char *default_devaddr)
> -{
> -    Error *err = NULL;
> -    PCIDevice *res;
> -
> -    if (qemu_show_nic_models(nd->model, pci_nic_models))
> -        exit(0);
> -
> -    res = pci_nic_init(nd, rootbus, default_model, default_devaddr, &err);
> -    if (!res) {
> -        if (err) {
> -            error_report_err(err);
> -        }
>          exit(1);
>      }
> -    return res;
> +
> +    return pci_dev;
>  }
>  
>  PCIDevice *pci_vga_init(PCIBus *bus)
> -- 
> 1.8.3.1