[02/10] qdev: add creation function that may fail

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 hw/qdev.c |   14 +++++++++++++-
 hw/qdev.h |    1 +
 2 files changed, 14 insertions(+), 1 deletions(-)

Blue Swirl <blauwirbel@gmail.com> writes:

> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> ---
>  hw/qdev.c |   14 +++++++++++++-
>  hw/qdev.h |    1 +
>  2 files changed, 14 insertions(+), 1 deletions(-)
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index c7fec44..1aa1ea0 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -106,6 +106,18 @@ static DeviceState
> *qdev_create_from_info(BusState *bus, DeviceInfo *info)
>     initialize the actual device emulation.  */
>  DeviceState *qdev_create(BusState *bus, const char *name)
>  {
> +    DeviceState *dev;
> +
> +    dev = qdev_try_create(bus, name);
> +    if (!dev) {
> +        hw_error("Unknown device '%s' for bus '%s'\n", name, bus->info->name);

Aside: I never liked the use hw_error() for this purpose.  Dumping CPU
state isn't helpful when the problem is a bad device name in board setup
code.

> +    }
> +
> +    return dev;
> +}
> +
> +DeviceState *qdev_try_create(BusState *bus, const char *name)
> +{
>      DeviceInfo *info;
>
>      if (!bus) {
> @@ -114,7 +126,7 @@ DeviceState *qdev_create(BusState *bus, const char *name)
>
>      info = qdev_find_info(bus->info, name);
>      if (!info) {
> -        hw_error("Unknown device '%s' for bus '%s'\n", name, bus->info->name);
> +        return NULL;
>      }
>
>      return qdev_create_from_info(bus, info);
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 9808f85..8a13ec9 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -122,6 +122,7 @@ typedef struct GlobalProperty {
>  /*** Board API.  This should go away once we have a machine config file.  ***/
>
>  DeviceState *qdev_create(BusState *bus, const char *name);
> +DeviceState *qdev_try_create(BusState *bus, const char *name);
>  int qdev_device_help(QemuOpts *opts);
>  DeviceState *qdev_device_add(QemuOpts *opts);
>  int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT;

I'd prefer these to follow qdev_init() / qdev_init_nofail() precedence,
for consistency, i.e. rename existing qdev_create() to
qdev_create_nofail(), call your new function qdev_create().

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/qdev.c |   14 +++++++++++++-
>>  hw/qdev.h |    1 +
>>  2 files changed, 14 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/qdev.c b/hw/qdev.c
>> index c7fec44..1aa1ea0 100644
>> --- a/hw/qdev.c
>> +++ b/hw/qdev.c
>> @@ -106,6 +106,18 @@ static DeviceState
>> *qdev_create_from_info(BusState *bus, DeviceInfo *info)
>>     initialize the actual device emulation.  */
>>  DeviceState *qdev_create(BusState *bus, const char *name)
>>  {
>> +    DeviceState *dev;
>> +
>> +    dev = qdev_try_create(bus, name);
>> +    if (!dev) {
>> +        hw_error("Unknown device '%s' for bus '%s'\n", name, bus->info->name);
>
> Aside: I never liked the use hw_error() for this purpose.  Dumping CPU
> state isn't helpful when the problem is a bad device name in board setup
> code.

Right, perhaps hw_error shouldn't dump CPU state. Instead there should
be a new function (cpu_error) for that.

>> +    }
>> +
>> +    return dev;
>> +}
>> +
>> +DeviceState *qdev_try_create(BusState *bus, const char *name)
>> +{
>>      DeviceInfo *info;
>>
>>      if (!bus) {
>> @@ -114,7 +126,7 @@ DeviceState *qdev_create(BusState *bus, const char *name)
>>
>>      info = qdev_find_info(bus->info, name);
>>      if (!info) {
>> -        hw_error("Unknown device '%s' for bus '%s'\n", name, bus->info->name);
>> +        return NULL;
>>      }
>>
>>      return qdev_create_from_info(bus, info);
>> diff --git a/hw/qdev.h b/hw/qdev.h
>> index 9808f85..8a13ec9 100644
>> --- a/hw/qdev.h
>> +++ b/hw/qdev.h
>> @@ -122,6 +122,7 @@ typedef struct GlobalProperty {
>>  /*** Board API.  This should go away once we have a machine config file.  ***/
>>
>>  DeviceState *qdev_create(BusState *bus, const char *name);
>> +DeviceState *qdev_try_create(BusState *bus, const char *name);
>>  int qdev_device_help(QemuOpts *opts);
>>  DeviceState *qdev_device_add(QemuOpts *opts);
>>  int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT;
>
> I'd prefer these to follow qdev_init() / qdev_init_nofail() precedence,
> for consistency, i.e. rename existing qdev_create() to
> qdev_create_nofail(), call your new function qdev_create().

That would be a big rename operation. Care to submit a patch?