* [Qemu-devel] [PATCH 03/10] pci: add creation functions that may fail
@ 2011-02-03 20:59 Blue Swirl
2011-02-12 17:10 ` Markus Armbruster
0 siblings, 1 reply; 3+ messages in thread
From: Blue Swirl @ 2011-02-03 20:59 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
hw/pci.c | 20 ++++++++++++++++++++
hw/pci.h | 4 ++++
2 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index d5bbba9..5e6e216 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1708,6 +1708,21 @@ PCIDevice *pci_create_multifunction(PCIBus
*bus, int devfn, bool multifunction,
return DO_UPCAST(PCIDevice, qdev, dev);
}
+PCIDevice *pci_try_create_multifunction(PCIBus *bus, int devfn,
+ bool multifunction,
+ const char *name)
+{
+ DeviceState *dev;
+
+ dev = qdev_try_create(&bus->qbus, name);
+ if (!dev) {
+ return NULL;
+ }
+ qdev_prop_set_uint32(dev, "addr", devfn);
+ qdev_prop_set_bit(dev, "multifunction", multifunction);
+ return DO_UPCAST(PCIDevice, qdev, dev);
+}
+
PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
bool multifunction,
const char *name)
@@ -1727,6 +1742,11 @@ PCIDevice *pci_create_simple(PCIBus *bus, int
devfn, const char *name)
return pci_create_simple_multifunction(bus, devfn, false, name);
}
+PCIDevice *pci_try_create(PCIBus *bus, int devfn, const char *name)
+{
+ return pci_try_create_multifunction(bus, devfn, false, name);
+}
+
static int pci_find_space(PCIDevice *pdev, uint8_t size)
{
int config_size = pci_config_size(pdev);
diff --git a/hw/pci.h b/hw/pci.h
index 0d2753f..113e556 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -453,8 +453,12 @@ PCIDevice *pci_create_multifunction(PCIBus *bus,
int devfn, bool multifunction,
PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
bool multifunction,
const char *name);
+PCIDevice *pci_try_create_multifunction(PCIBus *bus, int devfn,
+ bool multifunction,
+ const char *name);
PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);
PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
+PCIDevice *pci_try_create(PCIBus *bus, int devfn, const char *name);
static inline int pci_is_express(const PCIDevice *d)
{
--
1.6.2.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH 03/10] pci: add creation functions that may fail
2011-02-03 20:59 [Qemu-devel] [PATCH 03/10] pci: add creation functions that may fail Blue Swirl
@ 2011-02-12 17:10 ` Markus Armbruster
2011-02-12 17:25 ` Blue Swirl
0 siblings, 1 reply; 3+ messages in thread
From: Markus Armbruster @ 2011-02-12 17:10 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-devel
Blue Swirl <blauwirbel@gmail.com> writes:
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> ---
> hw/pci.c | 20 ++++++++++++++++++++
> hw/pci.h | 4 ++++
> 2 files changed, 24 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index d5bbba9..5e6e216 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1708,6 +1708,21 @@ PCIDevice *pci_create_multifunction(PCIBus
> *bus, int devfn, bool multifunction,
> return DO_UPCAST(PCIDevice, qdev, dev);
> }
>
> +PCIDevice *pci_try_create_multifunction(PCIBus *bus, int devfn,
> + bool multifunction,
> + const char *name)
> +{
> + DeviceState *dev;
> +
> + dev = qdev_try_create(&bus->qbus, name);
> + if (!dev) {
> + return NULL;
> + }
> + qdev_prop_set_uint32(dev, "addr", devfn);
> + qdev_prop_set_bit(dev, "multifunction", multifunction);
> + return DO_UPCAST(PCIDevice, qdev, dev);
> +}
> +
Near-duplicate of pci_create_multifunction(). What about implementing
pci_create_multifunction() on top of pci_try_create_multifunction()?
> PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
> bool multifunction,
> const char *name)
> @@ -1727,6 +1742,11 @@ PCIDevice *pci_create_simple(PCIBus *bus, int
> devfn, const char *name)
> return pci_create_simple_multifunction(bus, devfn, false, name);
> }
>
> +PCIDevice *pci_try_create(PCIBus *bus, int devfn, const char *name)
> +{
> + return pci_try_create_multifunction(bus, devfn, false, name);
> +}
> +
> static int pci_find_space(PCIDevice *pdev, uint8_t size)
> {
> int config_size = pci_config_size(pdev);
> diff --git a/hw/pci.h b/hw/pci.h
> index 0d2753f..113e556 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -453,8 +453,12 @@ PCIDevice *pci_create_multifunction(PCIBus *bus,
> int devfn, bool multifunction,
> PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
> bool multifunction,
> const char *name);
> +PCIDevice *pci_try_create_multifunction(PCIBus *bus, int devfn,
> + bool multifunction,
> + const char *name);
> PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);
> PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
> +PCIDevice *pci_try_create(PCIBus *bus, int devfn, const char *name);
>
> static inline int pci_is_express(const PCIDevice *d)
> {
Six functions to create PCI devices the "old way", i.e. from board setup
code. Isn't that baroque? :)
Existing code varies them along two dimensions:
* Automatically "init" the device ("simple") or not. The "simple"
functions save their callers wrapping qdev_init_nofail() around the
non-simple ones. Marginally useful.
Aside: calling something that combinines create with init
"create_simple" isn't exactly the acme of good taste, in my opinion.
* Multifunction parameter or not. The non-"multifunction" functions
save their callers passing a false argument. They also make the more
general function have a much longer name. Bah.
You add a third:
* May return failure or not. You don't implement it for "simple", thus
we end up with "only" six rather than eight functions.
I figure just your pci_try_create_multifunction() and
pci_create_simple_multifunction() would do just fine. With names of
more sensible length.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH 03/10] pci: add creation functions that may fail
2011-02-12 17:10 ` Markus Armbruster
@ 2011-02-12 17:25 ` Blue Swirl
0 siblings, 0 replies; 3+ messages in thread
From: Blue Swirl @ 2011-02-12 17:25 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel
On Sat, Feb 12, 2011 at 7:10 PM, Markus Armbruster <armbru@redhat.com> wrote:
> Blue Swirl <blauwirbel@gmail.com> writes:
>
>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>> ---
>> hw/pci.c | 20 ++++++++++++++++++++
>> hw/pci.h | 4 ++++
>> 2 files changed, 24 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/pci.c b/hw/pci.c
>> index d5bbba9..5e6e216 100644
>> --- a/hw/pci.c
>> +++ b/hw/pci.c
>> @@ -1708,6 +1708,21 @@ PCIDevice *pci_create_multifunction(PCIBus
>> *bus, int devfn, bool multifunction,
>> return DO_UPCAST(PCIDevice, qdev, dev);
>> }
>>
>> +PCIDevice *pci_try_create_multifunction(PCIBus *bus, int devfn,
>> + bool multifunction,
>> + const char *name)
>> +{
>> + DeviceState *dev;
>> +
>> + dev = qdev_try_create(&bus->qbus, name);
>> + if (!dev) {
>> + return NULL;
>> + }
>> + qdev_prop_set_uint32(dev, "addr", devfn);
>> + qdev_prop_set_bit(dev, "multifunction", multifunction);
>> + return DO_UPCAST(PCIDevice, qdev, dev);
>> +}
>> +
>
> Near-duplicate of pci_create_multifunction(). What about implementing
> pci_create_multifunction() on top of pci_try_create_multifunction()?
I considered that, but then the error handling would be duplicated
(from qdev error handling).
>> PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
>> bool multifunction,
>> const char *name)
>> @@ -1727,6 +1742,11 @@ PCIDevice *pci_create_simple(PCIBus *bus, int
>> devfn, const char *name)
>> return pci_create_simple_multifunction(bus, devfn, false, name);
>> }
>>
>> +PCIDevice *pci_try_create(PCIBus *bus, int devfn, const char *name)
>> +{
>> + return pci_try_create_multifunction(bus, devfn, false, name);
>> +}
>> +
>> static int pci_find_space(PCIDevice *pdev, uint8_t size)
>> {
>> int config_size = pci_config_size(pdev);
>> diff --git a/hw/pci.h b/hw/pci.h
>> index 0d2753f..113e556 100644
>> --- a/hw/pci.h
>> +++ b/hw/pci.h
>> @@ -453,8 +453,12 @@ PCIDevice *pci_create_multifunction(PCIBus *bus,
>> int devfn, bool multifunction,
>> PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
>> bool multifunction,
>> const char *name);
>> +PCIDevice *pci_try_create_multifunction(PCIBus *bus, int devfn,
>> + bool multifunction,
>> + const char *name);
>> PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);
>> PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
>> +PCIDevice *pci_try_create(PCIBus *bus, int devfn, const char *name);
>>
>> static inline int pci_is_express(const PCIDevice *d)
>> {
>
> Six functions to create PCI devices the "old way", i.e. from board setup
> code. Isn't that baroque? :)
No, this is the current way. ;-)
> Existing code varies them along two dimensions:
>
> * Automatically "init" the device ("simple") or not. The "simple"
> functions save their callers wrapping qdev_init_nofail() around the
> non-simple ones. Marginally useful.
>
> Aside: calling something that combinines create with init
> "create_simple" isn't exactly the acme of good taste, in my opinion.
>
> * Multifunction parameter or not. The non-"multifunction" functions
> save their callers passing a false argument. They also make the more
> general function have a much longer name. Bah.
>
> You add a third:
>
> * May return failure or not. You don't implement it for "simple", thus
> we end up with "only" six rather than eight functions.
>
> I figure just your pci_try_create_multifunction() and
> pci_create_simple_multifunction() would do just fine. With names of
> more sensible length.
Alternatively there could be one or two combined generic functions
with parameters to specify whether to return on failure or not etc.,
others could be implemented on top of these.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-02-12 17:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-03 20:59 [Qemu-devel] [PATCH 03/10] pci: add creation functions that may fail Blue Swirl
2011-02-12 17:10 ` Markus Armbruster
2011-02-12 17:25 ` Blue Swirl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).