Patchwork [02/10] parallel: make optional

login
register
mail settings
Submitter Blue Swirl
Date Feb. 12, 2011, 10:40 p.m.
Message ID <AANLkTimVWX+4CYiqrpOQOMwH+SnUHPgbXHDGgQByg6r9@mail.gmail.com>
Download mbox | patch
Permalink /patch/82948/
State New
Headers show

Comments

Blue Swirl - Feb. 12, 2011, 10:40 p.m.
Ignore failure with parallel device creation.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 hw/pc.h |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

     if (qdev_init(&dev->qdev) < 0) {
David Ahern - Feb. 13, 2011, 5:30 p.m.
On 02/12/11 15:40, Blue Swirl wrote:
> Ignore failure with parallel device creation.
> 
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> ---
>  hw/pc.h |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/pc.h b/hw/pc.h
> index 443ba34..f823b7d 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -24,7 +24,10 @@ static inline bool parallel_init(int index,
> CharDriverState *chr)
>  {
>      ISADevice *dev;
> 
> -    dev = isa_create("isa-parallel");
> +    dev = isa_try_create("isa-parallel");
> +    if (!dev) {
> +        return false;
> +    }
>      qdev_prop_set_uint32(&dev->qdev, "index", index);
>      qdev_prop_set_chr(&dev->qdev, "chardev", chr);
>      if (qdev_init(&dev->qdev) < 0) {


How is this design supposed to be better than wrapping init functions in
#ifdef CONFIG_XXXX ... #endif?

If a hardware model is compiled out via the CONFIG options qemu should
fail to accept the command line parameters and not try to create the
device. Instead with this design it tries and fails to create the device
and yet continues on.

David
Blue Swirl - Feb. 13, 2011, 6:39 p.m.
On Sun, Feb 13, 2011 at 7:30 PM, David Ahern <dsahern@gmail.com> wrote:
>
>
> On 02/12/11 15:40, Blue Swirl wrote:
>> Ignore failure with parallel device creation.
>>
>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>> ---
>>  hw/pc.h |    5 ++++-
>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/pc.h b/hw/pc.h
>> index 443ba34..f823b7d 100644
>> --- a/hw/pc.h
>> +++ b/hw/pc.h
>> @@ -24,7 +24,10 @@ static inline bool parallel_init(int index,
>> CharDriverState *chr)
>>  {
>>      ISADevice *dev;
>>
>> -    dev = isa_create("isa-parallel");
>> +    dev = isa_try_create("isa-parallel");
>> +    if (!dev) {
>> +        return false;
>> +    }
>>      qdev_prop_set_uint32(&dev->qdev, "index", index);
>>      qdev_prop_set_chr(&dev->qdev, "chardev", chr);
>>      if (qdev_init(&dev->qdev) < 0) {
>
>
> How is this design supposed to be better than wrapping init functions in
> #ifdef CONFIG_XXXX ... #endif?

We avoid the #ifdeffery and the device model is not violated. Most of
the code is not affected in any way by leaving the device out.

> If a hardware model is compiled out via the CONFIG options qemu should
> fail to accept the command line parameters and not try to create the
> device. Instead with this design it tries and fails to create the device
> and yet continues on.

Maybe command line parameter handling should be pushed to the devices.
The devices should pull their parameters, removing them thus from a
parameter pool. After the machine init finishes, errors could be
reported for unhandled parameters.

Patch

diff --git a/hw/pc.h b/hw/pc.h
index 443ba34..f823b7d 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -24,7 +24,10 @@  static inline bool parallel_init(int index,
CharDriverState *chr)
 {
     ISADevice *dev;

-    dev = isa_create("isa-parallel");
+    dev = isa_try_create("isa-parallel");
+    if (!dev) {
+        return false;
+    }
     qdev_prop_set_uint32(&dev->qdev, "index", index);
     qdev_prop_set_chr(&dev->qdev, "chardev", chr);