* [Qemu-devel] [PATCH] pci: Merge pci_nic_init() into pci_nic_init_nofail()
@ 2015-04-28 10:50 Thomas Huth
2015-04-28 11:42 ` Paolo Bonzini
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Thomas Huth @ 2015-04-28 10:50 UTC (permalink / raw)
To: qemu-devel, Michael S. Tsirkin; +Cc: Peter Crosthwaite, Markus Armbruster
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>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] pci: Merge pci_nic_init() into pci_nic_init_nofail()
2015-04-28 10:50 [Qemu-devel] [PATCH] pci: Merge pci_nic_init() into pci_nic_init_nofail() Thomas Huth
@ 2015-04-28 11:42 ` Paolo Bonzini
2015-04-28 15:07 ` Markus Armbruster
2015-04-28 15:55 ` Michael S. Tsirkin
2 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2015-04-28 11:42 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Michael S. Tsirkin
Cc: Peter Crosthwaite, Markus Armbruster
On 28/04/2015 12:50, 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>
> ---
> hw/pci/pci.c | 43 ++++++++++++++-----------------------------
> 1 file changed, 14 insertions(+), 29 deletions(-)
I agree that, for a _nofail function, it makes sense to remove the
Error** altogether...
Paolo
> 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)
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] pci: Merge pci_nic_init() into pci_nic_init_nofail()
2015-04-28 10:50 [Qemu-devel] [PATCH] pci: Merge pci_nic_init() into pci_nic_init_nofail() Thomas Huth
2015-04-28 11:42 ` Paolo Bonzini
@ 2015-04-28 15:07 ` Markus Armbruster
2015-04-28 15:55 ` Michael S. Tsirkin
2 siblings, 0 replies; 4+ messages in thread
From: Markus Armbruster @ 2015-04-28 15:07 UTC (permalink / raw)
To: Thomas Huth; +Cc: Peter Crosthwaite, qemu-devel, Michael S. Tsirkin
Thomas Huth <thuth@redhat.com> writes:
> 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>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] pci: Merge pci_nic_init() into pci_nic_init_nofail()
2015-04-28 10:50 [Qemu-devel] [PATCH] pci: Merge pci_nic_init() into pci_nic_init_nofail() Thomas Huth
2015-04-28 11:42 ` Paolo Bonzini
2015-04-28 15:07 ` Markus Armbruster
@ 2015-04-28 15:55 ` Michael S. Tsirkin
2 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2015-04-28 15:55 UTC (permalink / raw)
To: Thomas Huth; +Cc: Peter Crosthwaite, qemu-devel, Markus Armbruster
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-04-28 15:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-28 10:50 [Qemu-devel] [PATCH] pci: Merge pci_nic_init() into pci_nic_init_nofail() Thomas Huth
2015-04-28 11:42 ` Paolo Bonzini
2015-04-28 15:07 ` Markus Armbruster
2015-04-28 15:55 ` Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.